Re: [Qemu-devel] [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os

[Paolo Bonzini <pbonzini@redhat.com>]

Il 17/07/2014 14:55, arei.gonglei@huawei.com ha scritto:
> From: Gonglei <arei.gonglei@huawei.com>
>
> Assuming that we hotplug three virtio-scsi disk as follow steps:
> 1. start vm with virtio-scsi as system disk (guest os: suse11 sp3 ).
> 2. hotplug disk 1 (as lun2)
>  -drive file=/Images/TestImg/kvm-disk-scsi_001,if=none,id=drive-scsi0-0-0-2,format=raw, \
>  cache=none,aio=native -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=2, \
>  drive=drive-scsi0-0-0-2,id=scsi0-0-0-2
> 2. hotplug disk 2 (as lun3)
>  -drive file=/Images/TestImg/kvm-disk-scsi_002,if=none,id=drive-scsi0-0-0-3,format=raw,\
>  cache=none,aio=native -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=3,\
>  drive=drive-scsi0-0-0-3,id=scsi0-0-0-3
> 3. hotplug disk 3 (as lun4)
>  -drive file=/Images/TestImg/kvm-disk-scsi_003,if=none,id=drive-scsi0-0-0-4,format=raw,\
>  cache=none,aio=native -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=4,\
>  drive=drive-scsi0-0-0-4,id=scsi0-0-0-4
>
> We can see lun2 as sdb, lun3 as sdc, lun4 as sdd in the guest os.
> But after rebootint the guest, the scsi disk symbol will changed,
> lun2 change to sdd, lun3 as sdc, lun4 as sdb.
>
> Lun2 -> sdb   reboot    lun2 -> sdd
> Lun3 -> sdc   ------>   lun3 -> sdc
> Lun4 -> sdd             lun4 -> sdb
>
> In Linux os, the scsi_scan_host() will scan scsi host adapter's lun, firstly scan lun 0
> and add into system, secondly send REPORT_LUNS command to qurey the other luns.
>
> In QEMU, the scsi_target_emulate_report_luns() emulate the REPORT_LUNS command.
> The function will scan virtio-scsi-bus's children and report to guest os finally.
>  QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
>
> The step of attaching device in QEMU:
>  qdev_device_add
>    qdev_set_parent_bus
>       bus_add_child
>           QTAILQ_INSERT_HEAD(&bus->children, kid, sibling); // insert list head
>
> The latest hotplugged disk is at the head of bus->children list.
>
> Finally those cause the disk symbol confusion in the guest os.
>
> Fix the issue by QTAILQ_FOREACH_REVERSE replace QTAILQ_FOREACH in
> scsi_target_emulate_report_luns(), which follow the FIFO principle for
> scsi disks hotplugging.
>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/scsi/scsi-bus.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 4341754..b6671ea 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -371,7 +371,8 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
>      memset(r->buf, 0, len);
>      stl_be_p(&r->buf[0], n);
>      i = found_lun0 ? 8 : 16;
> -    QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
> +    QTAILQ_FOREACH_REVERSE(kid, &r->req.bus->qbus.children,
> +                                   ChildrenHead, sibling) {
>          DeviceState *qdev = kid->child;
>          SCSIDevice *dev = SCSI_DEVICE(qdev);
>
>

This is a change to the guest ABI, so you would have to make it only for 
the latest machine type; and this unfortunately disqualifies it already 
from 2.1 since it's very late.

Does this happen for cold-plugged disks too?  If so, I wonder if this 
would cause more pain than it fixes, because it could break machines 
when you upgrade QEMU.  In the end, /dev/sd* is not stable and people 
should use /dev/disk/* and/or partition labels instead.

Paolo