From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43262) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxCJT-0004y2-Mt for qemu-devel@nongnu.org; Fri, 13 Nov 2015 06:14:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZxCJQ-0001E2-G4 for qemu-devel@nongnu.org; Fri, 13 Nov 2015 06:14:39 -0500 Message-ID: <5645C5E0.6060601@ilande.co.uk> Date: Fri, 13 Nov 2015 11:13:36 +0000 From: Mark Cave-Ayland MIME-Version: 1.0 References: <1447345846-15624-1-git-send-email-pl@kamp.de> In-Reply-To: <1447345846-15624-1-git-send-email-pl@kamp.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V4 0/6] ide: avoid main-loop hang on CDROM/NFS failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: kwolf@redhat.com, stefanha@gmail.com, jcody@redhat.com, jsnow@redhat.com, famz@redhat.com On 12/11/15 16:30, 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. > > v3->v4: - Patch 1: remove buf argument for cd_read_sector{_sync} > - Patch 1: fix iov_base offset for 2352 sector size > - Patch 2: fix indent [Fam] > - Patch 3: fix leaking of req->iov.iov_base [Fam] > > v2->v3: - adressed Stefans comments on Patch 1 > - added patches 2,4,5,6 > - avoided the term cancel in Patch 3. Added an iovec, > added a BH [Stefan] > 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 (6): > ide/atapi: make PIO read requests async > block: add blk_abort_aio_request > ide: add support for IDEBufferedRequest > ide: orphan all buffered requests on DMA cancel > ide: enable buffered requests for ATAPI devices > ide: enable buffered requests for PIO read requests > > block/block-backend.c | 17 +++---- > hw/ide/atapi.c | 103 +++++++++++++++++++++++++++++++++++------ > hw/ide/core.c | 51 +++++++++++++++++++- > hw/ide/internal.h | 14 ++++++ > hw/ide/pci.c | 19 ++++++++ > include/sysemu/block-backend.h | 3 ++ > 6 files changed, 182 insertions(+), 25 deletions(-) Patches 4-6 look like they may handle issues I've found with migration in DMA requests on PPC g3beige and mac99 machines. Unfortunately due to alignment issues, the macio controller has implement its own versions of some of these routines, so should parts of these also be applied hw/ide/macio.c aswell? ATB, Mark.