* [Qemu-devel] [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os
@ 2014-07-17 12:55 arei.gonglei
2014-07-17 13:45 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: arei.gonglei @ 2014-07-17 12:55 UTC (permalink / raw)
To: qemu-devel; +Cc: luonengjun, pbonzini, Gonglei, weidong.huang
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);
--
1.7.12.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os
2014-07-17 12:55 [Qemu-devel] [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os arei.gonglei
@ 2014-07-17 13:45 ` Paolo Bonzini
2014-07-18 1:32 ` Gonglei (Arei)
2014-07-18 2:01 ` Gonglei (Arei)
0 siblings, 2 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-07-17 13:45 UTC (permalink / raw)
To: arei.gonglei, qemu-devel; +Cc: luonengjun, weidong.huang
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os
2014-07-17 13:45 ` Paolo Bonzini
@ 2014-07-18 1:32 ` Gonglei (Arei)
2014-07-18 2:01 ` Gonglei (Arei)
1 sibling, 0 replies; 4+ messages in thread
From: Gonglei (Arei) @ 2014-07-18 1:32 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Luonengjun, Huangweidong (C)
Hi,
> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, July 17, 2014 9:45 PM
> To: Gonglei (Arei); qemu-devel@nongnu.org
> Cc: Huangweidong (C); Luonengjun
> Subject: Re: [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os
>
> 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=r
> aw, \
> > 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=r
> aw,\
> > 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=r
> aw,\
> > 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;
Would you give me more details? Thanks.
> and this unfortunately disqualifies it already
> from 2.1 since it's very late.
>
OK.
> 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.
No, it doesn't happen for cold-plugged disks.
> In the end, /dev/sd* is not stable and people
> should use /dev/disk/* and/or partition labels instead.
>
Agree.
But for general users, the /dev/sd* usually more commonly used. They
can be used easily through easy mount command "/dev/sd* /mnt/sd* ".
So, this changes make sense for people IMHO.
Best regards,
-Gonglei
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os
2014-07-17 13:45 ` Paolo Bonzini
2014-07-18 1:32 ` Gonglei (Arei)
@ 2014-07-18 2:01 ` Gonglei (Arei)
1 sibling, 0 replies; 4+ messages in thread
From: Gonglei (Arei) @ 2014-07-18 2:01 UTC (permalink / raw)
To: Gonglei (Arei), Paolo Bonzini, qemu-devel; +Cc: Luonengjun, Huangweidong (C)
> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Friday, July 18, 2014 9:32 AM
> To: 'Paolo Bonzini'; qemu-devel@nongnu.org
> Cc: Huangweidong (C); Luonengjun
> Subject: RE: [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os
>
> Hi,
>
> > -----Original Message-----
> > From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> > Sent: Thursday, July 17, 2014 9:45 PM
> > To: Gonglei (Arei); qemu-devel@nongnu.org
> > Cc: Huangweidong (C); Luonengjun
> > Subject: Re: [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os
> >
> > 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=r
> > aw, \
> > > 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=r
> > aw,\
> > > 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=r
> > aw,\
> > > 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;
>
> Would you give me more details? Thanks.
>
> > and this unfortunately disqualifies it already
> > from 2.1 since it's very late.
> >
> OK.
>
> > 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.
>
> No, it doesn't happen for cold-plugged disks.
>
Sorry, I may misunderstand your mean. This change will happen for
cold-plugged disks too when using the QEMU before committing this patch.
Best regards,
-Gonglei
> > In the end, /dev/sd* is not stable and people
> > should use /dev/disk/* and/or partition labels instead.
> >
> Agree.
>
> But for general users, the /dev/sd* usually more commonly used. They
> can be used easily through easy mount command "/dev/sd* /mnt/sd* ".
> So, this changes make sense for people IMHO.
>
> Best regards,
> -Gonglei
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-18 2:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-17 12:55 [Qemu-devel] [PATCH for-2.1] scsi: fix scsi disk symbol confusion in guest os arei.gonglei
2014-07-17 13:45 ` Paolo Bonzini
2014-07-18 1:32 ` Gonglei (Arei)
2014-07-18 2:01 ` Gonglei (Arei)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.