blockdev: reset werror/rerror on drive_del
PATCH 1/1 blockdev: reset werror/rerror on drive_del
Paolo Bonzini <[email protected]> suggested the following test case:
1. Launch a guest and wait at the GRUB boot menu:
qemu-system-x86_64 -enable-kvm -m 1024 \ -drive if=none,cache=none,file=test.img,id=foo,werror=stop,rerror=stop -device virtio-blk-pci,drive=foo,id=virtio0,addr=4
2. Hot unplug the device:
(qemu) drive_del foo
3. Select the first boot menu entry
Without this patch the guest pauses due to ENOMEDIUM. But it is not possible to resolve this situation - the drive has become anonymous.
With this patch the guest the guest gets the ENOMEDIUM error.
Note that this scenario actually happens sometimes during libvirt disk hot unplug, where device_del is followed by drive_del. I/O may still be submitted to the drive after drive_del if the guest does not process the PCI hot unplug notification.
--- blockdev.c | 4 ++++ 1 file changed, 4 insertions(+)
blockdev.c +4 -0
@@ -1180,6 +1180,10 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)*/ if (bdrv_get_attached_dev (bs)) {bdrv_make_anon (bs); + +/* Further I/O must not pause the guest */ +bdrv_set_on_error (bs,BLOCKDEV_ON_ERROR_REPORT , +BLOCKDEV_ON_ERROR_REPORT ); }else {drive_uninit (drive_get_by_blockdev (bs)); }
Signature · 1 line
1.8.1.4
Il 03/06/2013 16:58, Stefan Hajnoczi ha scritto:
Quoted 36 lines
Paolo Bonzini <[email protected]> suggested the following test case:
1. Launch a guest and wait at the GRUB boot menu:
qemu-system-x86_64 -enable-kvm -m 1024 \ -drive if=none,cache=none,file=test.img,id=foo,werror=stop,rerror=stop -device virtio-blk-pci,drive=foo,id=virtio0,addr=42. Hot unplug the device:
(qemu)
drive_delfoo3. Select the first boot menu entry
Without this patch the guest pauses due to ENOMEDIUM. But it is not possible to resolve this situation - the drive has become anonymous.
With this patch the guest the guest gets the ENOMEDIUM error.
Note that this scenario actually happens sometimes during libvirt disk hot unplug, where
device_delis followed bydrive_del. I/O may still be submitted to the drive afterdrive_delif the guest does not process the PCI hot unplug notification.Reported-by Dafna Ron <[email protected]>Signed-off-by Stefan Hajnoczi <[email protected]>--- blockdev.c | 4 ++++ 1 file changed, 4 insertions(+)blockdev.c +0
-0
blockdev.c int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) · line 1189
Quoted 10 lines
blockdev.c +4
-0@@ -1180,6 +1180,10 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)*/ if (bdrv_get_attached_dev (bs)) {bdrv_make_anon (bs); + +/* Further I/O must not pause the guest */ +bdrv_set_on_error (bs,BLOCKDEV_ON_ERROR_REPORT , +BLOCKDEV_ON_ERROR_REPORT ); }else {drive_uninit (drive_get_by_blockdev (bs)); }
Paolo
Quoted 35 lines · Stefan Hajnoczi
Paolo Bonzini <[email protected]> suggested the following test case:
1. Launch a guest and wait at the GRUB boot menu:
qemu-system-x86_64 -enable-kvm -m 1024 \ -drive if=none,cache=none,file=test.img,id=foo,werror=stop,rerror=stop -device virtio-blk-pci,drive=foo,id=virtio0,addr=42. Hot unplug the device:
(qemu)
drive_delfoo3. Select the first boot menu entry
Without this patch the guest pauses due to ENOMEDIUM. But it is not possible to resolve this situation - the drive has become anonymous.
With this patch the guest the guest gets the ENOMEDIUM error.
Note that this scenario actually happens sometimes during libvirt disk hot unplug, where
device_delis followed bydrive_del. I/O may still be submitted to the drive afterdrive_delif the guest does not process the PCI hot unplug notification.Reported-by Dafna Ron <[email protected]>Signed-off-by Stefan Hajnoczi <[email protected]>--- blockdev.c | 4 ++++ 1 file changed, 4 insertions(+)blockdev.c +0
-0
blockdev.c int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) · line 1189
Quoted 10 lines
blockdev.c +4
-0@@ -1180,6 +1180,10 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)*/ if (bdrv_get_attached_dev (bs)) {bdrv_make_anon (bs); + +/* Further I/O must not pause the guest */ +bdrv_set_on_error (bs,BLOCKDEV_ON_ERROR_REPORT , +BLOCKDEV_ON_ERROR_REPORT ); }else {drive_uninit (drive_get_by_blockdev (bs)); }
The user gets exactly what he ordered. He ordered "stop on error", then provoked errors by turning the virtual block device into a virtual pile of scrap metal. Because that's exactly what drive_del does when used while a device model is attached to the drive.
The only sane use case for drive_del I can think of is revoking access to an image violently, after the guest failed to honor a hot unplug.
Even then, using drive_del when the block device is removable is unnecessary. Just rip out the medium with eject -f. Look ma, no scrap metal.
I'm not sure what you mean by "it is not possible to resolve this situation". The device is shot! Can't see how that could be resolved.
I figure the bit that can't be resolved now is letting the user switch off "stop on error" safely before a drive_del. Even if we had a command for that, there'd still be a window between that command's execution and drive_del's. Your patch solves the problem by having drive_del switch it off unconditionally. Oookay, but please document it, because it's not exactly obvious.
Re "the guest gets the ENOMEDIUM error": depends on the device. I doubt disks can signal "no medium", and even if they could, I doubt device drivers are prepared for it.
Re "this scenario actually happens sometimes during libvirt disk hot unplug, where device_del is followed by drive_del": if I remember correctly, libvirt disk hot unplug runs drive_del right after device_del, opening a window where the guest sees a dead device. That's asking for trouble, and trouble is known to oblige.
Il 04/06/2013 18:37, Markus Armbruster ha scritto:
Quoted 6 lines
I figure the bit that can't be resolved now is letting the user switch off "stop on error" safely before a
drive_del. Even if we had a command for that, there'd still be a window between that command's execution and drive_del's. Your patch solves the problem by havingdrive_delswitch it off unconditionally. Oookay, but please document it, because it's not exactly obvious.
It is not obvious, but it is not surprising either when you see it (i.e. you won't really be surprised by the errors in the guest and won't need to know that, under the hood, rerror has been changed from the value you specified).
Quoted 3 lines
Re "the guest gets the ENOMEDIUM error": depends on the device. I doubt disks can signal "no medium", and even if they could, I doubt device drivers are prepared for it.
SCSI disks can signal whatever they want. Device drivers will just treat it as any other error (sense code) they don't recognize.
Quoted 5 lines
Re "this scenario actually happens sometimes during libvirt disk hot unplug, where
device_delis followed bydrive_del": if I remember correctly, libvirt disk hot unplug runsdrive_delright afterdevice_del, opening a window where the guest sees a dead device. That's asking for trouble, and trouble is known to oblige.
I think it's causing too much trouble though, and Stefan's patch is making the trouble evident to the guest. Surprise removal is a fact of life, I don't think it makes sense to stop the machine on surprise removal. It's very different from I/O errors.
Paolo
Quoted 13 lines · Paolo Bonzini
Il 04/06/2013 18:37, Markus Armbruster ha scritto:
Quoted 6 lines
I figure the bit that can't be resolved now is letting the user switch off "stop on error" safely before a
drive_del. Even if we had a command for that, there'd still be a window between that command's execution and drive_del's. Your patch solves the problem by havingdrive_delswitch it off unconditionally. Oookay, but please document it, because it's not exactly obvious.It is not obvious, but it is not surprising either when you see it (i.e. you won't really be surprised by the errors in the guest and won't need to know that, under the hood, rerror has been changed from the value you specified).
Quoted 6 lines
Quoted 3 lines
Re "the guest gets the ENOMEDIUM error": depends on the device. I doubt disks can signal "no medium", and even if they could, I doubt device drivers are prepared for it.
SCSI disks can signal whatever they want. Device drivers will just treat it as any other error (sense code) they don't recognize.
My point is: the commit message claims "the guest gets the ENOMEDIUM error", which isn't really true. No biggie, of course.
Quoted 10 lines
Quoted 5 lines
Re "this scenario actually happens sometimes during libvirt disk hot unplug, where
device_delis followed bydrive_del": if I remember correctly, libvirt disk hot unplug runsdrive_delright afterdevice_del, opening a window where the guest sees a dead device. That's asking for trouble, and trouble is known to oblige.I think it's causing too much trouble though, and Stefan's patch is making the trouble evident to the guest. Surprise removal is a fact of life, I don't think it makes sense to stop the machine on surprise removal. It's very different from I/O errors.
I don't disagree with Stefan's patch, or your defense of it.
Except I'm reluctant to not document something non-obvious because "you don't need to know" when I can document it in less time it would take me to overcome my resistance to "you don't need to know" arguments ;)
This is drive_add's documentation in hmp-commands.hx:
Remove host block device. The result is that guest generated IO is no longer submitted against the host device underlying the disk. Once a drive has been deleted, the QEMU Block layer returns -EIO which results in IO errors in the guest for applications that are reading/writing to the device.
Suggest to add:
These errors are always reported to the guest, regardless of the drive's error actions (drive options rerror, werror).
Independently, libvirt needs fixing.
Quoted 24 lines · Markus Armbruster
Quoted 8 lines · Paolo Bonzini
Il 04/06/2013 18:37, Markus Armbruster ha scritto:
Quoted 6 lines
I figure the bit that can't be resolved now is letting the user switch off "stop on error" safely before a
drive_del. Even if we had a command for that, there'd still be a window between that command's execution and drive_del's. Your patch solves the problem by havingdrive_delswitch it off unconditionally. Oookay, but please document it, because it's not exactly obvious.Quoted 5 lines
It is not obvious, but it is not surprising either when you see it (i.e. you won't really be surprised by the errors in the guest and won't need to know that, under the hood, rerror has been changed from the value you specified).
This is drive_add's documentation in hmp-commands.hx:
Remove host block device. The result is that guest generated IO is no longer submitted against the host device underlying the disk. Once a drive has been deleted, the QEMU Block layer returns
-EIOwhich results in IO errors in the guest for applications that are reading/writing to the device.
Quoted 6 lines
Suggest to add:
These errors are always reported to the guest, regardless of the drive's error actions (drive options rerror, werror).
Independently, libvirt needs fixing.
Total agreement that libvirt needs to use a saner disk hot-unplug sequence when it is known that qemu provides one. I've filed
https://bugzilla.redhat.com/show_bug.cgi?id=970761
to remind us to fix libvirt.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 30 lines · Eric Blake
Quoted 29 lines · Markus Armbruster
Quoted 8 lines · Paolo Bonzini
Il 04/06/2013 18:37, Markus Armbruster ha scritto:
Quoted 6 lines
I figure the bit that can't be resolved now is letting the user switch off "stop on error" safely before a
drive_del. Even if we had a command for that, there'd still be a window between that command's execution and drive_del's. Your patch solves the problem by havingdrive_delswitch it off unconditionally. Oookay, but please document it, because it's not exactly obvious.Quoted 5 lines
It is not obvious, but it is not surprising either when you see it (i.e. you won't really be surprised by the errors in the guest and won't need to know that, under the hood, rerror has been changed from the value you specified).
This is drive_add's documentation in hmp-commands.hx:
Remove host block device. The result is that guest generated IO is no longer submitted against the host device underlying the disk. Once a drive has been deleted, the QEMU Block layer returns
-EIOwhich results in IO errors in the guest for applications that are reading/writing to the device.Suggest to add:
These errors are always reported to the guest, regardless of the drive's error actions (drive options rerror, werror).
Quoted 6 lines
Quoted 1 line
Independently, libvirt needs fixing.
Total agreement that libvirt needs to use a saner disk hot-unplug sequence when it is known that qemu provides one. I've filed
https://bugzilla.redhat.com/show_bug.cgi?id=970761
to remind us to fix libvirt.
Sane sequence
1. device_del
2. Wait for DEVICE_DELETED
3. if timeout, drive_del
Might make sense to offer a choice in the API between "fail" and "destroy the block device" if guest doesn't cooperate.
Quoted 56 lines · Markus Armbruster
Quoted 35 lines · Stefan Hajnoczi
Paolo Bonzini <[email protected]> suggested the following test case:
1. Launch a guest and wait at the GRUB boot menu:
qemu-system-x86_64 -enable-kvm -m 1024 \ -drive if=none,cache=none,file=test.img,id=foo,werror=stop,rerror=stop -device virtio-blk-pci,drive=foo,id=virtio0,addr=42. Hot unplug the device:
(qemu)
drive_delfoo3. Select the first boot menu entry
Without this patch the guest pauses due to ENOMEDIUM. But it is not possible to resolve this situation - the drive has become anonymous.
With this patch the guest the guest gets the ENOMEDIUM error.
Note that this scenario actually happens sometimes during libvirt disk hot unplug, where
device_delis followed bydrive_del. I/O may still be submitted to the drive afterdrive_delif the guest does not process the PCI hot unplug notification.Reported-by Dafna Ron <[email protected]>Signed-off-by Stefan Hajnoczi <[email protected]>--- blockdev.c | 4 ++++ 1 file changed, 4 insertions(+)blockdev.c +0
-0blockdev.c int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) · line 1189
Quoted 10 lines
blockdev.c +4
-0@@ -1180,6 +1180,10 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)*/ if (bdrv_get_attached_dev (bs)) {bdrv_make_anon (bs); + +/* Further I/O must not pause the guest */ +bdrv_set_on_error (bs,BLOCKDEV_ON_ERROR_REPORT , +BLOCKDEV_ON_ERROR_REPORT ); }else {drive_uninit (drive_get_by_blockdev (bs)); }The user gets exactly what he ordered. He ordered "stop on error", then provoked errors by turning the virtual block device into a virtual pile of scrap metal. Because that's exactly what
drive_deldoes when used while a device model is attached to the drive.The only sane use case for
drive_delI can think of is revoking access to an image violently, after the guest failed to honor a hot unplug.
Quoted 6 lines
Even then, using
drive_delwhen the block device is removable is unnecessary. Just rip out the medium with eject -f. Look ma, no scrap metal.I'm not sure what you mean by "it is not possible to resolve this situation". The device is shot! Can't see how that could be resolved.
This is the critical part: the guest is paused and there is no way to resolve the continuous pause loop. The drive is gone but the guest hasn't PCI hot unplugged the storage controller. As a user, there's nothing you can do on the QEMU monitor to resume the guest - it will just pause itself again.
This behavior is really bad, QEMU has basically wedged the guest into an unrecoverable state and that's what I was trying to describe.
Quoted 6 lines
I figure the bit that can't be resolved now is letting the user switch off "stop on error" safely before a
drive_del. Even if we had a command for that, there'd still be a window between that command's execution and drive_del's. Your patch solves the problem by havingdrive_delswitch it off unconditionally. Oookay, but please document it, because it's not exactly obvious.
Thanks for the documentation suggestion, will add it in v2.
Quoted 3 lines
Re "the guest gets the ENOMEDIUM error": depends on the device. I doubt disks can signal "no medium", and even if they could, I doubt device drivers are prepared for it.
Yep, error reporting depends on the emulated storage controller. virtio-blk and IDE just report a generic error status.
Quoted 5 lines
Re "this scenario actually happens sometimes during libvirt disk hot unplug, where
device_delis followed bydrive_del": if I remember correctly, libvirt disk hot unplug runsdrive_delright afterdevice_del, opening a window where the guest sees a dead device. That's asking for trouble, and trouble is known to oblige.
Agreed.
Quoted 10 lines · Stefan Hajnoczi
Paolo Bonzini <[email protected]> suggested the following test case:
1. Launch a guest and wait at the GRUB boot menu:
qemu-system-x86_64 -enable-kvm -m 1024 \ -drive if=none,cache=none,file=test.img,id=foo,werror=stop,rerror=stop -device virtio-blk-pci,drive=foo,id=virtio0,addr=42. Hot unplug the device:
Quoted 8 lines
(qemu)
drive_delfoo3. Select the first boot menu entry
Without this patch the guest pauses due to ENOMEDIUM. But it is not possible to resolve this situation - the drive has become anonymous.
With this patch the guest the guest gets the ENOMEDIUM error.
s/the guest the guest/the guest/
Signature · 1 line
Fam