[PATCH v2 0/2] qcow2: Force preallocation with data-file-raw

Max Reitz posted 2 patches 3 years ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210326145509.163455-1-mreitz@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
block/qcow2.c              |  34 ++++++++++++
tests/qemu-iotests/244     | 104 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/244.out |  68 ++++++++++++++++++++++--
3 files changed, 201 insertions(+), 5 deletions(-)
[PATCH v2 0/2] qcow2: Force preallocation with data-file-raw
Posted by Max Reitz 3 years ago
v1: https://lists.nongnu.org/archive/html/qemu-block/2020-06/msg00992.html


Hi,

I think that qcow2 images with data-file-raw should always have
preallocated 1:1 L1/L2 tables, so that the image always looks the same
whether you respect or ignore the qcow2 metadata.  The easiest way to
achieve that is to enforce at least metadata preallocation whenever
data-file-raw is given.

As far as I could tell, there were two main critique points about v1:
(1) If we force metadata preallocation on creation, we should also do it
    when the image is grown.
(2) We could go even further and make qemu ignore all L1/L2 tables for
    images with raw external data files.  Ideally, we wouldn’t even
    write them at all.

(1) is addressed in this v2.

As for (2)...  It’s complicated.  I think we want the fix from this
series now and if we want (2), we can have a go at it later.  Many
things are to be considered there.

For example: data-file-raw is an autoclear flag.  Technically, it is
possible for some qcow2 implementation to support data-file, but not
data-file-raw.  If we ignore metadata for images with data-file-raw, we
would break them, because “ignoring” would mean we don’t even create it,
ever, so the external data file would appear empty to such
implementations.
Now, in practice, there is no such implementation.  data-file-raw has
been introduced alongside data-file.
However, also in practice, qemu always did and still does rely on the
metadata in the qcow2 image.  So we have to ensure the metadata is
there, or all versions of qemu that support data-file will break.

The easiest way to ensure the metadata is there is to preallocate it on
creation/growth.  If at same later point we decide we want to ignore it
on runtime, this preallocation would actually allow us to do that.  So
the preallocation is the necessary first step (the second step would
probably be a second auto-clear flag that states that all metadata has
been preallocated and can thus be ignored at runtime).

((Even today, we could ignore the L2 tables when reading, but the
problems are that (1) images can then appear differently to qemu
versions that do ignore them and versions that don’t, and (2) when
writing to a cluster, we still need to ensure that its L2 entry is there
(i.e., allocated and pointing to the correct offset).  I don’t think it
makes sense to ignore the tables when reading but not when writing.))


There have also been proposals of instead just not writing any metadata.
This would naturally require an incompatible new flag, because such
images would not be usable by current qemu versions.  Such a flag would
make this series unnecessary, but do we really want to break
incompatibility with all qemu versions going back to 4.0 just so we
don’t have to waste space on L2 tables?  Users are free to just use 2M
clusters for data-file-raw images so the wasted space is minimized (to
1/2M of the image size, e.g. 512M per 1T).

And in any case: I think patch 1 is simple enough that we can just take
it now and it wouldn’t be too bad to write it off as a loss if we ever
add an incompatible no-l2 flag.

Point is, we have no actual patches to implement a no-l2 flag, but there
is something that needs to be fixed about raw external data files, and
this series fixes it.


v2:
- Patch 1: Force metadata preallocation when the image is resized
- Patch 2:
  - Use blockdev-create to create the qcow2 image instead of creating
    the qcow2 image first and then (technically illegally) writing to
    the external data file
  - Test growing a qcow2 image with an external data file, where the
    data file is grown first and the new area is filled with data


git-backport-diff against v1:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/2:[0012] [FC] 'qcow2: Force preallocation with data-file-raw'
002/2:[0110] [FC] 'iotests/244: Test preallocation for data-file-raw'


Max Reitz (2):
  qcow2: Force preallocation with data-file-raw
  iotests/244: Test preallocation for data-file-raw

 block/qcow2.c              |  34 ++++++++++++
 tests/qemu-iotests/244     | 104 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/244.out |  68 ++++++++++++++++++++++--
 3 files changed, 201 insertions(+), 5 deletions(-)

-- 
2.29.2


Re: [PATCH v2 0/2] qcow2: Force preallocation with data-file-raw
Posted by Max Reitz 3 years ago
On 26.03.21 15:55, Max Reitz wrote:
> v1: https://lists.nongnu.org/archive/html/qemu-block/2020-06/msg00992.html
> 
> 
> Hi,
> 
> I think that qcow2 images with data-file-raw should always have
> preallocated 1:1 L1/L2 tables, so that the image always looks the same
> whether you respect or ignore the qcow2 metadata.  The easiest way to
> achieve that is to enforce at least metadata preallocation whenever
> data-file-raw is given.

Thanks for the review, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max