qdev: modify func qdev_build_hotpluggable_device_list

PATCH In review Jun Li 3 participants 5 messages Nov 4, 2014 – Nov 5, 2014

Share & tools

Jun Li

author

2014-11-04 06:56 UTC #0 lore raw

PATCH v2 1/1 qdev: modify func qdev_build_hotpluggable_device_list

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 <[email protected]>

Reviewed-by Paolo Bonzini <[email protected]>

---
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(-)

hw/core/qdev.c +9 -4

index b3d5196..2282e95 100644
@@ -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;
 }

Signature · 1 line

1.9.3

Andreas Färber

2014-11-04 18:39 UTC #1 lore raw

Hi,

Am 04.11.2014 um 07:55 schrieb Jun Li:

Quoted 27 lines

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 <[email protected]>
Reviewed-by Paolo Bonzini <[email protected]>
---
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

Quoted 7 lines

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

Signature · 2 lines

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

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

Quoted 58 lines · 2014-11-5 上午2:39于 "Andreas Färber" <[email protected]>写道：

Hi,
Am 04.11.2014 um 07:55 schrieb Jun Li:
Quoted 1 line
Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch
just
Quoted 1 line
fixed it. When *obj is not a TYPE_DEVICE, just do not add it to
hotpluggable
Quoted 8 lines
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
Quoted 17 lines
"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 <[email protected]>
Reviewed-by Paolo Bonzini <[email protected]>
---
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
Quoted 7 lines
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

Fam Zheng

2014-11-05 01:59 UTC #3 lore raw

Quoted 5 lines · jun muzi

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

Quoted 62 lines

Jun Li
Quoted 58 lines · 2014-11-5 上午2:39于 "Andreas Färber" <[email protected]>写道：
Hi,
Am 04.11.2014 um 07:55 schrieb Jun Li:
Quoted 1 line
Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch
just
Quoted 1 line
fixed it. When *obj is not a TYPE_DEVICE, just do not add it to
hotpluggable
Quoted 8 lines
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
Quoted 17 lines
"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 <[email protected]>
Reviewed-by Paolo Bonzini <[email protected]>
---
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
Quoted 7 lines
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

Jun Li

author

2014-11-05 06:48 UTC #4 lore raw

Quoted 8 lines · Fam Zheng

Quoted 5 lines · jun muzi
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.

Quoted 65 lines

Fam
Quoted 62 lines
Jun Li
Quoted 58 lines · 2014-11-5 上午2:39于 "Andreas Färber" <[email protected]>写道：
Hi,
Am 04.11.2014 um 07:55 schrieb Jun Li:
Quoted 1 line
Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch
just
Quoted 1 line
fixed it. When *obj is not a TYPE_DEVICE, just do not add it to
hotpluggable
Quoted 8 lines
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
Quoted 17 lines
"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 <[email protected]>
Reviewed-by Paolo Bonzini <[email protected]>
---
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
Quoted 7 lines
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