[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V4 0/6] ide: avoid main-loop hang on CDROM/NFS f
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH V4 0/6] ide: avoid main-loop hang on CDROM/NFS failure |
Date: |
Mon, 16 Nov 2015 14:24:53 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/16/2015 01:17 AM, Fam Zheng wrote:
> On Fri, 11/13 17:44, John Snow wrote:
>>
>>
>> On 11/12/2015 11:30 AM, 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(-)
>>>
>>
>> It looks sane to me:
>>
>> Reviewed-by: John Snow <address@hidden>
>>
>> Fam, Stefan: Do you think this is still sane for 2.5? I am inclined to
>> get it in as a fix, especially since we've been bouncing it around for
>> so long.
>
> I have no objection here, there is no new feature in the series, and guest
> hanging right after installing something is admittedly annoying to users.
>
> Thanks,
> Fam
>
I will send a PR. Fam, should I add your R-B?
--js
- Re: [Qemu-devel] [PATCH V4 1/6] ide/atapi: make PIO read requests async, (continued)
- [Qemu-devel] [PATCH V4 2/6] block: add blk_abort_aio_request, Peter Lieven, 2015/11/12
- [Qemu-devel] [PATCH V4 3/6] ide: add support for IDEBufferedRequest, Peter Lieven, 2015/11/12
- [Qemu-devel] [PATCH V4 5/6] ide: enable buffered requests for ATAPI devices, Peter Lieven, 2015/11/12
- [Qemu-devel] [PATCH V4 6/6] ide: enable buffered requests for PIO read requests, Peter Lieven, 2015/11/12
- Re: [Qemu-devel] [PATCH V4 0/6] ide: avoid main-loop hang on CDROM/NFS failure, Mark Cave-Ayland, 2015/11/13
- Re: [Qemu-devel] [PATCH V4 0/6] ide: avoid main-loop hang on CDROM/NFS failure, John Snow, 2015/11/13