[Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Kevin Wolf]

make_completely_empty() is an optimisated path for bdrv_make_empty()
where completely new metadata is created inside the image file instead
of going through all clusters and discarding them. For an external data
file, however, we actually need to do discard operations on the data
file; just overwriting the qcow2 file doesn't get rid of the data.

The necessary slow path with an explicit discard operation already
exists for other cases. Use it for external data files, too.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 7fbef97aab..097fde56f9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4384,7 +4384,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
 
     if (s->qcow_version >= 3 && !s->snapshots && !s->nb_bitmaps &&
         3 + l1_clusters <= s->refcount_block_size &&
-        s->crypt_method_header != QCOW_CRYPT_LUKS) {
+        s->crypt_method_header != QCOW_CRYPT_LUKS &&
+        !has_data_file(bs)) {
         /* The following function only works for qcow2 v3 images (it
          * requires the dirty flag) and only as long as there are no
          * features that reserve extra clusters (such as snapshots,
-- 
2.20.1

[Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Kevin Wolf]

make_completely_empty() is an optimisated path for bdrv_make_empty()
where completely new metadata is created inside the image file instead
of going through all clusters and discarding them. For an external data
file, however, we actually need to do discard operations on the data
file; just overwriting the qcow2 file doesn't get rid of the data.

The necessary slow path with an explicit discard operation already
exists for other cases. Use it for external data files, too.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 7fbef97aab..097fde56f9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4384,7 +4384,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
 
     if (s->qcow_version >= 3 && !s->snapshots && !s->nb_bitmaps &&
         3 + l1_clusters <= s->refcount_block_size &&
-        s->crypt_method_header != QCOW_CRYPT_LUKS) {
+        s->crypt_method_header != QCOW_CRYPT_LUKS &&
+        !has_data_file(bs)) {
         /* The following function only works for qcow2 v3 images (it
          * requires the dirty flag) and only as long as there are no
          * features that reserve extra clusters (such as snapshots,
-- 
2.20.1

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Kevin Wolf]

Am 29.04.2019 um 12:57 hat Kevin Wolf geschrieben:
> make_completely_empty() is an optimisated path for bdrv_make_empty()
> where completely new metadata is created inside the image file instead
> of going through all clusters and discarding them. For an external data
> file, however, we actually need to do discard operations on the data
> file; just overwriting the qcow2 file doesn't get rid of the data.
> 
> The necessary slow path with an explicit discard operation already
> exists for other cases. Use it for external data files, too.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7fbef97aab..097fde56f9 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -4384,7 +4384,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
>  
>      if (s->qcow_version >= 3 && !s->snapshots && !s->nb_bitmaps &&
>          3 + l1_clusters <= s->refcount_block_size &&
> -        s->crypt_method_header != QCOW_CRYPT_LUKS) {
> +        s->crypt_method_header != QCOW_CRYPT_LUKS &&
> +        !has_data_file(bs)) {
>          /* The following function only works for qcow2 v3 images (it
>           * requires the dirty flag) and only as long as there are no
>           * features that reserve extra clusters (such as snapshots,

Oops, I hadn't everything committed yet. I'll add a comment change as
well:

          * LUKS header, or persistent bitmaps), because it completely
          * empties the image.  Furthermore, the L1 table and three
          * additional clusters (image header, refcount table, one
-         * refcount block) have to fit inside one refcount block. */
+         * refcount block) have to fit inside one refcount block. It
+         * only resets the image file, i.e. does not work with an
+         * external data file. */
         return make_completely_empty(bs);
     }

Kevin

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Kevin Wolf]

Am 29.04.2019 um 12:57 hat Kevin Wolf geschrieben:
> make_completely_empty() is an optimisated path for bdrv_make_empty()
> where completely new metadata is created inside the image file instead
> of going through all clusters and discarding them. For an external data
> file, however, we actually need to do discard operations on the data
> file; just overwriting the qcow2 file doesn't get rid of the data.
> 
> The necessary slow path with an explicit discard operation already
> exists for other cases. Use it for external data files, too.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7fbef97aab..097fde56f9 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -4384,7 +4384,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
>  
>      if (s->qcow_version >= 3 && !s->snapshots && !s->nb_bitmaps &&
>          3 + l1_clusters <= s->refcount_block_size &&
> -        s->crypt_method_header != QCOW_CRYPT_LUKS) {
> +        s->crypt_method_header != QCOW_CRYPT_LUKS &&
> +        !has_data_file(bs)) {
>          /* The following function only works for qcow2 v3 images (it
>           * requires the dirty flag) and only as long as there are no
>           * features that reserve extra clusters (such as snapshots,

Oops, I hadn't everything committed yet. I'll add a comment change as
well:

          * LUKS header, or persistent bitmaps), because it completely
          * empties the image.  Furthermore, the L1 table and three
          * additional clusters (image header, refcount table, one
-         * refcount block) have to fit inside one refcount block. */
+         * refcount block) have to fit inside one refcount block. It
+         * only resets the image file, i.e. does not work with an
+         * external data file. */
         return make_completely_empty(bs);
     }

