[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 17/18] qemu-io: Add background write
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 17/18] qemu-io: Add background write |
Date: |
Thu, 21 Sep 2017 22:59:43 +0800 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Thu, 09/21 16:40, Max Reitz wrote:
> On 2017-09-19 10:03, Fam Zheng wrote:
> > On Mon, 09/18 19:53, Max Reitz wrote:
> >> On 2017-09-18 08:46, Fam Zheng wrote:
> >>> On Wed, 09/13 20:19, Max Reitz wrote:
> >>>> Add a new parameter -B to qemu-io's write command. When used, qemu-io
> >>>> will not wait for the result of the operation and instead execute it in
> >>>> the background.
> >>>
> >>> Cannot aio_write be used for this purpose?
> >>
> >> Depends. I have been trained to dislike *_aio_*, so that's probably the
> >> initial reason why I didn't use it.
> >>
> >> Second, I'd have to fix aio_write before it can be used. Currently,
> >> this aborts:
> >>
> >> echo 'qemu-io drv0 "aio_write -P 0x11 0 64M"' \
> >> | x86_64-softmmu/qemu-system-x86_64 -monitor stdio \
> >> -blockdev node-name=drv0,driver=null-co
> >>
> >> because aio_write_done thinks it's a good idea to use qemu-io's
> >> BlockBackend -- but when qemu-io is executed through the HMP, the
> >> BlockBackend is only created for the duration of the qemu-io command
> >> (unless there already is a BB). So what I'd have to do is add a
> >> blk_ref()/blk_unref() there, but for some reason I really don't like that.
> >
> > What is the reason? If it crashes it should be fixed anyway, I assume?
>
> Because the AIO CB (aio_write_done()) continues to use qemu-io's BB --
> but in case of HMP's qemu-io, that is pretty much already gone once the
> command is done.
I can see aio_{read,write}_done accesses BB for accounting, we can probably skip
this part altogether if issued from HMP (because the BB is gone). This way you
don't need the blk_ref/unref pair.
Fam
>
> That could be fixed, as I said, by blk_ref()ing the BB before aio_write
> returns (and then blk_unref()ing it in aio_write_done()). However, I'm
> not even sure whether aio_write_done() is always executed in the main
> thread...
>
> Other than that, I just have a bad feeling about adding the pair, not
> sure why. Probably because it means having to carry a temporary BB
> around until the command is done, which is weird. Well, it's not an
> issue permission-wise, because the qemu-io BB simply doesn't take the
> proper permissions (no, I'm not going to question the fact how it's then
> possible to even write to it, considering we have assertions that check
> whether the correct permissions have been taken...), and I can't think
> of another way.
>
> In any case, you're right, it probably needs to be fixed anyway -- even
> if the fix is just not allowing aio_write with a temporary BB (i.e. from
> HMP).
>
> Max
>
- Re: [Qemu-devel] [Qemu-block] [PATCH 15/18] block/mirror: Add active mirroring, (continued)
[Qemu-devel] [PATCH 16/18] block/mirror: Add copy mode QAPI interface, Max Reitz, 2017/09/13
[Qemu-devel] [PATCH 17/18] qemu-io: Add background write, Max Reitz, 2017/09/13
[Qemu-devel] [PATCH 18/18] iotests: Add test for active mirroring, Max Reitz, 2017/09/13
Re: [Qemu-devel] [PATCH 00/18] block/mirror: Add active-sync mirroring, Stefan Hajnoczi, 2017/09/14