diff mbox

scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial

Message ID 1377273718-26937-1-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Aug. 23, 2013, 4:01 p.m. UTC
From: Markus Armbruster <armbru@redhat.com>

scsi_bus_legacy_add_drive() creates either a scsi-disk or a
scsi-generic device.  It sets property "serial" to argument serial
unless null.  Crashes with scsi-generic, because it doesn't have such
the property.

Only usb_msd_initfn_storage() passes non-null serial.  Reproducer:

    $ qemu-system-x86_64 -nodefaults -display none -S -usb \
    -drive if=none,file=/dev/sg1,id=usb-drv0 \
    -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123
    qemu-system-x86_64: -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123: Property '.serial' not found
    Aborted (core dumped)

Fix by handling exactly like "removable": set the property only when
it exists.

Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi/scsi-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andreas Färber Aug. 23, 2013, 4:08 p.m. UTC | #1
Am 23.08.2013 18:01, schrieb armbru@redhat.com:
> From: Markus Armbruster <armbru@redhat.com>
> 
> scsi_bus_legacy_add_drive() creates either a scsi-disk or a
> scsi-generic device.  It sets property "serial" to argument serial
> unless null.  Crashes with scsi-generic, because it doesn't have such
> the property.
> 
> Only usb_msd_initfn_storage() passes non-null serial.  Reproducer:
> 
>     $ qemu-system-x86_64 -nodefaults -display none -S -usb \
>     -drive if=none,file=/dev/sg1,id=usb-drv0 \
>     -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123
>     qemu-system-x86_64: -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123: Property '.serial' not found
>     Aborted (core dumped)
> 
> Fix by handling exactly like "removable": set the property only when
> it exists.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/scsi/scsi-bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas
Paolo Bonzini Aug. 26, 2013, 7:48 a.m. UTC | #2
Il 23/08/2013 18:01, armbru@redhat.com ha scritto:
> From: Markus Armbruster <armbru@redhat.com>
> 
> scsi_bus_legacy_add_drive() creates either a scsi-disk or a
> scsi-generic device.  It sets property "serial" to argument serial
> unless null.  Crashes with scsi-generic, because it doesn't have such
> the property.
> 
> Only usb_msd_initfn_storage() passes non-null serial.  Reproducer:
> 
>     $ qemu-system-x86_64 -nodefaults -display none -S -usb \
>     -drive if=none,file=/dev/sg1,id=usb-drv0 \
>     -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123
>     qemu-system-x86_64: -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123: Property '.serial' not found
>     Aborted (core dumped)
> 
> Fix by handling exactly like "removable": set the property only when
> it exists.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/scsi/scsi-bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index fbf9173..8fe4f4c 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -224,7 +224,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
>      if (object_property_find(OBJECT(dev), "removable", NULL)) {
>          qdev_prop_set_bit(dev, "removable", removable);
>      }
> -    if (serial) {
> +    if (serial && object_property_find(OBJECT(dev), "serial", NULL)) {
>          qdev_prop_set_string(dev, "serial", serial);
>      }
>      if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
> 

Applied to scsi-next branch, thanks.

Paolo
diff mbox

Patch

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index fbf9173..8fe4f4c 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -224,7 +224,7 @@  SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
     if (object_property_find(OBJECT(dev), "removable", NULL)) {
         qdev_prop_set_bit(dev, "removable", removable);
     }
-    if (serial) {
+    if (serial && object_property_find(OBJECT(dev), "serial", NULL)) {
         qdev_prop_set_string(dev, "serial", serial);
     }
     if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {