: Fix silly output for virtio devices in /proc/interrupts]
Subsystems (3)
PATCH 1/1 : Fix silly output for virtio devices in /proc/interrupts]
(sent to the kvm list erroneously)
register_virtio_device() is doing something silly by overwriting what the caller put into .bus_id. This causes the interrupt line for all virtio devices to show up as "0", "1", etc. in /proc/interrupts. The attached patch fixes it.
commit cb97605728fc1d7a15ddc6ab689e8bcd23871133
Author: Chris Lalancette <[email protected]> Date: Thu May 15 09:04:55 2008 -0400
register_virtio_device was doing something silly, in that it was overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of /proc/interrupts, since when you request_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not have register_virtio_device do anything with the bus_id, and assume the higher level driver set it up properly.
drivers/virtio/virtio.c +0 -1
@@ -128,7 +128,6 @@ int register_virtio_device(struct virtio_device *dev)int err; dev->dev.bus = &virtio_bus; -sprintf (dev->dev.bus_id,"%u" , dev->index);/* We always start by resetting the device, in case a previous * driver messed it up. This also tests that code path a little. */
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Quoted 10 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]>
Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.
OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd).
===
virtio: S/390 set name of virtio devices directly.
Chris has a patch 'Fix silly output for virtio devices in /proc/interrupts' which requires callers to the virtio driver infrastructure to set the bus_ids themselves. This does that for s/390.
--- drivers/s390/kvm/kvm_virtio.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/s390/kvm/kvm_virtio.c +2 -0
@@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; +snprintf (kdev->vdev.dev.bus_id,BUS_ID_SIZE ,"virtio%d" , + kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d;
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Quoted 10 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]>
Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.
OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd).
===
virtio: S/390 set name of virtio devices directly.
Chris has a patch 'Fix silly output for virtio devices in /proc/interrupts' which requires callers to the virtio driver infrastructure to set the bus_ids themselves. This does that for s/390.
--- drivers/s390/kvm/kvm_virtio.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/s390/kvm/kvm_virtio.c +2 -0
@@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; +snprintf (kdev->vdev.dev.bus_id,BUS_ID_SIZE ,"virtio%d" , + kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d;
Quoted 12 lines · Rusty Russell
Quoted 10 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]> Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was
overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.
Quoted 3 lines
OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
Ah, OK. Alternatively, we could do:
snprintf(bus_id, BUS_ID_SIZE, "virtio%d", index)
in register_virtio_device(), and just fix the one user who does it themselves (kvm) to not duplicate the work.
Either way is fine with me.
Chris Lalancette
Quoted 12 lines · Rusty Russell
Quoted 10 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]> Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was
overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.
Quoted 3 lines
OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
Ah, OK. Alternatively, we could do:
snprintf(bus_id, BUS_ID_SIZE, "virtio%d", index)
in register_virtio_device(), and just fix the one user who does it themselves (kvm) to not duplicate the work.
Either way is fine with me.
Chris Lalancette
Am Donnerstag, 22. Mai 2008 schrieb Rusty Russell:
Quoted 4 lines
virtio: S/390 set name of virtio devices directly.
Chris has a patch 'Fix silly output for virtio devices in
/proc/interrupts' which requires callers to the virtio driver infrastructure to set the
bus_ids
Quoted 18 lines
themselves. This does that for s/390.
Signed-off-by Rusty Russell <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Martin Schwidefsky <[email protected]>Cc Carsten Otte <[email protected]>Cc Heiko Carstens <[email protected]>Cc Chris Lalancette <[email protected]>--- drivers/s390/kvm/kvm_virtio.c | 2 ++ 1 file changed, 2 insertions(+)diff -r
c903ef6b391fdrivers/s390/kvm/kvm_virtio.c
drivers/s390/kvm/kvm_virtio.c static void add_kvm_device(struct kvm_de · line 272
Quoted 8 lines
drivers/s390/kvm/kvm_virtio.c +2
-0@@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; +snprintf (kdev->vdev.dev.bus_id,BUS_ID_SIZE ,"virtio%d" , + kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d;
This will probably create a fuzzy hunk with another patch from Cornelia Huck (CCed) which will be send to Gregs tree soon. This patch converts kvm_root to dynamic allocation, but the fuzz will be easy to fix.
Christian
Am Donnerstag, 22. Mai 2008 schrieb Rusty Russell:
Quoted 4 lines
virtio: S/390 set name of virtio devices directly.
Chris has a patch 'Fix silly output for virtio devices in
/proc/interrupts' which requires callers to the virtio driver infrastructure to set the
bus_ids
Quoted 18 lines
themselves. This does that for s/390.
Signed-off-by Rusty Russell <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Martin Schwidefsky <[email protected]>Cc Carsten Otte <[email protected]>Cc Heiko Carstens <[email protected]>Cc Chris Lalancette <[email protected]>--- drivers/s390/kvm/kvm_virtio.c | 2 ++ 1 file changed, 2 insertions(+)diff -r
c903ef6b391fdrivers/s390/kvm/kvm_virtio.c
drivers/s390/kvm/kvm_virtio.c static void add_kvm_device(struct kvm_de · line 272
Quoted 8 lines
drivers/s390/kvm/kvm_virtio.c +2
-0@@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; +snprintf (kdev->vdev.dev.bus_id,BUS_ID_SIZE ,"virtio%d" , + kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d;
This will probably create a fuzzy hunk with another patch from Cornelia Huck (CCed) which will be send to Gregs tree soon. This patch converts kvm_root to dynamic allocation, but the fuzz will be easy to fix.
Christian
Quoted 14 lines · Rusty Russell
Quoted 11 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]> Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was
overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.
Quoted 7 lines
OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd).
It may actually be better for virtio to set this up. The problem is that if you have multiple transports that are registering virtio devices, it's impossible at the transport level to guarantee uniqueness while still using the "virtio%d" naming. Except the current scheme is no good, we'd have to push the dev_index into virtio too.
Regards,
Anthony Liguori
Quoted 35 lines
===
virtio: S/390 set name of virtio devices directly.Chris has a patch 'Fix silly output for virtio devices in
/proc/interrupts' which requires callers to the virtio driver infrastructure to set thebus_idsthemselves. This does that for s/390.Signed-off-by Rusty Russell <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Martin Schwidefsky <[email protected]>Cc Carsten Otte <[email protected]>Cc Heiko Carstens <[email protected]>Cc Chris Lalancette <[email protected]>--- drivers/s390/kvm/kvm_virtio.c | 2 ++ 1 file changed, 2 insertions(+)diff -r
c903ef6b391fdrivers/s390/kvm/kvm_virtio.cdrivers/s390/kvm/kvm_virtio.c +2
-1@@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; +snprintf (kdev->vdev.dev.bus_id,BUS_ID_SIZE ,"virtio%d" , + kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d; --To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 14 lines · Rusty Russell
Quoted 11 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]> Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was
overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.
Quoted 7 lines
OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd).
It may actually be better for virtio to set this up. The problem is that if you have multiple transports that are registering virtio devices, it's impossible at the transport level to guarantee uniqueness while still using the "virtio%d" naming. Except the current scheme is no good, we'd have to push the dev_index into virtio too.
Regards,
Anthony Liguori
Quoted 35 lines
===
virtio: S/390 set name of virtio devices directly.Chris has a patch 'Fix silly output for virtio devices in
/proc/interrupts' which requires callers to the virtio driver infrastructure to set thebus_idsthemselves. This does that for s/390.Signed-off-by Rusty Russell <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Martin Schwidefsky <[email protected]>Cc Carsten Otte <[email protected]>Cc Heiko Carstens <[email protected]>Cc Chris Lalancette <[email protected]>--- drivers/s390/kvm/kvm_virtio.c | 2 ++ 1 file changed, 2 insertions(+)diff -r
c903ef6b391fdrivers/s390/kvm/kvm_virtio.cdrivers/s390/kvm/kvm_virtio.c +2
-1@@ -265,6 +265,8 @@ static void add_kvm_device(struct kvm_de kdev->vdev.dev.parent = &kvm_root; kdev->vdev.index = dev_index++; +snprintf (kdev->vdev.dev.bus_id,BUS_ID_SIZE ,"virtio%d" , + kdev->vdev.index); kdev->vdev.id.device = d->type; kdev->vdev.config = &kvm_vq_configspace_ops; kdev->desc = d; --To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 17 lines · On Friday 23 May 2008 07:58:44 Anthony Liguori wrote:
Quoted 16 lines · Rusty Russell
Quoted 10 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]> Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was
overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
Quoted 8 lines
Quoted 2 lines
The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd).
It may actually be better for virtio to set this up. The problem is that if you have multiple transports that are registering virtio devices, it's impossible at the transport level to guarantee uniqueness while still using the "virtio%d" naming. Except the current scheme is no good, we'd have to push the
dev_indexinto virtio too.
That's true. OK, let's hoist the index counter into common code instead. The alternative is to use different busid namings for each transport, and that seems like too much confusion: sysfs will tell you where it comes from if you really need to know.
Patch series coming.
Thanks,
Rusty.
Quoted 17 lines · On Friday 23 May 2008 07:58:44 Anthony Liguori wrote:
Quoted 16 lines · Rusty Russell
Quoted 10 lines · On Wednesday 21 May 2008 23:13:05 Chris Lalancette wrote:
Author: Chris Lalancette <[email protected]> Date: Thu May 15 09:04:55 2008 -0400
register_virtio_devicewas doing something silly, in that it was
overwriting what the calling driver stuck into .bus_id" for the name. This caused problems in the output of/proc/interrupts, since when yourequest_irq(), it doesn't actually copy the devname you pass in but just stores a pointer to the data. The fix is to just not haveregister_virtio_devicedo anything with thebus_id, and assume the higher level driver set it up properly.OK, but only one higher-level driver will set it up properly: kvm. Neither lguest nor s/390 do this, and as a result, they fail to register *any* devices.
Quoted 8 lines
Quoted 2 lines
The following patch should fix it for s/390 (it's identical to the lguest patch), but would prefer testing (S/390-ers cc'd).
It may actually be better for virtio to set this up. The problem is that if you have multiple transports that are registering virtio devices, it's impossible at the transport level to guarantee uniqueness while still using the "virtio%d" naming. Except the current scheme is no good, we'd have to push the
dev_indexinto virtio too.
That's true. OK, let's hoist the index counter into common code instead. The alternative is to use different busid namings for each transport, and that seems like too much confusion: sysfs will tell you where it comes from if you really need to know.
Patch series coming.
Thanks,
Rusty.