Kevin

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Eric Blake]

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

On 4/29/19 6:21 AM, Kevin Wolf wrote:
> Am 29.04.2019 um 12:57 hat Kevin Wolf geschrieben:
>> make_completely_empty() is an optimisated path for bdrv_make_empty()
>> where completely new metadata is created inside the image file instead
>> of going through all clusters and discarding them. For an external data
>> file, however, we actually need to do discard operations on the data
>> file; just overwriting the qcow2 file doesn't get rid of the data.
>>
>> The necessary slow path with an explicit discard operation already
>> exists for other cases. Use it for external data files, too.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block/qcow2.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)

> Oops, I hadn't everything committed yet. I'll add a comment change as
> well:

With the comment change squashed in,
Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Eric Blake]

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

On 4/29/19 6:21 AM, Kevin Wolf wrote:
> Am 29.04.2019 um 12:57 hat Kevin Wolf geschrieben:
>> make_completely_empty() is an optimisated path for bdrv_make_empty()
>> where completely new metadata is created inside the image file instead
>> of going through all clusters and discarding them. For an external data
>> file, however, we actually need to do discard operations on the data
>> file; just overwriting the qcow2 file doesn't get rid of the data.
>>
>> The necessary slow path with an explicit discard operation already
>> exists for other cases. Use it for external data files, too.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block/qcow2.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)

> Oops, I hadn't everything committed yet. I'll add a comment change as
> well:

With the comment change squashed in,
Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Stefan Hajnoczi]

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

On Mon, Apr 29, 2019 at 12:57:41PM +0200, Kevin Wolf wrote:
> make_completely_empty() is an optimisated path for bdrv_make_empty()
> where completely new metadata is created inside the image file instead
> of going through all clusters and discarding them. For an external data
> file, however, we actually need to do discard operations on the data
> file; just overwriting the qcow2 file doesn't get rid of the data.
> 
> The necessary slow path with an explicit discard operation already
> exists for other cases. Use it for external data files, too.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Stefan Hajnoczi]

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

On Mon, Apr 29, 2019 at 12:57:41PM +0200, Kevin Wolf wrote:
> make_completely_empty() is an optimisated path for bdrv_make_empty()
> where completely new metadata is created inside the image file instead
> of going through all clusters and discarding them. For an external data
> file, however, we actually need to do discard operations on the data
> file; just overwriting the qcow2 file doesn't get rid of the data.
> 
> The necessary slow path with an explicit discard operation already
> exists for other cases. Use it for external data files, too.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190429105741.31033-1-kwolf@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190429105741.31033-1-kwolf@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190429105741.31033-1-kwolf@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190429105741.31033-1-kwolf@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Eric Blake]

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

On 4/30/19 8:54 AM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190429105741.31033-1-kwolf@redhat.com/
> 
> 
> 
> Hi,
> 
> This series failed the docker-mingw@fedora build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
> 
> 
> 
> The full log is available at

Patchew trimmed a bit too much; also, the error appears to be transient
and unrelated to the patch at hand. Quoting from:

> http://patchew.org/logs/20190429105741.31033-1-kwolf@redhat.com/testing.docker-mingw@fedora/?type=message.



Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: git fetch_pack: expected ACK/NAK, got 'ERR upload-pack: not our
ref 1cd70b1217a3e02617dbba76d15d21be1e8e4aa0'
fatal: The remote end hung up unexpectedly
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
  File "patchew-tester/src/patchew-cli", line 521, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf, True)
  File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo
    stdout=logf, stderr=logf)
  File "/usr/lib64/python3.4/subprocess.py", line 558, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f',
'--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384',
'https://github.com/patchew-project/qemu']' returned non-zero exit status 1


> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[Eric Blake]

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

On 4/30/19 8:54 AM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190429105741.31033-1-kwolf@redhat.com/
> 
> 
> 
> Hi,
> 
> This series failed the docker-mingw@fedora build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
> 
> 
> 
> The full log is available at

Patchew trimmed a bit too much; also, the error appears to be transient
and unrelated to the patch at hand. Quoting from:

> http://patchew.org/logs/20190429105741.31033-1-kwolf@redhat.com/testing.docker-mingw@fedora/?type=message.



Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: git fetch_pack: expected ACK/NAK, got 'ERR upload-pack: not our
ref 1cd70b1217a3e02617dbba76d15d21be1e8e4aa0'
fatal: The remote end hung up unexpectedly
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
  File "patchew-tester/src/patchew-cli", line 521, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf, True)
  File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo
    stdout=logf, stderr=logf)
  File "/usr/lib64/python3.4/subprocess.py", line 558, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f',
'--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384',
'https://github.com/patchew-project/qemu']' returned non-zero exit status 1


> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190429105741.31033-1-kwolf@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190429105741.31033-1-kwolf@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH] qcow2: Fix qcow2_make_empty() with external data file

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190429105741.31033-1-kwolf@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190429105741.31033-1-kwolf@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com