From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=33563 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PiR56-0002NB-67
	for qemu-devel@nongnu.org; Thu, 27 Jan 2011 07:36:09 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <berrange@redhat.com>) id 1PiR55-0000nU-0F
	for qemu-devel@nongnu.org; Thu, 27 Jan 2011 07:36:08 -0500
Received: from mx1.redhat.com ([209.132.183.28]:10326)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <berrange@redhat.com>) id 1PiR54-0000nH-Of
	for qemu-devel@nongnu.org; Thu, 27 Jan 2011 07:36:06 -0500
Date: Thu, 27 Jan 2011 12:35:53 +0000
From: "Daniel P. Berrange" <berrange@redhat.com>
Subject: Re: [Qemu-devel] Commit 622b520f changed -drive if=scsi, index=N,
	intentional?
Message-ID: <20110127123553.GA11333@redhat.com>
References: <m3oc72eed1.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <m3oc72eed1.fsf@blackfin.pond.sub.org>
Reply-To: "Daniel P. Berrange" <berrange@redhat.com>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Christoph Hellwig <hch@lst.de>, qemu-devel@nongnu.org, Hannes Reinecke <hare@suse.de>

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