| Message ID | a8c2db3b9b1f3c4bb81aca352b69e33260f36545.1361246206.git.peter.crosthwaite@xilinx.com |
|---|---|
| State | New |
| Headers | show |
Il 19/02/2013 05:02, Peter Crosthwaite ha scritto: > num_interfaces only tells you how many interfaces the concrete child class has > (as defined in the TypeInfo). This means if you have a child class which defines > no interfaces of its own, but its parent has interfaces you cannot cast to those > parent interfaces. > > Fixed changing the guard to check the class->interfaces list instead (which is > a complete flattened list of implemented interfaces). > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > changed from v1: > Change guard implementation rather than removed it (Paolo review) > > qom/object.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 4b72a64..3d638ff 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -449,7 +449,8 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class, > TypeImpl *type = class->type; > ObjectClass *ret = NULL; > > - if (type->num_interfaces && type_is_ancestor(target_type, type_interface)) { > + if (type->class->interfaces && > + type_is_ancestor(target_type, type_interface)) { > int found = 0; > GSList *i; > > Series looks good, thanks! Paolo
Hi Paolo, On Tue, Feb 19, 2013 at 7:07 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 19/02/2013 05:02, Peter Crosthwaite ha scritto: >> num_interfaces only tells you how many interfaces the concrete child class has >> (as defined in the TypeInfo). This means if you have a child class which defines >> no interfaces of its own, but its parent has interfaces you cannot cast to those >> parent interfaces. >> >> Fixed changing the guard to check the class->interfaces list instead (which is >> a complete flattened list of implemented interfaces). >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> changed from v1: >> Change guard implementation rather than removed it (Paolo review) >> >> qom/object.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/qom/object.c b/qom/object.c >> index 4b72a64..3d638ff 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -449,7 +449,8 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class, >> TypeImpl *type = class->type; >> ObjectClass *ret = NULL; >> >> - if (type->num_interfaces && type_is_ancestor(target_type, type_interface)) { >> + if (type->class->interfaces && >> + type_is_ancestor(target_type, type_interface)) { >> int found = 0; >> GSList *i; >> >> > > Series looks good, thanks! > Can I take that as an RB or acked-by? :-) Regards, Peter > Paolo >
> >> diff --git a/qom/object.c b/qom/object.c > >> index 4b72a64..3d638ff 100644 > >> --- a/qom/object.c > >> +++ b/qom/object.c > >> @@ -449,7 +449,8 @@ ObjectClass > >> *object_class_dynamic_cast(ObjectClass *class, > >> TypeImpl *type = class->type; > >> ObjectClass *ret = NULL; > >> > >> - if (type->num_interfaces && type_is_ancestor(target_type, > >> type_interface)) { > >> + if (type->class->interfaces && > >> + type_is_ancestor(target_type, type_interface)) { > >> int found = 0; > >> GSList *i; > >> > >> > > > > Series looks good, thanks! > > Can I take that as an RB or acked-by? :-) I try to stay as far as possible from QOM these days, but people don't let me... :) Yes, it is a Reviewed-by. Paolo
Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes: > num_interfaces only tells you how many interfaces the concrete child class has > (as defined in the TypeInfo). This means if you have a child class which defines > no interfaces of its own, but its parent has interfaces you cannot cast to those > parent interfaces. > > Fixed changing the guard to check the class->interfaces list instead (which is > a complete flattened list of implemented interfaces). > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> I must admit I don't understand it yet, but this breaks hot unplug. Here is the qemu-test output: [aliguori@ccnode4 qemu-test]$ QEMU_TEST_SEED=45240 ./qemu-test ~/build/qemu/x86_64-softmmu/qemu-system-x86_64 tests/device-del.sh Using RANDOM seed 45240 Testing nic Formatting '.tmp-31849/disk.img', fmt=qcow2 size=10737418240 encryption=off cluster_size=65536 lazy_refcounts=off /home/aliguori/build/qemu/x86_64-softmmu/qemu-system-x86_64 -kernel /usr/local/share/qemu-jeos/kernel-x86_64-pc -initrd .tmp-31849/initramfs-31849.img.gz -device isa-debug-exit -append console=ttyS0 seed=45240 -nographic -enable-kvm -device virtio-balloon-pci -pidfile .tmp-31849/pidfile-31849.pid -qmp unix:.tmp-31849/qmpsock-31849.sock,server,nowait [ 0.000000] Initializing cgroup subsys cpuset [ 0.000000] Initializing cgroup subsys cpu [ 0.000000] Linux version 3.4.0 (root@ccnode4) (gcc version 4.6.4 20120830 (prerelease) (GCC) ) #2 SMP Mon Dec 3 19:40:41 CST 2012 [ 0.000000] Command line: console=ttyS0 seed=45240 [ 0.000000] BIOS-provided physical RAM map: <- snip -> [ 0.865631] Freeing unused kernel memory: 584k freed [ 0.866320] Write protecting the kernel read-only data: 12288k [ 0.868626] Freeing unused kernel memory: 640k freed [ 0.873366] Freeing unused kernel memory: 1724k freed Setting guest RANDOM seed to 45240 *** Running tests *** /tests/device-del.sh Running test /tests/device-del.sh... ** waiting for hotplug ** ./tests/device-del.sh: line 36: test: !=: unary operator expected ** waiting for remove ** ** waiting for guest to see device ** ./qemu-test: line 99: 31883 Segmentation fault (core dumped) "$@" The backtrace is: Program terminated with signal 11, Segmentation fault. #0 qemu_bh_delete (bh=0x0) at /home/aliguori/git/qemu/async.c:116 116 bh->scheduled = 0; Missing separate debuginfos, use: debuginfo-install bzip2-libs-1.0.6-7.fc18.x86_64 glib2-2.34.2-2.fc18.x86_64 glibc-2.16-28.fc18.x86_64 libaio-0.3.109-6.fc18.x86_64 libgcc-4.7.2-8.fc18.x86_64 pixman-0.26.2-5.fc18.x86_64 xen-libs-4.2.1-5.fc18.x86_64 xz-libs-5.1.2-2alpha.fc18.x86_64 zlib-1.2.7-9.fc18.x86_64 (gdb) bt #0 qemu_bh_delete (bh=0x0) at /home/aliguori/git/qemu/async.c:116 #1 0x00007f55cac9bd95 in virtio_net_exit (vdev=0x7f55cd6eb960) at /home/aliguori/git/qemu/hw/virtio-net.c:1413 #2 0x00007f55cabde7f9 in virtio_net_exit_pci (pci_dev=0x7f55cd668260) at /home/aliguori/git/qemu/hw/virtio-pci.c:1016 #3 0x00007f55cab92e4a in pci_unregister_device (dev=<optimized out>) at /home/aliguori/git/qemu/hw/pci/pci.c:889 #4 0x00007f55caba7485 in device_unparent (obj=<optimized out>) at /home/aliguori/git/qemu/hw/qdev.c:773 #5 0x00007f55cac1ea0f in object_unparent (obj=0x7f55cd668260) at /home/aliguori/git/qemu/qom/object.c:370 #6 0x00007f55caba7a4d in qdev_free (dev=<optimized out>) at /home/aliguori/git/qemu/hw/qdev.c:270 #7 0x00007f55cab38da0 in acpi_piix_eject_slot (s=0x7f55cd68fd50, slots=<optimized out>) at /home/aliguori/git/qemu/hw/acpi_piix4.c:306 #8 0x00007f55caca70f2 in access_with_adjusted_size (addr=addr@entry=8, value=value@entry=0x7f55c8104c28, size=4, access_size_min=<optimized out>, access_size_max=<optimized out>, access=access@entry= 0x7f55caca7710 <memory_region_write_accessor>, opaque=opaque@entry= 0x7f55cd690540) at /home/aliguori/git/qemu/memory.c:364 #9 0x00007f55caca8767 in memory_region_iorange_write ( iorange=<optimized out>, offset=8, width=4, data=32) at /home/aliguori/git/qemu/memory.c:439 It very much looks like a double free to me of the device. The test is doing a simple device_del in the monitor. Regards, Anthony Liguori > --- > changed from v1: > Change guard implementation rather than removed it (Paolo review) > > qom/object.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 4b72a64..3d638ff 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -449,7 +449,8 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class, > TypeImpl *type = class->type; > ObjectClass *ret = NULL; > > - if (type->num_interfaces && type_is_ancestor(target_type, type_interface)) { > + if (type->class->interfaces && > + type_is_ancestor(target_type, type_interface)) { > int found = 0; > GSList *i; > > -- > 1.7.0.4
Anthony Liguori <aliguori@us.ibm.com> writes: > Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes: > >> num_interfaces only tells you how many interfaces the concrete child class has >> (as defined in the TypeInfo). This means if you have a child class which defines >> no interfaces of its own, but its parent has interfaces you cannot cast to those >> parent interfaces. >> >> Fixed changing the guard to check the class->interfaces list instead (which is >> a complete flattened list of implemented interfaces). >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > I must admit I don't understand it yet, but this breaks hot unplug. It was a bad bisect. It doesn't always happen and I wasn't running enough tests to catch it. Am going again now. Stay tuned. Regards, Anthony Liguori > > Here is the qemu-test output: > > [aliguori@ccnode4 qemu-test]$ QEMU_TEST_SEED=45240 ./qemu-test ~/build/qemu/x86_64-softmmu/qemu-system-x86_64 tests/device-del.sh > Using RANDOM seed 45240 > Testing nic > Formatting '.tmp-31849/disk.img', fmt=qcow2 size=10737418240 encryption=off cluster_size=65536 lazy_refcounts=off > /home/aliguori/build/qemu/x86_64-softmmu/qemu-system-x86_64 -kernel /usr/local/share/qemu-jeos/kernel-x86_64-pc -initrd .tmp-31849/initramfs-31849.img.gz -device isa-debug-exit -append console=ttyS0 seed=45240 -nographic -enable-kvm -device virtio-balloon-pci -pidfile .tmp-31849/pidfile-31849.pid -qmp unix:.tmp-31849/qmpsock-31849.sock,server,nowait > [ 0.000000] Initializing cgroup subsys cpuset > [ 0.000000] Initializing cgroup subsys cpu > [ 0.000000] Linux version 3.4.0 (root@ccnode4) (gcc version 4.6.4 20120830 (prerelease) (GCC) ) #2 SMP Mon Dec 3 19:40:41 CST 2012 > [ 0.000000] Command line: console=ttyS0 seed=45240 > [ 0.000000] BIOS-provided physical RAM map: > > <- snip -> > > [ 0.865631] Freeing unused kernel memory: 584k freed > [ 0.866320] Write protecting the kernel read-only data: 12288k > [ 0.868626] Freeing unused kernel memory: 640k freed > [ 0.873366] Freeing unused kernel memory: 1724k freed > Setting guest RANDOM seed to 45240 > *** Running tests *** > /tests/device-del.sh > Running test /tests/device-del.sh... > ** waiting for hotplug ** > ./tests/device-del.sh: line 36: test: !=: unary operator expected > ** waiting for remove ** > ** waiting for guest to see device ** > ./qemu-test: line 99: 31883 Segmentation fault (core dumped) "$@" > > The backtrace is: > > Program terminated with signal 11, Segmentation fault. > #0 qemu_bh_delete (bh=0x0) at /home/aliguori/git/qemu/async.c:116 > 116 bh->scheduled = 0; > Missing separate debuginfos, use: debuginfo-install bzip2-libs-1.0.6-7.fc18.x86_64 glib2-2.34.2-2.fc18.x86_64 glibc-2.16-28.fc18.x86_64 libaio-0.3.109-6.fc18.x86_64 libgcc-4.7.2-8.fc18.x86_64 pixman-0.26.2-5.fc18.x86_64 xen-libs-4.2.1-5.fc18.x86_64 xz-libs-5.1.2-2alpha.fc18.x86_64 zlib-1.2.7-9.fc18.x86_64 > (gdb) bt > #0 qemu_bh_delete (bh=0x0) at /home/aliguori/git/qemu/async.c:116 > #1 0x00007f55cac9bd95 in virtio_net_exit (vdev=0x7f55cd6eb960) > at /home/aliguori/git/qemu/hw/virtio-net.c:1413 > #2 0x00007f55cabde7f9 in virtio_net_exit_pci (pci_dev=0x7f55cd668260) > at /home/aliguori/git/qemu/hw/virtio-pci.c:1016 > #3 0x00007f55cab92e4a in pci_unregister_device (dev=<optimized out>) > at /home/aliguori/git/qemu/hw/pci/pci.c:889 > #4 0x00007f55caba7485 in device_unparent (obj=<optimized out>) > at /home/aliguori/git/qemu/hw/qdev.c:773 > #5 0x00007f55cac1ea0f in object_unparent (obj=0x7f55cd668260) > at /home/aliguori/git/qemu/qom/object.c:370 > #6 0x00007f55caba7a4d in qdev_free (dev=<optimized out>) > at /home/aliguori/git/qemu/hw/qdev.c:270 > #7 0x00007f55cab38da0 in acpi_piix_eject_slot (s=0x7f55cd68fd50, > slots=<optimized out>) at /home/aliguori/git/qemu/hw/acpi_piix4.c:306 > #8 0x00007f55caca70f2 in access_with_adjusted_size (addr=addr@entry=8, > value=value@entry=0x7f55c8104c28, size=4, access_size_min=<optimized out>, > access_size_max=<optimized out>, access=access@entry= > 0x7f55caca7710 <memory_region_write_accessor>, opaque=opaque@entry= > 0x7f55cd690540) at /home/aliguori/git/qemu/memory.c:364 > #9 0x00007f55caca8767 in memory_region_iorange_write ( > iorange=<optimized out>, offset=8, width=4, data=32) > at /home/aliguori/git/qemu/memory.c:439 > > It very much looks like a double free to me of the device. > > The test is doing a simple device_del in the monitor. > > Regards, > > Anthony Liguori > >> --- >> changed from v1: >> Change guard implementation rather than removed it (Paolo review) >> >> qom/object.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/qom/object.c b/qom/object.c >> index 4b72a64..3d638ff 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -449,7 +449,8 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class, >> TypeImpl *type = class->type; >> ObjectClass *ret = NULL; >> >> - if (type->num_interfaces && type_is_ancestor(target_type, type_interface)) { >> + if (type->class->interfaces && >> + type_is_ancestor(target_type, type_interface)) { >> int found = 0; >> GSList *i; >> >> -- >> 1.7.0.4
diff --git a/qom/object.c b/qom/object.c index 4b72a64..3d638ff 100644 --- a/qom/object.c +++ b/qom/object.c @@ -449,7 +449,8 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class, TypeImpl *type = class->type; ObjectClass *ret = NULL; - if (type->num_interfaces && type_is_ancestor(target_type, type_interface)) { + if (type->class->interfaces && + type_is_ancestor(target_type, type_interface)) { int found = 0; GSList *i;
num_interfaces only tells you how many interfaces the concrete child class has (as defined in the TypeInfo). This means if you have a child class which defines no interfaces of its own, but its parent has interfaces you cannot cast to those parent interfaces. Fixed changing the guard to check the class->interfaces list instead (which is a complete flattened list of implemented interfaces). Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- changed from v1: Change guard implementation rather than removed it (Paolo review) qom/object.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)