[PATCH v5 29/31] qcow2: Assert that expand_zero_clusters_in_l1() does not support subclusters

This function is only used by qcow2_expand_zero_clusters() to downgrade a qcow2 image to a previous version. It is however not possible to downgrade an image with extended L2 entries because older versions of qcow2 do not have this feature. Signed-off-by: Alberto Garcia --- block/qcow2

[PATCH v5 31/31] iotests: Add tests for qcow2 images with extended L2 entries

Signed-off-by: Alberto Garcia --- tests/qemu-iotests/271 | 664 + tests/qemu-iotests/271.out | 519 + tests/qemu-iotests/group | 1 + 3 files changed, 1184 insertions(+) create mode 100755 tests/qemu-iotests/271 create

[PATCH v5 06/31] qcow2: Add get_l2_entry() and set_l2_entry()

to get_l2_entry() and set_l2_entry(). Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 12 block/qcow2-cluster.c | 63 ++ block/qcow2-refcount.c | 17

[PATCH v5 30/31] qcow2: Add subcluster support to qcow2_measure()

Extended L2 entries are bigger than normal L2 entries so this has an impact on the amount of metadata needed for a qcow2 file. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- block/qcow2.c | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/block

[PATCH v5 14/31] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()

patch. Signed-off-by: Alberto Garcia --- block/qcow2.h | 127 +- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/block/qcow2.h b/block/qcow2.h index 4ad93772b9..be7816a3b8 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -80,6 +80,21

[PATCH v5 28/31] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit

Now that the implementation of subclusters is complete we can finally add the necessary options to create and read images with this feature, which we call "extended L2 entries". Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- qapi/block-core.json | 7 +++ blo

[PATCH v5 25/31] qcow2: Clear the L2 bitmap when allocating a compressed cluster

Compressed clusters always have the bitmap part of the extended L2 entry set to 0. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- block/qcow2-cluster.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 4544a40aa0..0a295076a3

[PATCH v5 16/31] qcow2: Add cluster type parameter to qcow2_get_host_offset()

and in order to detect errors more easily. This patch makes qcow2_get_host_offset() return 0 on success and puts the returned cluster type in a separate parameter. There are no semantic changes. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block

[PATCH v5 26/31] qcow2: Add subcluster support to handle_alloc_space()

with zeroes the other subclusters if we can guarantee that we are not overwriting existing data. However this would waste more disk space, so we should first evaluate if it's really worth doing. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block

[PATCH v5 05/31] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()

for subclusters. Signed-off-by: Alberto Garcia --- block/qcow2-cluster.c | 256 +++--- 1 file changed, 141 insertions(+), 115 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 80f9787461..fce0be7a08 100644 --- a/block/qcow2-cluster.c

[PATCH v5 19/31] qcow2: Add subcluster support to calculate_l2_meta()

() and l2meta_cow_end() are not necessarily cluster-aligned anymore. We need to update the calculation of old_start and old_end in handle_dependencies() to guarantee that no two requests try to write on the same cluster. Signed-off-by: Alberto Garcia --- block/qcow2-cluster.c | 174

[PATCH v5 22/31] qcow2: Add subcluster support to discard_in_l2_slice()

that the image will read back as zeroes. If this is important for the caller it should forbid it as qcow2_co_pdiscard() does (see 80f5c01183 for more details). Signed-off-by: Alberto Garcia --- block/qcow2-cluster.c | 51 +++ 1 file changed, 22

[PATCH v5 09/31] qcow2: Add subcluster-related fields to BDRVQcow2State

reated as if they had exactly one subcluster per cluster (i.e. subcluster_size = cluster_size). Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 5 + block/qcow2.c | 5 + 2 files changed, 10 insertions(+) diff --git a/block/q

[PATCH v5 13/31] qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap()

the get/set_l2_bitmap() functions that are used to access the bitmaps. For convenience we allow calling get_l2_bitmap() on images without subclusters. In this case the returned value is always 0 and has no meaning. Signed-off-by: Alberto Garcia --- block/qcow2.h | 21 + 1 file

