[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] lock-free monitor?
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] lock-free monitor? |
Date: |
Mon, 15 Feb 2016 15:19:35 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Stefan Hajnoczi <address@hidden> writes:
> On Sun, Feb 14, 2016 at 02:22:10PM +0800, Fam Zheng wrote:
>> On Tue, 02/09 13:47, Stefan Hajnoczi wrote:
>> > On Mon, Feb 08, 2016 at 03:17:23PM +0000, Dr. David Alan Gilbert wrote:
>> > > Does this make sense to everyone else, or does anyone have any better
>> > > suggestions?
>> >
>> > As a concrete example, any monitor command that calls bdrv_drain_all()
>> > can hang forever with the QEMU global mutex held if I/O requests are
>> > stuck (e.g. NFS mount is unreachable).
>> >
>> > bdrv_aio_cancel() can also hang but is mostly exposed to device
>> > emulation, not the monitor.
>> >
>> > One solution for these block layer functions is to add a timeout
>> > argument and let them return an error. This way the monitor and device
>> > emulation do not hang forever.
>>
>> Yes, there are a few places in block layer invoking aio_poll() in a loop
>> waiting for certain events, and a disconnected network link could make QEMU
>> hang. In these cases a timeout is a huge improvement. Maybe we can mark the
>> BDS as "hanging" (-EIO is returned for all further requests) and let
>> bdrv_drain_all() return.
>
> No, we need to be very careful about hung requests that are still in
> flight. They could complete after a long time and modify the disk or
> guest RAM.
>
> The only thing bdrv_drain_all() can return is -ETIME. Beyond that it
> cannot pretend that requests have been drained since that could lead to
> data corruption/loss.
>
> The caller has to abort whatever it was trying to do since it has no
> guarantee that requests have drained. Maybe the remaining requests will
> fail and go away, maybe they will complete. We don't know...
>
>> >
>> > The benefit of the timeout is that both monitor and device emulation
>> > hangs are tackled. It also doesn't require monitor changes.
>> >
>> > I'm not sure who chooses the timeout value and which value makes sense
>> > (policy vs mechanism separation)...
>>
>> Default to 30 seconds like Linux, and make it tunable through command line
>> options as well as QMP?
>
> Yes. Either commands that block can take an optional timeout (seconds)
> parameter, or we could have a global disk I/O timeout value. I prefer
> passing an optional per-command value.
>
> Libvirt and other sophisticated clients could begin using it in a
> backwards-compatible way. Existing code wouldn't use it and still
> suffer from the hang problem (but at least QEMU is
> backwards-compatible).
I'm not arguing against timeouts, I just want to remind everyone that
blocking while holding the BQL is a bug, and limiting the block time
with timeouts is a work-around, no more, no less.