qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: Nir Soffer <nsoffer@redhat.com>,
	Alberto Garcia <berto@igalia.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	qemu-block <qemu-block@nongnu.org>
Subject: Re: [PATCH 0/2] qcow2: Force preallocation with data-file-raw
Date: Tue, 23 Jun 2020 12:04:00 +0200	[thread overview]
Message-ID: <20200623100400.GC5853@linux.fritz.box> (raw)
In-Reply-To: <66581529-dc44-5fd0-54a3-1117b073077a@redhat.com>

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

Am 23.06.2020 um 09:28 hat Max Reitz geschrieben:
> On 22.06.20 19:44, Alberto Garcia wrote:
> > On Mon 22 Jun 2020 11:47:32 AM CEST, Max Reitz wrote:
> >>> I don't know the internals of qcow2 data_file, but are we really using
> >>> qcow2 metadata when accessing the data file?
> >>
> >> Yes.
> >>
> >>> This may have unwanted performance consequences.
> >>
> >> I don’t think so, because in practice normal lookups of L1/L2 mappings
> >> generally don’t cost that much performance.
> > 
> > ...if the L2 cache size is large enough. Otherwise you need one extra
> > read operation to retrieve the L2 metadata.
> > 
> > Possible performance problems when you have preallocation:
> > 
> >    - If a block hasn't been written yet (it's all zeroes) then you still
> >      need to read the L2 entry and read the data block. If there is not
> >      L2 table then you can simply return zeroes without going to disk at
> >      all. This of course assumes that the contents of the unwritten data
> >      block are zeroes.
> > 
> >    - QEMU still needs to read from disk (and cache in memory) the L2
> >      metadata, when it already knows in advance the contents of the L2
> >      entry (guest_offset == host_offset).
> 
> We could well optimize this regardless of preallocation.  With
> data-file-raw, qemu doesn’t have to look at the L2 metadata at all.
> 
> So the problem isn’t preallocation at all, it’s the fact that we don’t
> have such an optimization.  But note that to implement such an
> optimization, we really do need preallocation: Because it would mean
> that we wouldn’t touch the L1/L2 tables for data-file-raw images during
> runtime, which would effectively make those images empty to today’s qemu
> versions.

It depends. For reads, bypassing the L1/L2 tables is completely fine
with data-file-raw. It may miss opportunities to optimise reading
unallocated/zeroed clusters, but if the data file is actually sparse, it
shouldn't make a big difference. Maybe we should just do this.

For (potentially allocating) writes, you're right that we need to be
more careful. If we want to completely bypass L1/L2 tables,
preallocation is not enough, but we have to make sure that we never
discard any clusters.

Whatever we do for writes will be a non-trivial change. I wonder if it's
really worth doing this for optimisation when nobody uses the feature
yet anyway.

> (OTOH, preallocation would then be pretty much superfluous for all newer
> versions of qemu.  To address that, we could then add an incompatible
> version of data-file-raw.  But I think we should only think about that
> once we get to that point.)

Well, if we create an incompatible version, we can have one that doesn't
even store L1/L2 tables.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2020-06-23 10:05 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 10:40 [PATCH 0/2] qcow2: Force preallocation with data-file-raw Max Reitz
2020-06-19 10:40 ` [PATCH 1/2] " Max Reitz
2020-06-19 16:47   ` Alberto Garcia
2020-06-22  9:35     ` Max Reitz
2020-06-22  9:48       ` Max Reitz
2020-06-23 10:26         ` Kevin Wolf
2020-06-22 14:46       ` Alberto Garcia
2020-06-22 15:06         ` Max Reitz
2020-06-22 15:15           ` Nir Soffer
2020-06-22 15:48             ` Max Reitz
2020-06-22 18:34               ` Eric Blake
2020-06-22 17:36           ` Alberto Garcia
2020-06-23  7:28             ` Max Reitz
2020-06-19 10:40 ` [PATCH 2/2] iotests/244: Test preallocation for data-file-raw Max Reitz
2020-06-19 11:55 ` [PATCH 0/2] qcow2: Force preallocation with data-file-raw no-reply
2020-06-21 22:25 ` Nir Soffer
2020-06-22  9:47   ` Max Reitz
2020-06-22 15:50     ` Nir Soffer
2020-06-23 10:18       ` Kevin Wolf
2020-06-22 17:44     ` Alberto Garcia
2020-06-23  7:28       ` Max Reitz
2020-06-23 10:04         ` Kevin Wolf [this message]

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=20200623100400.GC5853@linux.fritz.box \
    --to=kwolf@redhat.com \
    --cc=berto@igalia.com \
    --cc=mreitz@redhat.com \
    --cc=nsoffer@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).