[PATCH v5 04/31] qcow2: Split cluster_needs_cow() out of count_cow_clusters()

We are going to need it in other places. Signed-off-by: Alberto Garcia Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/qcow2-cluster.c | 34 +++--- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/block/qcow2-cluster.c b

[PATCH v5 12/31] qcow2: Add l2_entry_size()

. This function returns the proper value for the image. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- block/qcow2.h | 9 + block/qcow2-cluster.c | 12 ++-- block/qcow2-refcount.c | 14 -- block/qcow2.c | 8 4 files changed, 27 insertions

[PATCH v5 15/31] qcow2: Add qcow2_cluster_is_allocated()

This helper function tells us if a cluster is allocated (that is, there is an associated host offset for it). Signed-off-by: Alberto Garcia --- block/qcow2.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/block/qcow2.h b/block/qcow2.h index be7816a3b8..b5db8d2f36 100644 --- a/block

[PATCH v5 21/31] qcow2: Add subcluster support to zero_in_l2_slice()

not need to be updated. Signed-off-by: Alberto Garcia --- block/qcow2-cluster.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index f500cbfb8e..bf32ed0825 100644 --- a/block/qcow2-cluster.c +++ b

[PATCH v5 27/31] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes()

. 2) if the tail area was compressed we are writing zeroes to the head and the body areas, which are already zeroized. Signed-off-by: Alberto Garcia --- block/qcow2.h | 4 +-- block/qcow2-cluster.c | 80 +++ block/qcow2.c | 27

[PATCH v5 07/31] qcow2: Document the Extended L2 Entries feature

-off-by: Alberto Garcia Reviewed-by: Max Reitz --- docs/interop/qcow2.txt | 68 -- docs/qcow2-cache.txt | 19 +++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt index 298a031310

[PATCH v5 20/31] qcow2: Add subcluster support to qcow2_get_host_offset()

The logic of this function remains pretty much the same, except that it uses count_contiguous_subclusters(), which combines the logic of count_contiguous_clusters() / count_contiguous_clusters_unallocated() and checks individual subclusters. Signed-off-by: Alberto Garcia --- block/qcow2.h

[PATCH v5 02/31] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

-off-by: Alberto Garcia Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 4 ++-- block/qcow2-cluster.c | 42 +++--- block/qcow2.c | 24 +++- 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/block

[PATCH v5 01/31] qcow2: Make Qcow2AioTask store the full host offset

with this patch but it is documented now. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.c | 69 ++ block/trace-events | 2 +- 2 files changed, 34 insertions

[PATCH v5 11/31] qcow2: Add offset_into_subcluster() and size_to_subclusters()

Like offset_into_cluster() and size_to_clusters(), but for subclusters. Signed-off-by: Alberto Garcia --- block/qcow2.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/block/qcow2.h b/block/qcow2.h index e68febb15b..8b1ed1cbcf 100644 --- a/block/qcow2.h +++ b/block/qcow2.h

[PATCH v5 24/31] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()

affected subclusters were already allocated). This is detected in calculate_l2_meta(), and qcow2_alloc_cluster_link_l2() is never called in those cases. Signed-off-by: Alberto Garcia --- block/qcow2-cluster.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/block/qcow2-cluster.c

[PATCH v5 08/31] qcow2: Add dummy has_subclusters() function

value. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/block/qcow2.h b/block/qcow2.h index 97fbaba574..5e8036c3dd 100644 --- a/block/qcow2.h +++ b

[PATCH v5 03/31] qcow2: Add calculate_l2_meta()

handle_alloc() creates a QCowL2Meta structure in order to update the image metadata and perform the necessary copy-on-write operations. This patch moves that code to a separate function so it can be used from other places. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- block/qcow2

[PATCH v5 18/31] qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC

When dealing with subcluster types there is a new value called QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC that has no equivalent in QCow2ClusterType. This patch handles that value in all places where subcluster types are processed. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed

[PATCH v5 10/31] qcow2: Add offset_to_sc_index()

