| Message ID | 20190429105741.31033-1-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: Fix qcow2_make_empty() with external data file | expand |
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
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>
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>
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
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 >
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
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,
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(-)