Re: [Qemu-devel] [PATCH V2 0/4] ide: avoid main-loop hang on CDROM/NFS failure

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3426 bytes --]

On Mon, Oct 26, 2015 at 11:56:26AM +0100, Peter Lieven wrote:
> Am 26.10.2015 um 11:42 schrieb Stefan Hajnoczi:
> >On Mon, Oct 12, 2015 at 02:27:21PM +0200, Peter Lieven wrote:
> >>This series aims at avoiding a hanging main-loop if a vserver has a
> >>CDROM image mounted from a NFS share and that NFS share goes down.
> >>Typical situation is that users mount an CDROM ISO to install something
> >>and then forget to eject that CDROM afterwards.
> >>As a consequence this mounted CD is able to bring down the
> >>whole vserver if the backend NFS share is unreachable. This is bad
> >>especially if the CDROM itself is not needed anymore at this point.
> >>
> >>This series aims at fixing 2 blocking I/O operations that would
> >>hang if the NFS server is unavailable:
> >>  - ATAPI PIO read requests used sync calls to blk_read, convert
> >>    them to an async variant where possible.
> >>  - If a busmaster DMA request is cancelled all requests are drained.
> >>    Convert the drain to an async request canceling.
> >>
> >>v1->v2: - fix offset for 2352 byte sector size [Kevin]
> >>         - use a sync request if we continue an elementary transfer.
> >>           As John pointed out we enter a race condition between next
> >>           IDE command and async transfer otherwise. This is sill not
> >>           optimal, but it fixes the NFS down problems for all cases where
> >>           the NFS server goes down while there is no PIO CD activity.
> >>           Of course, it could still happen during a PIO transfer, but I
> >>           expect this to be the unlikelier case.
> >>           I spent some effort trying to read more sectors at once and
> >>           avoiding continuation of elementary transfers, but with
> >>           whatever I came up it was destroying migration between different
> >>           Qemu versions. I have a quite hackish patch that works and
> >>           should survive migration, but I am not happy with it. So I
> >>           would like to start with this version as it is a big improvement
> >>           already.
> >>         - Dropped Patch 5 because it is upstream meanwhile.
> >>
> >>Peter Lieven (4):
> >>   ide/atapi: make PIO read requests async
> >>   ide/atapi: blk_aio_readv may return NULL
> >>   ide: add support for cancelable read requests
> >>   ide/atapi: enable cancelable requests
> >>
> >>  hw/ide/atapi.c    | 99 +++++++++++++++++++++++++++++++++++++++++++++++++------
> >>  hw/ide/core.c     | 55 +++++++++++++++++++++++++++++++
> >>  hw/ide/internal.h | 16 +++++++++
> >>  hw/ide/pci.c      | 42 +++++++++++++++--------
> >>  4 files changed, 188 insertions(+), 24 deletions(-)
> >Any reason why write and discard requests aren't covered in this series?
> >
> >If this is a good idea for CD-ROM it should be a good idea for all PCI
> >IDE devices.
> >
> >Having a specialized code path is often a sign that it hasn't been
> >tested enough.  Can we get confident enough to enable this everywhere?
> 
> The reason is that the buffered request trick does only work for
> read-only devices (like a CDROM). A write request that is completed
> on the backend storage at a later point (after the OS thinks the request
> is canceled) can cause damage to the filesystem.

Of course, you are right.

This is really annoying because it means a guest cannot reboot if writes
are pending...

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]