For a given offset, return the subcluster number within its cluster (i.e. with 32 subclusters per cluster it returns a number between 0 and 31). Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 5 + 1 file changed, 5

Re: [PATCH v3] qcow2: Avoid integer wraparound in qcow2_co_truncate()

On Tue 05 May 2020 10:54:12 AM CEST, Kevin Wolf wrote: > But I think there is a more important problem with the test: It seems > to pass even with old binaries that don't have the fix. Is this only > on my system or do you get the same? With old binaries when qcow2_cluster_zeroize() is called it

Re: [PATCH v3] qcow2: Avoid integer wraparound in qcow2_co_truncate()

On Mon 04 May 2020 06:01:19 PM CEST, Eric Blake wrote: >> +_supported_fmt qcow2 >> +_supported_proto file > > Do we have to limit it to qcow2 and file? Yes, it's testing a bugfix > for qcow2, but are there other formats that it doesn't hurt to have > the extra testing? It doesn't work with any

[PATCH v3] qcow2: Avoid integer wraparound in qcow2_co_truncate()

-by: Alberto Garcia --- v3: - Add test case [Kevin] - Move MIN(zero_start, offset) to the block that writes zeroes [Kevin] v2: - Don't call qcow2_cluster_zeroize() if offset == zero_start block/qcow2.c | 12 --- tests/qemu-iotests/292 | 73 ++ tests

[PATCH v2] qcow2: Avoid integer wraparound in qcow2_co_truncate()

-by: Alberto Garcia --- block/qcow2.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) v2: - Don't call qcow2_cluster_zeroize() if offset == zero_start diff --git a/block/qcow2.c b/block/qcow2.c index 2ba0b17c39..7ca0327995 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4234,15

Re: [PATCH] qcow2: Avoid integer wraparound in qcow2_co_truncate()

