All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Hanna Reitz <hreitz@redhat.com>
Cc: Emanuele Giuseppe Esposito <eesposit@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [PATCH 2/4] qcow2: Do not reopen data_file in invalidate_cache
Date: Wed, 27 Apr 2022 18:05:35 +0200	[thread overview]
Message-ID: <Ymlpz3Vyh7vTi4JS@redhat.com> (raw)
In-Reply-To: <20220427114057.36651-3-hreitz@redhat.com>

Am 27.04.2022 um 13:40 hat Hanna Reitz geschrieben:
> qcow2_co_invalidate_cache() closes and opens the qcow2 file, by calling
> qcow2_close() and qcow2_do_open().  These two functions must thus be
> usable from both a global-state and an I/O context.
> 
> As they are, they are not safe to call in an I/O context, because they
> use bdrv_unref_child() and bdrv_open_child() to close/open the data_file
> child, respectively, both of which are global-state functions.  When
> used from qcow2_co_invalidate_cache(), we do not need to close/open the
> data_file child, though (we do not do this for bs->file or bs->backing
> either), and so we should skip it in the qcow2_co_invalidate_cache()
> path.
> 
> To do so, add a parameter to qcow2_do_open() and qcow2_close() to make
> them skip handling s->data_file, and have qcow2_co_invalidate_cache()
> exempt it from the memset() on the BDRVQcow2State.
> 
> (Note that the QED driver similarly closes/opens the QED image by
> invoking bdrv_qed_close()+bdrv_qed_do_open(), but both functions seem
> safe to use in an I/O context.)
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/945
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>

This feels a bit like a hack, and we'll have to change it again if we
ever want to allow changing the data_file with reopen. But it should do
the job for now.

Kevin



  parent reply	other threads:[~2022-04-27 16:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-27 11:40 [PATCH 0/4] Revert "main-loop: Disable GLOBAL_STATE_CODE() assertions" Hanna Reitz
2022-04-27 11:40 ` [PATCH 1/4] block: Classify bdrv_get_flags() as I/O function Hanna Reitz
2022-04-27 13:14   ` Eric Blake
2022-04-27 11:40 ` [PATCH 2/4] qcow2: Do not reopen data_file in invalidate_cache Hanna Reitz
2022-04-27 13:20   ` Eric Blake
2022-04-27 16:05   ` Kevin Wolf [this message]
2022-04-27 11:40 ` [PATCH 3/4] Revert "main-loop: Disable GLOBAL_STATE_CODE() assertions" Hanna Reitz
2022-04-27 13:22   ` Eric Blake
2022-04-27 11:40 ` [PATCH 4/4] iotests: Add regression test for issue 945 Hanna Reitz
2022-04-27 13:52   ` Eric Blake
2022-04-27 16:13 ` [PATCH 0/4] Revert "main-loop: Disable GLOBAL_STATE_CODE() assertions" Kevin Wolf

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=Ymlpz3Vyh7vTi4JS@redhat.com \
    --to=kwolf@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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.