Re: [Qemu-devel] Commit 622b520f changed -drive if=scsi, index=N, intentional?

[Daniel P. Berrange]

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