On Fri 01 May 2020 08:48:31 PM CEST, Eric Blake wrote: > Since your reproducer triggers assertion failure, I suggest doing this > instead: >>> +++ b/block/qcow2.c >>> @@ -4234,6 +4234,9 @@ static int coroutine_fn >>> qcow2_co_truncate(BlockDriverState *bs, int64_t offset, >>>   if ((flags &

[PATCH] qcow2: Avoid integer wraparound in qcow2_co_truncate()

be reproduced with these steps: qemu-img create -f qcow2 backing.qcow2 1M qemu-img create -f qcow2 -b backing.qcow2 top.qcow2 qemu-img resize --shrink top.qcow2 520k qemu-img resize top.qcow2 567k In the last step offset - zero_start causes an integer wraparound. Signed-off-by: Alberto Garcia

Re: [PATCH 1/3] backup: Improve error for bdrv_getlength() failure

le at it, start with upper case to make the message consistent with > the rest of the function. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH] block: Comment cleanups

On Tue 28 Apr 2020 11:38:07 PM CEST, Eric Blake wrote: > It's been a while since we got rid of the sector-based bdrv_read and > bdrv_write (commit 2e11d756); let's finish the job on a few remaining > comments. > > Signed-off-by: Eric Blake Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

On Mon 27 Apr 2020 09:49:00 AM CEST, Max Reitz wrote: >> The point is this: Consider 'write -P 0xff 0 64k', then 'write -z 16k >> 16k', then 'read 0 64k'. For normal clusters, we can just do a >> scatter-gather iov read of read 0-16k and 32-64k, plus a memset of >> 16-32k. But for compressed

Re: [PATCH v4 23/30] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()

On Fri 24 Apr 2020 09:39:25 PM CEST, Eric Blake wrote: >> +/* Update bitmap with the subclusters that were just written */ >> +if (has_subclusters(s)) { >> +unsigned written_from = m->cow_start.offset; >> +unsigned written_to = m->cow_end.offset +

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

On Fri 24 Apr 2020 08:15:04 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > AFAIK, now compressed clusters can't be used in scenarios with guest, > as qcow2 driver doesn't support rewriting them. You can write to those images just fine, it's just not efficient because you have to COW the

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

On Fri 24 Apr 2020 08:25:45 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >>> Reading the entire cluster will be interesting - you'll have to >>> decompress the entire memory, then overwrite the zeroed portions. >> >> I don't think so, qcow2_get_host_offset() would detect the number of >>

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

On Fri 24 Apr 2020 07:44:33 PM CEST, Eric Blake wrote: >>> at the same time, I can see where you're coming from in stating that >>> if it makes management of extended L2 easier to allow zero subclusters >>> on top of a compressed cluster, then there's no reason to forbid it. >> >> I'm not sure

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

On Fri 24 Apr 2020 07:11:08 PM CEST, Eric Blake wrote: >> 'write -c 0 64k' followed by 'write -z 16k 16k' would not need to do any >> copy on write. The compressed data would remain untouched on disk but >> some of the subclusters would have the 'all zeroes' bit set, exactly >> like what happens

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

On Tue 17 Mar 2020 07:16:21 PM CET, Alberto Garcia wrote: > Compressed clusters always have the bitmap part of the extended L2 > entry set to 0. I was just finishing some improvements to the new code that allows BDRV_REQ_ZERO_WRITE at the subcluster level, and I'm starting to entertain th

Re: [PATCH v4 21/30] qcow2: Add subcluster support to check_refcounts_l2()

On Wed 22 Apr 2020 02:06:56 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > 17.03.2020 21:16, Alberto Garcia wrote: >> Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an >> image has subclusters. Instead, the individual 'all zeroes' bits must >> be us

Re: [PATCH v4 22/30] qcow2: Fix offset calculation in handle_dependencies()

On Wed 22 Apr 2020 02:38:54 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > 17.03.2020 21:16, Alberto Garcia wrote: >> l2meta_cow_start() and l2meta_cow_end() are not necessarily >> cluster-aligned if the image has subclusters, so update the >> calculation of old_start and

Re: [PATCH v4 20/30] qcow2: Add subcluster support to discard_in_l2_slice()

On Wed 22 Apr 2020 08:09:53 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >> There's currently an inconsistency now that I think of it: if an image >> has subclusters and QCOW_OFLAG_ZERO set then qcow2_get_cluster_type() >> returns QCOW2_CLUSTER_ZERO_* but qcow2_get_subcluster_type() returns >>

Re: [PATCH v4 20/30] qcow2: Add subcluster support to discard_in_l2_slice()

On Wed 22 Apr 2020 01:35:25 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > 17.03.2020 21:16, Alberto Garcia wrote: >> Two changes are needed in this function: >> >> 1) A full discard deallocates a cluster so we can skip the operation if >> it is already unallocate

Re: [PATCH v4 19/30] qcow2: Add subcluster support to zero_in_l2_slice()

On Wed 22 Apr 2020 01:06:42 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >> @@ -1897,7 +1897,7 @@ static int zero_in_l2_slice(BlockDriverState *bs, >> uint64_t offset, > > As I see, function is not prepared to handle unaligned offset. Worth > add an assertion while being here? The only caller

Re: [PATCH v4 18/30] qcow2: Add subcluster support to qcow2_get_host_offset()

On Wed 22 Apr 2020 10:07:30 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> +static int count_contiguous_subclusters(BlockDriverState *bs, int >> nb_clusters, >> +unsigned sc_index, uint64_t >> *l2_slice, >> +int

Re: [PATCH v4 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

On Tue 21 Apr 2020 10:47:17 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> +if ((flags & BDRV_REQ_ZERO_WRITE) && offset > old_length) { >> +ret = qcow2_cluster_zeroize(bs, old_length, offset - old_length, 0); > > Hmm. As I understand, qcow2_cluster_zeroize is unprepared to >

Re: [PATCH v20 1/4] qcow2: introduce compression type feature

_change) > * add "compression type" for test output matching when it isn't filtered > affected tests: 049, 060, 061, 065, 144, 182, 242, 255 > > Signed-off-by: Denis Plotnikov > Reviewed-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Eric Blake > QAPI part: > Acked-by: Markus Armbruster Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 7/9] block: truncate: Don't make backing file data visible

t; > After commit top.qcow2 to mid.qcow2, the following happens: > > mid.qcow2: CB-C00C0 (correct result) > mid.qcow2: CB-C--C- (before this fix) > > Without the fix, blocks that previously read as zeros on top.qcow2 > suddenly turn into A. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 6/9] file-posix: Support BDRV_REQ_ZERO_WRITE for truncate

On Mon 20 Apr 2020 03:32:11 PM CEST, Kevin Wolf wrote: > For regular files, we always get BDRV_REQ_ZERO_WRITE behaviour from the > OS, so we can advertise the flag and just ignore it. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 5/9] raw-format: Support BDRV_REQ_ZERO_WRITE for truncate

On Mon 20 Apr 2020 03:32:10 PM CEST, Kevin Wolf wrote: > The raw format driver can simply forward the flag and let its bs->file > child take care of actually providing the zeros. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

t L2 entries. If an external data file is in use, a write_zeroes > request to the data file is made instead. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 1/9] block: Add flags to BlockDriver.bdrv_co_truncate()

of truncate. > > For now, we always pass 0 and no drivers declare support for any flag. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 3/9] block-backend: Add flags to blk_truncate()

On Mon 20 Apr 2020 03:32:08 PM CEST, Kevin Wolf wrote: > Now that node level interface bdrv_truncate() supports passing request > flags to the block driver, expose this on the BlockBackend level, too. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 2/9] block: Add flags to bdrv(_co)_truncate()

