[Qemu-devel] [PATCH v2] qdev: modify func qdev_build_hotpluggable_device_list

[Qemu-devel] [PATCH v2] qdev: modify func qdev_build_hotpluggable_device_list

[Jun Li]

Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch just
fixed it. When *obj is not a TYPE_DEVICE, just do not add it to hotpluggable
device list.

This patch also fixed the following issue:
1, boot qemu using cli:
$ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
-device virtio-scsi-pci,id=scsi0

2, device_del scsi0 via hmp using tab key(first input device_del, then press
"Tab" key).
(qemu) device_del

After step2, qemu will abort.
(qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list:
Object 0x5555563a2460 is not an instance of type device

Signed-off-by: Jun Li <junmuzi@gmail.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
v2:
  This version just do a little changes for the commit message.
As following show:
In v1,
1, boot qemu using cli:
virtio-scsi-pci,id=scsi0 -enable-kvm

In v2,
1, boot qemu using cli:
$ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
-device virtio-scsi-pci,id=scsi0
---
 hw/core/qdev.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b3d5196..2282e95 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -927,13 +927,18 @@ void qdev_alias_all_properties(DeviceState *target, Object *source)
 int qdev_build_hotpluggable_device_list(Object *obj, void *opaque)
 {
     GSList **list = opaque;
-    DeviceState *dev = DEVICE(obj);
+    DeviceState *dev = (DeviceState *)object_dynamic_cast(OBJECT(obj),
+                                                          "device");
+
+    if (dev) {
+        if (dev->realized &&
+            object_property_get_bool(obj, "hotpluggable", NULL)) {
+            *list = g_slist_append(*list, dev);
+        }
 
-    if (dev->realized && object_property_get_bool(obj, "hotpluggable", NULL)) {
-        *list = g_slist_append(*list, dev);
+        object_child_foreach(obj, qdev_build_hotpluggable_device_list, opaque);
     }
 
-    object_child_foreach(obj, qdev_build_hotpluggable_device_list, opaque);
     return 0;
 }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2] qdev: modify func qdev_build_hotpluggable_device_list

[Andreas Färber]

Hi,

Am 04.11.2014 um 07:55 schrieb Jun Li:
> Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch just
> fixed it. When *obj is not a TYPE_DEVICE, just do not add it to hotpluggable
> device list.
> 
> This patch also fixed the following issue:
> 1, boot qemu using cli:
> $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> -device virtio-scsi-pci,id=scsi0
> 
> 2, device_del scsi0 via hmp using tab key(first input device_del, then press
> "Tab" key).
> (qemu) device_del
> 
> After step2, qemu will abort.
> (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list:
> Object 0x5555563a2460 is not an instance of type device
> 
> Signed-off-by: Jun Li <junmuzi@gmail.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> v2:
>   This version just do a little changes for the commit message.
> As following show:
> In v1,
> 1, boot qemu using cli:
> virtio-scsi-pci,id=scsi0 -enable-kvm
> 
> In v2,
> 1, boot qemu using cli:
> $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> -device virtio-scsi-pci,id=scsi0
> ---
>  hw/core/qdev.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

Thanks, I've queued this patch, but we should give it a more meaningful
subject - maybe "qdev: Avoid type assertion in qdev_build_...()"?

Also, we could avoid reindentation by returning early:
if (dev == NULL) {
    return 0;
}

What do you think?

Regards,
Andreas

-- 
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg

Re: [Qemu-devel] [PATCH v2] qdev: modify func qdev_build_hotpluggable_device_list

[jun muzi]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

Yes, in my original thought i just want to do as you said. But it will have
two "return 0" in one function. So i think it's not so smart. If you still
think two "return 0" is better, i will submit a new version. Thanks.

BTW, for subject, i agree with you.

Jun Li
2014-11-5 上午2:39于 "Andreas Färber" <afaerber@suse.de>写道：

> Hi,
>
> Am 04.11.2014 um 07:55 schrieb Jun Li:
> > Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch
> just
> > fixed it. When *obj is not a TYPE_DEVICE, just do not add it to
> hotpluggable
> > device list.
> >
> > This patch also fixed the following issue:
> > 1, boot qemu using cli:
> > $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> > -device virtio-scsi-pci,id=scsi0
> >
> > 2, device_del scsi0 via hmp using tab key(first input device_del, then
> press
> > "Tab" key).
> > (qemu) device_del
> >
> > After step2, qemu will abort.
> > (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list:
> > Object 0x5555563a2460 is not an instance of type device
> >
> > Signed-off-by: Jun Li <junmuzi@gmail.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > v2:
> >   This version just do a little changes for the commit message.
> > As following show:
> > In v1,
> > 1, boot qemu using cli:
> > virtio-scsi-pci,id=scsi0 -enable-kvm
> >
> > In v2,
> > 1, boot qemu using cli:
> > $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> > -device virtio-scsi-pci,id=scsi0
> > ---
> >  hw/core/qdev.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
>
> Thanks, I've queued this patch, but we should give it a more meaningful
> subject - maybe "qdev: Avoid type assertion in qdev_build_...()"?
>
> Also, we could avoid reindentation by returning early:
> if (dev == NULL) {
>     return 0;
> }
>
> What do you think?
>
> Regards,
> Andreas
>
> --
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
>

[-- Attachment #2: Type: text/html, Size: 2722 bytes --]

Re: [Qemu-devel] [PATCH v2] qdev: modify func qdev_build_hotpluggable_device_list

[Fam Zheng]

On Wed, 11/05 08:53, jun muzi wrote:
> Yes, in my original thought i just want to do as you said. But it will have
> two "return 0" in one function. So i think it's not so smart. If you still
> think two "return 0" is better, i will submit a new version. Thanks.
> 
> BTW, for subject, i agree with you.

Please use inline reply for mailing list discussions.

Fam

> 
> Jun Li
> 2014-11-5 上午2:39于 "Andreas Färber" <afaerber@suse.de>写道：
> 
> > Hi,
> >
> > Am 04.11.2014 um 07:55 schrieb Jun Li:
> > > Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch
> > just
> > > fixed it. When *obj is not a TYPE_DEVICE, just do not add it to
> > hotpluggable
> > > device list.
> > >
> > > This patch also fixed the following issue:
> > > 1, boot qemu using cli:
> > > $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> > > -device virtio-scsi-pci,id=scsi0
> > >
> > > 2, device_del scsi0 via hmp using tab key(first input device_del, then
> > press
> > > "Tab" key).
> > > (qemu) device_del
> > >
> > > After step2, qemu will abort.
> > > (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list:
> > > Object 0x5555563a2460 is not an instance of type device
> > >
> > > Signed-off-by: Jun Li <junmuzi@gmail.com>
> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > > v2:
> > >   This version just do a little changes for the commit message.
> > > As following show:
> > > In v1,
> > > 1, boot qemu using cli:
> > > virtio-scsi-pci,id=scsi0 -enable-kvm
> > >
> > > In v2,
> > > 1, boot qemu using cli:
> > > $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> > > -device virtio-scsi-pci,id=scsi0
> > > ---
> > >  hw/core/qdev.c | 13 +++++++++----
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > Thanks, I've queued this patch, but we should give it a more meaningful
> > subject - maybe "qdev: Avoid type assertion in qdev_build_...()"?
> >
> > Also, we could avoid reindentation by returning early:
> > if (dev == NULL) {
> >     return 0;
> > }
> >
> > What do you think?
> >
> > Regards,
> > Andreas
> >
> > --
> > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
> >

Re: [Qemu-devel] [PATCH v2] qdev: modify func qdev_build_hotpluggable_device_list

[Jun Li]

On Wed, 11/05 09:59, Fam Zheng wrote:
> On Wed, 11/05 08:53, jun muzi wrote:
> > Yes, in my original thought i just want to do as you said. But it will have
> > two "return 0" in one function. So i think it's not so smart. If you still
> > think two "return 0" is better, i will submit a new version. Thanks.
> > 
> > BTW, for subject, i agree with you.
> 
> Please use inline reply for mailing list discussions.

Ok, got it. Thx.

> 
> Fam
> 
> > 
> > Jun Li
> > 2014-11-5 上午2:39于 "Andreas Färber" <afaerber@suse.de>写道：
> > 
> > > Hi,
> > >
> > > Am 04.11.2014 um 07:55 schrieb Jun Li:
> > > > Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch
> > > just
> > > > fixed it. When *obj is not a TYPE_DEVICE, just do not add it to
> > > hotpluggable
> > > > device list.
> > > >
> > > > This patch also fixed the following issue:
> > > > 1, boot qemu using cli:
> > > > $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> > > > -device virtio-scsi-pci,id=scsi0
> > > >
> > > > 2, device_del scsi0 via hmp using tab key(first input device_del, then
> > > press
> > > > "Tab" key).
> > > > (qemu) device_del
> > > >
> > > > After step2, qemu will abort.
> > > > (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list:
> > > > Object 0x5555563a2460 is not an instance of type device
> > > >
> > > > Signed-off-by: Jun Li <junmuzi@gmail.com>
> > > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > ---
> > > > v2:
> > > >   This version just do a little changes for the commit message.
> > > > As following show:
> > > > In v1,
> > > > 1, boot qemu using cli:
> > > > virtio-scsi-pci,id=scsi0 -enable-kvm
> > > >
> > > > In v2,
> > > > 1, boot qemu using cli:
> > > > $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
> > > > -device virtio-scsi-pci,id=scsi0
> > > > ---
> > > >  hw/core/qdev.c | 13 +++++++++----
> > > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > Thanks, I've queued this patch, but we should give it a more meaningful
> > > subject - maybe "qdev: Avoid type assertion in qdev_build_...()"?
> > >
> > > Also, we could avoid reindentation by returning early:
> > > if (dev == NULL) {
> > >     return 0;
> > > }
> > >
> > > What do you think?
> > >
> > > Regards,
> > > Andreas
> > >
> > > --
> > > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
> > >