Patchwork Commit 622b520f changed -drive if=scsi, index=N, intentional?

login
register
mail settings
Submitter Markus Armbruster
Date Jan. 27, 2011, 12:10 p.m.
Message ID <m3oc72eed1.fsf@blackfin.pond.sub.org>
Download mbox | patch
Permalink /patch/80656/
State New
Headers show

Comments

Markus Armbruster - Jan. 27, 2011, 12:10 p.m.
Consider -drive if=scsi,index=12,...

Before the commit, index=12 meant bus=1,unit=5.  Example:

    $ qemu-system-x86_64 -nodefaults -vnc :0 -S -monitor stdio -drive if=scsi,index=12,media=cdrom
    QEMU 0.13.50 monitor - type 'help' for more information
    (qemu) info block
    scsi1-cd5: type=cdrom removable=1 locked=0 [not inserted]
    (qemu) info qtree
    [...]
        bus: pci.0
          type PCI
          dev: lsi53c895a, id ""
            bus-prop: addr = 03.0
            bus-prop: romfile = <null>
            bus-prop: rombar = 1
            bus-prop: multifunction = off
            class SCSI controller, addr 00:03.0, pci id 1000:0012 (sub 1af4:1000)
            bar 0: i/o at 0xffffffffffffffff [0xfe]
            bar 1: mem at 0xffffffffffffffff [0x3fe]
            bar 2: mem at 0xffffffffffffffff [0x1ffe]
            bus: scsi.0
              type SCSI
              dev: scsi-disk, id ""
                dev-prop: drive = scsi1-cd5
                dev-prop: logical_block_size = 512
                dev-prop: physical_block_size = 512
                dev-prop: min_io_size = 0
                dev-prop: opt_io_size = 0
                dev-prop: ver = "0.13.50"
                dev-prop: serial = "0"
                bus-prop: scsi-id = 5
          dev: lsi53c895a, id ""
            bus-prop: addr = 02.0
            bus-prop: romfile = <null>
            bus-prop: rombar = 1
            bus-prop: multifunction = off
            class SCSI controller, addr 00:02.0, pci id 1000:0012 (sub 1af4:1000)
            bar 0: i/o at 0xffffffffffffffff [0xfe]
            bar 1: mem at 0xffffffffffffffff [0x3fe]
            bar 2: mem at 0xffffffffffffffff [0x1ffe]
            bus: scsi.0
              type SCSI
    [...]

    Two scsi-buses, and scsi1-cd5 with scsi-id 5 is on the second one,
    i.e. bus=1, unit=5.

After the commit, it means bus=0,unit=12.  The drive is created, but not
the guest device.  That's because lsi53c895a supports only 7 units
(LSI_MAX_DEVS), and scsi_bus_legacy_handle_cmdline() ignores drives with
unit numbers exceeding that limit.  Example:

    $ ~/work/qemu/bld-x86/x86_64-softmmu/qemu-system-x86_64 -nodefaults -usb -m 384 -vnc :0 -enable-kvm -S -readconfig ~/work/qemu/test.cfg -drive if=scsi,index=12,media=cdrom
    QEMU 0.13.50 monitor - type 'help' for more information
    (qemu) info block
    hda: type=hd removable=0 file=/home/armbru/work/images/test.img ro=0 drv=raw encrypted=0
    scsi0-cd12: type=cdrom removable=1 locked=0 [not inserted]
    (qemu) info qtree
    [...]
        bus: pci.0
          type PCI
    [...]
          dev: lsi53c895a, id ""
            bus-prop: addr = 02.0
            bus-prop: romfile = <null>
            bus-prop: rombar = 1
            bus-prop: multifunction = off
            bus-prop: command_serr_enable = on
            class SCSI controller, addr 00:02.0, pci id 1000:0012 (sub 1af4:1000)
            bar 0: i/o at 0xffffffffffffffff [0xfe]
            bar 1: mem at 0xffffffffffffffff [0x3fe]
            bar 2: mem at 0xffffffffffffffff [0x1ffe]
            bus: scsi.0
              type SCSI
    [...]

    One scsi-bus, and scsi1-cd5 nowhere to be found.

I'd call this a regression.

What now?



commit 622b520fb4ca50b5028485f1d225317ece0a42b9
Author: Hannes Reinecke <hare@suse.de>
Date:   Wed Nov 24 12:15:56 2010 +0100

    scsi: Increase the number of possible devices
    
    The SCSI parallel interface has a limit of 8 devices, but
    not the SCSI stack in general. So we should be removing the
    hard-coded limit and use MAX_SCSI_DEVS instead.
    And we only need to scan those devices which are allocated
    by the bus.
    
    Signed-off-by: Hannes Reinecke <hare@suse.de>
    Acked-by: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Daniel P. Berrange - Jan. 27, 2011, 12:35 p.m.