On Mon 20 Apr 2020 03:32:07 PM CEST, Kevin Wolf wrote: > Now that block drivers can support flags for .bdrv_co_truncate, expose > the parameter in the node level interfaces bdrv_co_truncate() and > bdrv_truncate(). > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

Re: [PATCH v19 1/4] qcow2: introduce compression type feature

_change) > * add "compression type" for test output matching when it isn't filtered > affected tests: 049, 060, 061, 065, 144, 182, 242, 255 > > Signed-off-by: Denis Plotnikov > Reviewed-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Eric Blake > QAPI part: > Acked-by: Markus Armbruster Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 17/30] qcow2: Add subcluster support to calculate_l2_meta()

On Wed 15 Apr 2020 10:39:26 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> + * Returns 1 on success, -errno on failure (in order to match the >> + * return value of handle_copied() and handle_alloc()). > > Hmm, honestly, I don't like this idea. handle_copied and handle_alloc > has special return

Re: [PATCH v18 3/4] qcow2: add zstd cluster compression

output.pos <= SSIZE_MAX); The patch looks good to me, but why don't you assert this at the beginning of the function? You already know the buffer sizes... Either way, Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

On Fri 10 Apr 2020 11:29:59 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> Should we also document that extended L2 entries are incompatible >> with raw external files? [...] After all, when raw external file is >> enabled, the entire image is allocated, at which point subclusters >> don't make

[PATCH for-5.1] qcow2: Don't open images with a backing file and the data-file-raw bit

Although we cannot create these images with qemu-img it is still possible to do it using an external tool. QEMU should refuse to open them until the data-file-raw bit is cleared with 'qemu-img check'. Signed-off-by: Alberto Garcia --- block/qcow2.c | 39

Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

On Thu 09 Apr 2020 05:12:16 PM CEST, Eric Blake wrote: > Hmm - raw external files are incompatible with backing files. Pre-existing, but I just realized that we are not checking that in qcow2_do_open(), only on _create(). I suppose that if we find such an image we should either a) Show an

Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

On Tue 14 Apr 2020 08:06:38 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >>> In other words, cluster can't be unallocated with data file in use. >> >> I still don't follow... clusters can be unallocated, and when you >> create a new image they are indeed unallocated. > > with external data

Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

