All of lore.kernel.org
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Fam Zheng <famz@redhat.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org,
	Kevin Wolf <kwolf@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	John Snow <jsnow@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 17/18] qemu-io: Add background write
Date: Thu, 21 Sep 2017 16:40:19 +0200	[thread overview]
Message-ID: <7a0de5bd-5e1e-856b-e2f3-29b1baad7cf2@redhat.com> (raw)
In-Reply-To: <20170919080311.GC11534@lemon.lan>

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

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.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

  reply	other threads:[~2017-09-21 14:40 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-13 18:18 [Qemu-devel] [PATCH 00/18] block/mirror: Add active-sync mirroring Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 01/18] block: Add BdrvDeletedStatus Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 02/18] block: BDS deletion during bdrv_drain_recurse Max Reitz
2017-09-18  3:44   ` Fam Zheng
2017-09-18 16:13     ` Max Reitz
2017-10-09 18:30       ` Max Reitz
2017-10-10  8:36   ` Kevin Wolf
2017-10-11 11:41     ` Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 03/18] blockjob: Make drained_{begin, end} public Max Reitz
2017-09-18  3:46   ` Fam Zheng
2017-09-13 18:18 ` [Qemu-devel] [PATCH 04/18] block/mirror: Pull out mirror_perform() Max Reitz
2017-09-18  3:48   ` Fam Zheng
2017-09-25  9:38   ` Vladimir Sementsov-Ogievskiy
2017-09-13 18:18 ` [Qemu-devel] [PATCH 05/18] block/mirror: Convert to coroutines Max Reitz
2017-09-18  6:02   ` Fam Zheng
2017-09-18 16:41     ` Max Reitz
2017-10-10  9:14   ` Kevin Wolf
2017-10-11 11:43     ` Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 06/18] block/mirror: Use CoQueue to wait on in-flight ops Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 07/18] block/mirror: Wait for in-flight op conflicts Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 08/18] block/mirror: Use source as a BdrvChild Max Reitz
2017-10-10  9:27   ` Kevin Wolf
2017-10-11 11:46     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 09/18] block: Generalize should_update_child() rule Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 10/18] block/mirror: Make source the file child Max Reitz
2017-10-10  9:47   ` Kevin Wolf
2017-10-11 12:02     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 11/18] hbitmap: Add @advance param to hbitmap_iter_next() Max Reitz
2017-09-25 15:38   ` Vladimir Sementsov-Ogievskiy
2017-09-25 20:40     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 12/18] block/dirty-bitmap: Add bdrv_dirty_iter_next_area Max Reitz
2017-09-25 15:49   ` Vladimir Sementsov-Ogievskiy
2017-09-25 20:43     ` Max Reitz
2017-10-02 13:32     ` Vladimir Sementsov-Ogievskiy
2017-09-13 18:19 ` [Qemu-devel] [PATCH 13/18] block/mirror: Keep write perm for pending writes Max Reitz
2017-10-10  9:58   ` Kevin Wolf
2017-10-11 12:20     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 14/18] block/mirror: Distinguish active from passive ops Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 15/18] block/mirror: Add active mirroring Max Reitz
2017-09-14 15:57   ` Stefan Hajnoczi
2017-09-16 13:58     ` Max Reitz
2017-09-18 10:06       ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-18 16:26         ` Max Reitz
2017-09-19  9:44           ` Stefan Hajnoczi
2017-09-19  9:57             ` Daniel P. Berrange
2017-09-20 14:56               ` Stefan Hajnoczi
2017-10-10 10:16           ` Kevin Wolf
2017-10-11 12:33             ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 16/18] block/mirror: Add copy mode QAPI interface Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 17/18] qemu-io: Add background write Max Reitz
2017-09-18  6:46   ` Fam Zheng
2017-09-18 17:53     ` Max Reitz
2017-09-19  8:03       ` Fam Zheng
2017-09-21 14:40         ` Max Reitz [this message]
2017-09-21 14:59           ` Fam Zheng
2017-09-21 15:03             ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 18/18] iotests: Add test for active mirroring Max Reitz
2017-09-18  6:45   ` Fam Zheng
2017-09-18 16:53     ` Max Reitz
2017-09-19  8:08       ` Fam Zheng
2017-09-14 15:42 ` [Qemu-devel] [PATCH 00/18] block/mirror: Add active-sync mirroring Stefan Hajnoczi
2017-09-16 14:02   ` Max Reitz
2017-09-18 10:02     ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-18 15:42       ` Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a0de5bd-5e1e-856b-e2f3-29b1baad7cf2@redhat.com \
    --to=mreitz@redhat.com \
    --cc=famz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.