Re: [Qemu-devel] [PATCH] blockdev: reset werror/rerror on drive_del

[Markus Armbruster]

Stefan Hajnoczi <address@hidden> writes:

> Paolo Bonzini <address@hidden> 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.
>
> Reported-by: Dafna Ron <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  blockdev.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index d1ec99a..6eb81a3 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -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.