On Fri 10 Apr 2020 11:29:59 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> Hmm - raw external files are incompatible with backing files. Should >> we also document that extended L2 entries are incompatible with raw >> external files? (The text here reminded me about it, but it would be >> the

Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

On Tue 14 Apr 2020 06:19:18 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >>> It still may cache information about zeroed subclusters: gives more >>> detailed block-status. But we should mention somehow external >>> files. Hm. not only for raw external files, but it is documented that >>> cluster

Re: [PATCH v4 11/30] qcow2: Add l2_entry_size()

On Tue 14 Apr 2020 06:01:42 PM CEST, Eric Blake wrote: >>> And all occurrences of pure '8' (not many of them exist) >> >> I think most/all nowadays only refer to the number of bits per byte. > > CHAR_BIT (from ) is good for that. Wow, ok, I wonder if that actually makes the code more readable,

Re: [PATCH for-5.0] qcow2: Add incompatibility note between backing files and raw external data files

On Tue 14 Apr 2020 05:31:26 PM CEST, Kevin Wolf wrote: > I don't think this is critical for 5.0, so if I make a pull request > for other reasons, I'll include it, but if you agree, I won't send one > just for this patch. Sure, no problem. Berto

Re: [PATCH v4 14/30] qcow2: Add cluster type parameter to qcow2_get_host_offset()

On Tue 14 Apr 2020 02:30:30 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >> -ret = qcow2_get_host_offset(bs, offset, , ); >> -if (ret != QCOW2_CLUSTER_UNALLOCATED && >> -ret != QCOW2_CLUSTER_ZERO_PLAIN && >> -ret != QCOW2_CLUSTER_ZERO_ALLOC) { >> +

Re: [PATCH v4 11/30] qcow2: Add l2_entry_size()

On Tue 14 Apr 2020 02:29:13 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >>> Hmm. How to avoid it? Maybe, at least, refactor the code, to drop all >>> sizeof(uint64_t), converting them to L2_ENTRY_SIZE, L1_ENTRY_SIZE, >>> REFTABLE_ENTRY_SIZE etc? >> >> That wouldn't be a bad thing I guess but,

Re: [PATCH v4 11/30] qcow2: Add l2_entry_size()

On Tue 14 Apr 2020 11:44:57 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> /* allocate a new l2 entry */ >> >> -l2_offset = qcow2_alloc_clusters(bs, s->l2_size * sizeof(uint64_t)); >> +l2_offset = qcow2_alloc_clusters(bs, s->l2_size * l2_entry_size(s)); > > hmm. s->l2_size *

Re: [PATCH v4 30/30] iotests: Add tests for qcow2 images with extended L2 entries

On Thu 09 Apr 2020 02:22:37 PM CEST, Max Reitz wrote: >> +### Write subcluster #31-#34 (cluster overlap) ### > > #31-#34, I think. That's what I wrote :-? >> +### Partially zeroize an unallocated cluster (#3) >> +if [ "$use_backing_file" = "yes" ]; then >> +alloc="`seq 0

Re: [PATCH v4 27/30] qcow2: Assert that expand_zero_clusters_in_l1() does not support subclusters

On Thu 09 Apr 2020 12:27:36 PM CEST, Max Reitz wrote: >> +=== Testing version downgrade with extended L2 entries === >> + >> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 >> +qemu-img: Cannot downgrade an image with incompatible features 0x10 set > > This test fails in this commit,

Re: [PATCH v4 20/30] qcow2: Add subcluster support to discard_in_l2_slice()

On Thu 09 Apr 2020 12:05:12 PM CEST, Max Reitz wrote: >> switch (qcow2_get_cluster_type(bs, old_l2_entry)) { >> case QCOW2_CLUSTER_UNALLOCATED: >> -if (full_discard || !bs->backing) { >> +if (full_discard) { >> +/* If the image has extended

[PATCH for-5.0] qcow2: Add incompatibility note between backing files and raw external data files

Backing files and raw external data files are mutually exclusive. The documentation of the raw external data bit (in autoclear_features) already indicates that, but we should also mention it on the other side. Suggested-by: Eric Blake Signed-off-by: Alberto Garcia --- docs/interop/qcow2.txt

Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

On Thu 09 Apr 2020 05:12:16 PM CEST, Eric Blake wrote: > Hmm - raw external files are incompatible with backing files. Should > we also document that extended L2 entries are incompatible with raw > external files? Ok, I can also add additional checks to forbid creating such images. Berto

Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple clusters

On Thu 09 Apr 2020 08:39:12 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >> Because of this a test cannot expect that running the same commands on >> an empty image produces always the same results. >> >> Is this something that we should be concerned about? > > Parallel writing compressed

Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple clusters

On Mon 02 Dec 2019 01:15:05 PM CET, Andrey Shinkevich wrote: > +static coroutine_fn int > +qcow2_co_pwritev_compressed_part(BlockDriverState *bs, > + uint64_t offset, uint64_t bytes, > + QEMUIOVector *qiov, size_t qiov_offset) > +{ >

Re: [PATCH v4 05/30] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()

On Thu 09 Apr 2020 12:59:30 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >> static void calculate_l2_meta(BlockDriverState *bs, >> uint64_t host_cluster_offset, >> uint64_t guest_offset, unsigned bytes, >> -

Re: [PATCH v4 03/30] qcow2: Add calculate_l2_meta()

On Thu 09 Apr 2020 10:30:13 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> +static void calculate_l2_meta(BlockDriverState *bs, >> + uint64_t host_cluster_offset, >> + uint64_t guest_offset, unsigned bytes, >> +

Re: [PATCH v4 02/30] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

On Thu 09 Apr 2020 09:50:52 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> case QCOW2_CLUSTER_ZERO_PLAIN: >> case QCOW2_CLUSTER_UNALLOCATED: >> /* how many empty clusters ? */ >> c = count_contiguous_clusters_unallocated(bs, nb_clusters, >>

Re: [PATCH v4 02/30] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

On Thu 09 Apr 2020 09:57:59 AM CEST, Vladimir Sementsov-Ogievskiy wrote: > What about squashing this: > > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -615,32 +615,34 @@ int qcow2_get_host_offset(BlockDriverState *bs, > uint64_t offset, > break; > case

Re: [PATCH v4 13/30] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()

On Wed 08 Apr 2020 01:23:42 PM CEST, Max Reitz wrote: >> +switch (type) { >> +case QCOW2_CLUSTER_COMPRESSED: >> +return QCOW2_SUBCLUSTER_COMPRESSED; > > Why did you drop the check that l2_bitmap == 0 here? We don't generally check that reserved bits are 0. It would for

Re: [PATCH v4 18/30] qcow2: Add subcluster support to qcow2_get_host_offset()

On Wed 08 Apr 2020 02:49:14 PM CEST, Max Reitz wrote: > (Oops, totally missed the L1 entry out of bounds / L1 entry empty part > in v3.) Yeah, and you can mix values between different enum types in C quite easily without the compiler producing a warning. Berto

Re: [PATCH v4 02/30] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

On Wed 08 Apr 2020 12:51:24 PM CEST, Max Reitz wrote: >> -if (has_data_file(bs) && *cluster_offset != offset - >> offset_in_cluster) >> +if (has_data_file(bs) && *host_offset != offset - offset_in_cluster) >> { > > (1) The { should be moved to the preceding line; > > (2)

Re: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

I forgot to add the "for-5.0" tag in the subject, do I need to resend the patch? Berto

[PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

in 0d483dce38 Signed-off-by: Alberto Garcia --- block/qcow2.c | 5 + 1 file changed, 5 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 2bb536b014..587cf51948 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4349,6 +4349,11 @@ qcow2_co_pwritev_compressed_part(BlockDriver

[PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

: qcow2_co_pwritev_compressed_task: Assertion `bytes == s->cluster_size || (bytes < s->cluster_size && (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS))' failed. Aborted This patch fixes a regression introduced in 0d483dce38 Signed-off-by: Alberto

[PATCH v5] qcow2: Forbid discard in qcow2 v2 images with backing files

sed by qcow2_snapshot_create() to discard the clusters used by the VM state. In this case there's no risk of exposing stale data to the guest and we really want that the clusters are always discarded. Signed-off-by: Alberto Garcia --- v5: - Fix iotests 046 and 177 with compat=0.10 [Max] v4: - Show out

Re: [PATCH v4] qcow2: Forbid discard in qcow2 v2 images with backing files

On Tue 31 Mar 2020 10:57:18 AM CEST, Max Reitz wrote: > I’ll have to dequeue it again, because it breaks iotests 046 and 177 > (both of which already have special handling for v2-specific discard; > but it needs to be adjusted now that the discard operation no longer > reveals the backing file

Re: [PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files

On Fri 27 Mar 2020 07:57:40 PM CET, Eric Blake wrote: >> +/* If the image does not support QCOW_OFLAG_ZERO then discarding >> + * clusters could expose stale data from the backing file. */ >> +if (s->qcow_version < 3 && bs->backing) { >> +return -ENOTSUP; >> +} > > Hmm.

[PATCH v4] qcow2: Forbid discard in qcow2 v2 images with backing files

sed by qcow2_snapshot_create() to discard the clusters used by the VM state. In this case there's no risk of exposing stale data to the guest and we really want that the clusters are always discarded. Signed-off-by: Alberto Garcia --- v4: - Show output of qemu-img map when there's no backing file [Eric]

Re: [PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files

On Fri 27 Mar 2020 07:13:04 PM CET, Eric Blake wrote: >> +for qcow2_compat in 0.10 1.1; do >> +echo "# Create an image with compat=$qcow2_compat without a backing >> file" >> +_make_test_img -o "compat=$qcow2_compat" 128k >> + >> +echo "# Fill all clusters with data and then discard

[PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files

sed by qcow2_snapshot_create() to discard the clusters used by the VM state. In this case there's no risk of exposing stale data to the guest and we really want that the clusters are always discarded. Signed-off-by: Alberto Garcia --- v3: - Rebase and change iotest number - Show output of qemu-img map in iot

Re: [PATCH v2] qcow2: Forbid discard in qcow2 v2 images with backing files

On Fri 27 Mar 2020 03:14:42 PM CET, Kevin Wolf wrote: >> +echo "# Create a backing image and fill it with data" >> +BACKING_IMG="$TEST_IMG.base" >> +TEST_IMG="$BACKING_IMG" _make_test_img 128k >> +$QEMU_IO -c 'write -P 0xff 0 128k' "$BACKING_IMG" | _filter_qemu_io >> + >> +for qcow2_compat in 0.10

Re: [PATCH] qcow2: Forbid discard in qcow2 v2 images with backing files

On Tue 24 Mar 2020 03:46:07 PM CET, Eric Blake wrote: >> -$QEMU_IO -c 'discard 0k 64k' "$TEST_IMG" | _filter_qemu_io >> +poke_file "$TEST_IMG" '262144' "\x00\x00\x00\x00\x00\x00\x00\x00" # 0x4 >> - L2 entry >> +poke_file "$TEST_IMG" '131082' "\x00\x00" # 0x2000a - Refcount entry > > Instead

[PATCH v2] qcow2: Forbid discard in qcow2 v2 images with backing files

sed by qcow2_snapshot_create() to discard the clusters used by the VM state. In this case there's no risk of exposing stale data to the guest and we really want that the clusters are always discarded. Signed-off-by: Alberto Garcia --- v2: - Don't create the image with compat=0.10 in iotest 060 [Max] -

[PATCH] qcow2: Forbid discard in qcow2 v2 images with backing files

sed by qcow2_snapshot_create() to discard the clusters used by the VM state. In this case there's no risk of exposing stale data to the guest and we really want that the clusters are always discarded. Signed-off-by: Alberto Garcia --- block/qcow2.c | 6 +++ tests/qemu-iotests/060 |

<    1   2   3   4   5   6   7   8   9   10   >