On Thu, Jan 27, 2011 at 01:10:50PM +0100, Markus Armbruster wrote:
> Consider -drive if=scsi,index=12,...
> 
> Before the commit, index=12 meant bus=1,unit=5.  Example:
> 
>     $ qemu-system-x86_64 -nodefaults -vnc :0 -S -monitor stdio -drive if=scsi,index=12,media=cdrom
>     QEMU 0.13.50 monitor - type 'help' for more information
>     (qemu) info block
>     scsi1-cd5: type=cdrom removable=1 locked=0 [not inserted]
>     (qemu) info qtree
>     [...]
>         bus: pci.0
>           type PCI
>           dev: lsi53c895a, id ""
>             bus-prop: addr = 03.0
>             bus-prop: romfile = <null>
>             bus-prop: rombar = 1
>             bus-prop: multifunction = off
>             class SCSI controller, addr 00:03.0, pci id 1000:0012 (sub 1af4:1000)
>             bar 0: i/o at 0xffffffffffffffff [0xfe]
>             bar 1: mem at 0xffffffffffffffff [0x3fe]
>             bar 2: mem at 0xffffffffffffffff [0x1ffe]
>             bus: scsi.0
>               type SCSI
>               dev: scsi-disk, id ""
>                 dev-prop: drive = scsi1-cd5
>                 dev-prop: logical_block_size = 512
>                 dev-prop: physical_block_size = 512
>                 dev-prop: min_io_size = 0
>                 dev-prop: opt_io_size = 0
>                 dev-prop: ver = "0.13.50"
>                 dev-prop: serial = "0"
>                 bus-prop: scsi-id = 5
>           dev: lsi53c895a, id ""
>             bus-prop: addr = 02.0
>             bus-prop: romfile = <null>
>             bus-prop: rombar = 1
>             bus-prop: multifunction = off
>             class SCSI controller, addr 00:02.0, pci id 1000:0012 (sub 1af4:1000)
>             bar 0: i/o at 0xffffffffffffffff [0xfe]
>             bar 1: mem at 0xffffffffffffffff [0x3fe]
>             bar 2: mem at 0xffffffffffffffff [0x1ffe]
>             bus: scsi.0
>               type SCSI
>     [...]
> 
>     Two scsi-buses, and scsi1-cd5 with scsi-id 5 is on the second one,
>     i.e. bus=1, unit=5.
> 
> After the commit, it means bus=0,unit=12.  The drive is created, but not
> the guest device.  That's because lsi53c895a supports only 7 units
> (LSI_MAX_DEVS), and scsi_bus_legacy_handle_cmdline() ignores drives with
> unit numbers exceeding that limit.  Example:
> 
>     $ ~/work/qemu/bld-x86/x86_64-softmmu/qemu-system-x86_64 -nodefaults -usb -m 384 -vnc :0 -enable-kvm -S -readconfig ~/work/qemu/test.cfg -drive if=scsi,index=12,media=cdrom
>     QEMU 0.13.50 monitor - type 'help' for more information
>     (qemu) info block
>     hda: type=hd removable=0 file=/home/armbru/work/images/test.img ro=0 drv=raw encrypted=0
>     scsi0-cd12: type=cdrom removable=1 locked=0 [not inserted]
>     (qemu) info qtree
>     [...]
>         bus: pci.0
>           type PCI
>     [...]
>           dev: lsi53c895a, id ""
>             bus-prop: addr = 02.0
>             bus-prop: romfile = <null>
>             bus-prop: rombar = 1
>             bus-prop: multifunction = off
>             bus-prop: command_serr_enable = on
>             class SCSI controller, addr 00:02.0, pci id 1000:0012 (sub 1af4:1000)
>             bar 0: i/o at 0xffffffffffffffff [0xfe]
>             bar 1: mem at 0xffffffffffffffff [0x3fe]
>             bar 2: mem at 0xffffffffffffffff [0x1ffe]
>             bus: scsi.0
>               type SCSI
>     [...]
> 
>     One scsi-bus, and scsi1-cd5 nowhere to be found.
> 
> I'd call this a regression.
> 
> What now?

Not answering your question, but it occurs to me that you've
demonstrated a easy way to provide some unit tests for QEMU
to validate that we're not changing the device model in
ways like this that could upset guests. eg in the source
tree have a set of files each containing a list of command
line arguments, and a corresponding set of files containing
the 'info qdev' output. The unit tests run the command lines
and compare the 'info qdev' output against what's recorded
in the expect data files and complain if they differ.

Daniel

Patch

diff --git a/blockdev.h b/blockdev.h
index 2a0559e..4cb8ca9 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -32,7 +32,7 @@  struct DriveInfo {
 };
 
 #define MAX_IDE_DEVS	2
-#define MAX_SCSI_DEVS	7
+#define MAX_SCSI_DEVS	255
 
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
 int drive_get_max_bus(BlockInterfaceType type);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 5a3fd4b..74a08b7 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -108,7 +108,7 @@  int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
     int res = 0, unit;
 
     loc_push_none(&loc);
-    for (unit = 0; unit < MAX_SCSI_DEVS; unit++) {
+    for (unit = 0; unit < bus->ndev; unit++) {
         dinfo = drive_get(IF_SCSI, bus->busnr, unit);
         if (dinfo == NULL) {
             continue;
diff --git a/hw/scsi.h b/hw/scsi.h
index cb06d6d..9c798ae 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,6 +3,7 @@ 
 
 #include "qdev.h"
 #include "block.h"
+#include "blockdev.h"
 #include "block_int.h"
 
 #define SCSI_CMD_BUF_SIZE     16
@@ -86,7 +87,7 @@  struct SCSIBus {
     int tcq, ndev;
     scsi_completionfn complete;
 
-    SCSIDevice *devs[8];
+    SCSIDevice *devs[MAX_SCSI_DEVS];
 };
 
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,