| Message ID | m3oc72eed1.fsf@blackfin.pond.sub.org |
|---|---|
| State | New |
| Headers | show |
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
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,
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>