From: Stefan Hajnoczi <stefanha@redhat.com>

The qcow2 .bdrv_measure() code calculates the crypto payload offset.

This logic really belongs in crypto/block.c where it can be reused by

other image formats.

The "luks" block driver will need this same logic in order to implement

.bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()

function now.

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

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200221112522.1497712-2-stefanha@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2.c | 74 +++++++++++-------------------------------

crypto/block.c | 36 ++++++++++++++++++++

include/crypto/block.h | 22 +++++++++++++

3 files changed, 77 insertions(+), 55 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)

return ret;

}

-static ssize_t qcow2_measure_crypto_hdr_init_func(QCryptoBlock *block,

- size_t headerlen, void *opaque, Error **errp)

-{

- size_t *headerlenp = opaque;

-

- /* Stash away the payload size */

- *headerlenp = headerlen;

- return 0;

-}

-

-static ssize_t qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,

- size_t offset, const uint8_t *buf, size_t buflen,

- void *opaque, Error **errp)

-{

- /* Discard the bytes, we're not actually writing to an image */

- return buflen;

-}

-

-/* Determine the number of bytes for the LUKS payload */

-static bool qcow2_measure_luks_headerlen(QemuOpts *opts, size_t *len,

- Error **errp)

-{

- QDict *opts_qdict;

- QDict *cryptoopts_qdict;

- QCryptoBlockCreateOptions *cryptoopts;

- QCryptoBlock *crypto;

-

- /* Extract "encrypt." options into a qdict */

- opts_qdict = qemu_opts_to_qdict(opts, NULL);

- qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");

- qobject_unref(opts_qdict);

-

- /* Build QCryptoBlockCreateOptions object from qdict */

- qdict_put_str(cryptoopts_qdict, "format", "luks");

- cryptoopts = block_crypto_create_opts_init(cryptoopts_qdict, errp);

- qobject_unref(cryptoopts_qdict);

- if (!cryptoopts) {

- return false;

- }

-

- /* Fake LUKS creation in order to determine the payload size */

- crypto = qcrypto_block_create(cryptoopts, "encrypt.",

- qcow2_measure_crypto_hdr_init_func,

- qcow2_measure_crypto_hdr_write_func,

- len, errp);

- qapi_free_QCryptoBlockCreateOptions(cryptoopts);

- if (!crypto) {

- return false;

- }

-

- qcrypto_block_free(crypto);

- return true;

-}

-

static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,

Error **errp)

{

@@ -XXX,XX +XXX,XX @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,

g_free(optstr);

if (has_luks) {

+ g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;

+ QDict *opts_qdict;

+ QDict *cryptoopts;

size_t headerlen;

- if (!qcow2_measure_luks_headerlen(opts, &headerlen, &local_err)) {

+ opts_qdict = qemu_opts_to_qdict(opts, NULL);

+ qdict_extract_subqdict(opts_qdict, &cryptoopts, "encrypt.");

+ qobject_unref(opts_qdict);

+

+ qdict_put_str(cryptoopts, "format", "luks");

+

+ create_opts = block_crypto_create_opts_init(cryptoopts, errp);

+ qobject_unref(cryptoopts);

+ if (!create_opts) {

+ goto err;

+ }

+

+ if (!qcrypto_block_calculate_payload_offset(create_opts,

+ "encrypt.",

+ &headerlen,

+ &local_err)) {

goto err;

}

diff --git a/crypto/block.c b/crypto/block.c

index XXXXXXX..XXXXXXX 100644

--- a/crypto/block.c

+++ b/crypto/block.c

@@ -XXX,XX +XXX,XX @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,

}

+static ssize_t qcrypto_block_headerlen_hdr_init_func(QCryptoBlock *block,

+ size_t headerlen, void *opaque, Error **errp)

+{

+ size_t *headerlenp = opaque;

+

+ /* Stash away the payload size */

+ *headerlenp = headerlen;

+ return 0;

+}

+

+

+static ssize_t qcrypto_block_headerlen_hdr_write_func(QCryptoBlock *block,

+ size_t offset, const uint8_t *buf, size_t buflen,

+ void *opaque, Error **errp)

+{

+ /* Discard the bytes, we're not actually writing to an image */

+ return buflen;

+}

+

+

+bool

+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,

+ const char *optprefix,

+ size_t *len,

+ Error **errp)

+{

+ /* Fake LUKS creation in order to determine the payload size */

+ g_autoptr(QCryptoBlock) crypto =

+ qcrypto_block_create(create_opts, optprefix,

+ qcrypto_block_headerlen_hdr_init_func,

+ qcrypto_block_headerlen_hdr_write_func,

+ len, errp);

+ return crypto != NULL;

+}

+

+

QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,

Error **errp)

{

diff --git a/include/crypto/block.h b/include/crypto/block.h

index XXXXXXX..XXXXXXX 100644

--- a/include/crypto/block.h

+++ b/include/crypto/block.h

@@ -XXX,XX +XXX,XX @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,

Error **errp);

+/**

+ * qcrypto_block_calculate_payload_offset:

+ * @create_opts: the encryption options

+ * @optprefix: name prefix for options

+ * @len: output for number of header bytes before payload

+ * @errp: pointer to a NULL-initialized error object

+ *

+ * Calculate the number of header bytes before the payload in an encrypted

+ * storage volume. The header is an area before the payload that is reserved

+ * for encryption metadata.

+ *

+ * Returns: true on success, false on error

+ */

+bool

+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,

+ const char *optprefix,

+ size_t *len,

+ Error **errp);

+

+

/**

* qcrypto_block_get_info:

* @block: the block encryption object

@@ -XXX,XX +XXX,XX @@ uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block);

void qcrypto_block_free(QCryptoBlock *block);

G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlock, qcrypto_block_free)

+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlockCreateOptions,

+ qapi_free_QCryptoBlockCreateOptions)

#endif /* QCRYPTO_BLOCK_H */

--

2.24.1

From: Stefan Hajnoczi <stefanha@redhat.com>

Add qemu-img measure support in the "luks" block driver.

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

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200221112522.1497712-3-stefanha@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/crypto.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++

1 file changed, 62 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c

index XXXXXXX..XXXXXXX 100644

--- a/block/crypto.c

+++ b/block/crypto.c

@@ -XXX,XX +XXX,XX @@ static int64_t block_crypto_getlength(BlockDriverState *bs)

}

+static BlockMeasureInfo *block_crypto_measure(QemuOpts *opts,

+ BlockDriverState *in_bs,

+ Error **errp)

+{

+ g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;

+ Error *local_err = NULL;

+ BlockMeasureInfo *info;

+ uint64_t size;

+ size_t luks_payload_size;

+ QDict *cryptoopts;

+

+ /*

+ * Preallocation mode doesn't affect size requirements but we must consume

+ * the option.

+ */

+ g_free(qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC));

+

+ size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);

+

+ if (in_bs) {

+ int64_t ssize = bdrv_getlength(in_bs);

+

+ if (ssize < 0) {

+ error_setg_errno(&local_err, -ssize,

+ "Unable to get image virtual_size");

+ goto err;

+ }

+

+ size = ssize;

+ }

+

+ cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,

+ &block_crypto_create_opts_luks, true);

+ qdict_put_str(cryptoopts, "format", "luks");

+ create_opts = block_crypto_create_opts_init(cryptoopts, &local_err);

+ qobject_unref(cryptoopts);

+ if (!create_opts) {

+ goto err;

+ }

+

+ if (!qcrypto_block_calculate_payload_offset(create_opts, NULL,

+ &luks_payload_size,

+ &local_err)) {

+ goto err;

+ }

+

+ /*

+ * Unallocated blocks are still encrypted so allocation status makes no

+ * difference to the file size.

+ */

+ info = g_new(BlockMeasureInfo, 1);

+ info->fully_allocated = luks_payload_size + size;

+ info->required = luks_payload_size + size;

+ return info;

+

+err:

+ error_propagate(errp, local_err);

+ return NULL;

+}

+

+

static int block_crypto_probe_luks(const uint8_t *buf,

int buf_size,

const char *filename) {

@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_crypto_luks = {

.bdrv_co_preadv = block_crypto_co_preadv,

.bdrv_co_pwritev = block_crypto_co_pwritev,

.bdrv_getlength = block_crypto_getlength,

+ .bdrv_measure = block_crypto_measure,

.bdrv_get_info = block_crypto_get_info_luks,

.bdrv_get_specific_info = block_crypto_get_specific_info_luks,

--

2.24.1

From: Stefan Hajnoczi <stefanha@redhat.com>

In most qemu-img sub-commands the --object option only makes sense when

there is a filename. qemu-img measure is an exception because objects

may be referenced from the image creation options instead of an existing

image file. Allow --object without a filename.

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

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200221112522.1497712-4-stefanha@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

qemu-img.c | 6 ++----

tests/qemu-iotests/178 | 2 +-

tests/qemu-iotests/178.out.qcow2 | 8 ++++----

tests/qemu-iotests/178.out.raw | 8 ++++----

4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static int img_measure(int argc, char **argv)

filename = argv[optind];

}

- if (!filename &&

- (object_opts || image_opts || fmt || snapshot_name || sn_opts)) {

- error_report("--object, --image-opts, -f, and -l "

- "require a filename argument.");

+ if (!filename && (image_opts || fmt || snapshot_name || sn_opts)) {

+ error_report("--image-opts, -f, and -l require a filename argument.");

goto out;

}

if (filename && img_size != UINT64_MAX) {

diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/178

+++ b/tests/qemu-iotests/178

@@ -XXX,XX +XXX,XX @@ _make_test_img 1G

$QEMU_IMG measure # missing arguments

$QEMU_IMG measure --size 2G "$TEST_IMG" # only one allowed

$QEMU_IMG measure "$TEST_IMG" a # only one filename allowed

-$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 # missing filename

+$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 # size or filename needed

$QEMU_IMG measure --image-opts # missing filename

$QEMU_IMG measure -f qcow2 # missing filename

$QEMU_IMG measure -l snap1 # missing filename

diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/178.out.qcow2

+++ b/tests/qemu-iotests/178.out.qcow2

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824

qemu-img: Either --size N or one filename must be specified.

qemu-img: --size N cannot be used together with a filename.

qemu-img: At most one filename argument is allowed.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

+qemu-img: Either --size N or one filename must be specified.

+qemu-img: --image-opts, -f, and -l require a filename argument.

+qemu-img: --image-opts, -f, and -l require a filename argument.

+qemu-img: --image-opts, -f, and -l require a filename argument.

qemu-img: Invalid option list: ,

qemu-img: Invalid parameter 'snapshot.foo'

qemu-img: Failed in parsing snapshot param 'snapshot.foo'

diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/178.out.raw

+++ b/tests/qemu-iotests/178.out.raw

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824

qemu-img: Either --size N or one filename must be specified.

qemu-img: --size N cannot be used together with a filename.

qemu-img: At most one filename argument is allowed.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

-qemu-img: --object, --image-opts, -f, and -l require a filename argument.

+qemu-img: Either --size N or one filename must be specified.

+qemu-img: --image-opts, -f, and -l require a filename argument.

+qemu-img: --image-opts, -f, and -l require a filename argument.

+qemu-img: --image-opts, -f, and -l require a filename argument.

qemu-img: Invalid option list: ,

qemu-img: Invalid parameter 'snapshot.foo'

qemu-img: Failed in parsing snapshot param 'snapshot.foo'

--

2.24.1

From: Stefan Hajnoczi <stefanha@redhat.com>

This test exercises the block/crypto.c "luks" block driver

.bdrv_measure() code.

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

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200221112522.1497712-5-stefanha@redhat.com>

[mreitz: Renamed test from 282 to 288]

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/288 | 93 ++++++++++++++++++++++++++++++++++++++

tests/qemu-iotests/288.out | 30 ++++++++++++

tests/qemu-iotests/group | 1 +

3 files changed, 124 insertions(+)

create mode 100755 tests/qemu-iotests/288

create mode 100644 tests/qemu-iotests/288.out

diff --git a/tests/qemu-iotests/288 b/tests/qemu-iotests/288

new file mode 100755

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/288

@@ -XXX,XX +XXX,XX @@

+#!/usr/bin/env bash

+#

+# qemu-img measure tests for LUKS images

+#

+# Copyright (C) 2020 Red Hat, Inc.

+#

+# This program is free software; you can redistribute it and/or modify

+# it under the terms of the GNU General Public License as published by

+# the Free Software Foundation; either version 2 of the License, or

+# (at your option) any later version.

+#

+# This program is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

+# GNU General Public License for more details.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program. If not, see <http://www.gnu.org/licenses/>.

+#

+

+# creator

+owner=stefanha@redhat.com

+

+seq=`basename $0`

+echo "QA output created by $seq"

+

+status=1 # failure is the default!

+

+_cleanup()

+{

+ _cleanup_test_img

+ rm -f "$TEST_IMG.converted"

+}

+trap "_cleanup; exit \$status" 0 1 2 3 15

+

+# get standard environment, filters and checks

+. ./common.rc

+. ./common.filter

+. ./common.pattern

+

+_supported_fmt luks

+_supported_proto file

+_supported_os Linux

+

+SECRET=secret,id=sec0,data=passphrase

+

+echo "== measure 1G image file =="

+echo

+

+$QEMU_IMG measure --object "$SECRET" \

+     -O "$IMGFMT" \

+         -o key-secret=sec0,iter-time=10 \

+         --size 1G

+

+echo

+echo "== create 1G image file (size should be no greater than measured) =="

+echo

+

+_make_test_img 1G

+stat -c "image file size in bytes: %s" "$TEST_IMG_FILE"

+

+echo

+echo "== modified 1G image file (size should be no greater than measured) =="

+echo

+

+$QEMU_IO --object "$SECRET" --image-opts "$TEST_IMG" -c "write -P 0x51 0x10000 0x400" | _filter_qemu_io | _filter_testdir

+stat -c "image file size in bytes: %s" "$TEST_IMG_FILE"

+

+echo

+echo "== measure preallocation=falloc 1G image file =="

+echo

+

+$QEMU_IMG measure --object "$SECRET" \

+     -O "$IMGFMT" \

+         -o key-secret=sec0,iter-time=10,preallocation=falloc \

+         --size 1G

+

+echo

+echo "== measure with input image file =="

+echo

+

+IMGFMT=raw IMGKEYSECRET= IMGOPTS= _make_test_img 1G | _filter_imgfmt

+QEMU_IO_OPTIONS= IMGOPTSSYNTAX= $QEMU_IO -f raw -c "write -P 0x51 0x10000 0x400" "$TEST_IMG_FILE" | _filter_qemu_io | _filter_testdir

+$QEMU_IMG measure --object "$SECRET" \

+     -O "$IMGFMT" \

+         -o key-secret=sec0,iter-time=10 \

+         -f raw \

+         "$TEST_IMG_FILE"

+

+# success, all done

+echo "*** done"

+rm -f $seq.full

+status=0

diff --git a/tests/qemu-iotests/288.out b/tests/qemu-iotests/288.out

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/288.out

@@ -XXX,XX +XXX,XX @@

+QA output created by 288

+== measure 1G image file ==

+

+required size: 1075810304

+fully allocated size: 1075810304

+

+== create 1G image file (size should be no greater than measured) ==

+

+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824

+image file size in bytes: 1075810304

+

+== modified 1G image file (size should be no greater than measured) ==

+

+wrote 1024/1024 bytes at offset 65536

+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

+image file size in bytes: 1075810304

+

+== measure preallocation=falloc 1G image file ==

+

+required size: 1075810304

+fully allocated size: 1075810304

+

+== measure with input image file ==

+

+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824

+wrote 1024/1024 bytes at offset 65536

+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

+required size: 1075810304

+fully allocated size: 1075810304

+*** done

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/group

+++ b/tests/qemu-iotests/group

@@ -XXX,XX +XXX,XX @@

283 auto quick

284 rw

286 rw quick

+288 quick

--

2.24.1

From: David Edmondson <david.edmondson@oracle.com>

RFC 7230 section 3.2 indicates that whitespace is permitted between

the field name and field value and after the field value.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>

Message-Id: <20200224101310.101169-2-david.edmondson@oracle.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/curl.c | 31 +++++++++++++++++++++++++++----

1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/block/curl.c b/block/curl.c

index XXXXXXX..XXXXXXX 100644

--- a/block/curl.c

+++ b/block/curl.c

@@ -XXX,XX +XXX,XX @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)

{

BDRVCURLState *s = opaque;

size_t realsize = size * nmemb;

- const char *accept_line = "Accept-Ranges: bytes";

+ const char *header = (char *)ptr;

+ const char *end = header + realsize;

+ const char *accept_ranges = "Accept-Ranges:";

+ const char *bytes = "bytes";

- if (realsize >= strlen(accept_line)

- && strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) {

- s->accept_range = true;

+ if (realsize >= strlen(accept_ranges)

+ && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) {

+

+ char *p = strchr(header, ':') + 1;

+

+ /* Skip whitespace between the header name and value. */

+ while (p < end && *p && g_ascii_isspace(*p)) {

+ p++;

+ }

+

+ if (end - p >= strlen(bytes)

+ && strncmp(p, bytes, strlen(bytes)) == 0) {

+

+ /* Check that there is nothing but whitespace after the value. */

+ p += strlen(bytes);

+ while (p < end && *p && g_ascii_isspace(*p)) {

+ p++;

+ }

+

+ if (p == end || !*p) {

+ s->accept_range = true;

+ }

+ }

}

return realsize;

--

2.24.1

From: David Edmondson <david.edmondson@oracle.com>

RFC 7230 section 3.2 indicates that HTTP header field names are case

insensitive.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>

Message-Id: <20200224101310.101169-3-david.edmondson@oracle.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/curl.c | 5 +++--

1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/curl.c b/block/curl.c

index XXXXXXX..XXXXXXX 100644

--- a/block/curl.c

+++ b/block/curl.c

@@ -XXX,XX +XXX,XX @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)

size_t realsize = size * nmemb;

const char *header = (char *)ptr;

const char *end = header + realsize;

- const char *accept_ranges = "Accept-Ranges:";

+ const char *accept_ranges = "accept-ranges:";

const char *bytes = "bytes";

if (realsize >= strlen(accept_ranges)

- && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) {

+ && g_ascii_strncasecmp(header, accept_ranges,

+ strlen(accept_ranges)) == 0) {

char *p = strchr(header, ':') + 1;

--

2.24.1

From: Eric Blake <eblake@redhat.com>

Tests 261 and 272 fail on RHEL 7 with coreutils 8.22, since od

--endian was not added until coreutils 8.23. Fix this by manually

constructing the final value one byte at a time.

Fixes: fc8ba423

Reported-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200226125424.481840-1-eblake@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/common.rc | 22 +++++++++++++++++-----

1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/common.rc

+++ b/tests/qemu-iotests/common.rc

@@ -XXX,XX +XXX,XX @@ poke_file()

# peek_file_le 'test.img' 512 2 => 65534

peek_file_le()

{

- # Wrap in echo $() to strip spaces

- echo $(od -j"$2" -N"$3" --endian=little -An -vtu"$3" "$1")

+ local val=0 shift=0 byte

+

+ # coreutils' od --endian is not portable, so manually assemble bytes.

+ for byte in $(od -j"$2" -N"$3" -An -v -tu1 "$1"); do

+ val=$(( val | (byte << shift) ))

+ shift=$((shift + 8))

+ done

+ printf %llu $val

}

# peek_file_be 'test.img' 512 2 => 65279

peek_file_be()

{

- # Wrap in echo $() to strip spaces

- echo $(od -j"$2" -N"$3" --endian=big -An -vtu"$3" "$1")

+ local val=0 byte

+

+ # coreutils' od --endian is not portable, so manually assemble bytes.

+ for byte in $(od -j"$2" -N"$3" -An -v -tu1 "$1"); do

+ val=$(( (val << 8) | byte ))

+ done

+ printf %llu $val

}

-# peek_file_raw 'test.img' 512 2 => '\xff\xfe'

+# peek_file_raw 'test.img' 512 2 => '\xff\xfe'. Do not use if the raw data

+# is likely to contain \0 or trailing \n.

peek_file_raw()

{

dd if="$1" bs=1 skip="$2" count="$3" status=none

--

2.24.1

From: Pan Nengyuan <pannengyuan@huawei.com>

'crypto_opts' forgot to free in qcow2_close(), this patch fix the bellow leak stack:

Direct leak of 24 byte(s) in 1 object(s) allocated from:

#0 0x7f0edd81f970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)

#1 0x7f0edc6d149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)

#2 0x55d7eaede63d in qobject_input_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qobject-input-visitor.c:295

#3 0x55d7eaed78b8 in visit_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qapi-visit-core.c:49

#4 0x55d7eaf5140b in visit_type_QCryptoBlockOpenOptions qapi/qapi-visit-crypto.c:290

#5 0x55d7eae43af3 in block_crypto_open_opts_init /mnt/sdb/qemu-new/qemu_test/qemu/block/crypto.c:163

#6 0x55d7eacd2924 in qcow2_update_options_prepare /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1148

#7 0x55d7eacd33f7 in qcow2_update_options /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1232

#8 0x55d7eacd9680 in qcow2_do_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1512

#9 0x55d7eacdc55e in qcow2_open_entry /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1792

#10 0x55d7eacdc8fe in qcow2_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1819

#11 0x55d7eac3742d in bdrv_open_driver /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1317

#12 0x55d7eac3e990 in bdrv_open_common /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1575

#13 0x55d7eac4442c in bdrv_open_inherit /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3126

#14 0x55d7eac45c3f in bdrv_open /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3219

#15 0x55d7ead8e8a4 in blk_new_open /mnt/sdb/qemu-new/qemu_test/qemu/block/block-backend.c:397

#16 0x55d7eacde74c in qcow2_co_create /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3534

#17 0x55d7eacdfa6d in qcow2_co_create_opts /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3668

#18 0x55d7eac1c678 in bdrv_create_co_entry /mnt/sdb/qemu-new/qemu_test/qemu/block.c:485

#19 0x55d7eb0024d2 in coroutine_trampoline /mnt/sdb/qemu-new/qemu_test/qemu/util/coroutine-ucontext.c:115

Reported-by: Euler Robot <euler.robot@huawei.com>

Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200227012950.12256-2-pannengyuan@huawei.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/block/qcow2.c b/block/qcow2.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ static void qcow2_close(BlockDriverState *bs)

qcrypto_block_free(s->crypto);

s->crypto = NULL;

+ qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);

g_free(s->unknown_header_fields);

cleanup_unknown_header_ext(bs);

--

2.24.1

From: Pan Nengyuan <pannengyuan@huawei.com>

collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory.

It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan.

Reported-by: Euler Robot <euler.robot@huawei.com>

Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Message-Id: <20200227012950.12256-3-pannengyuan@huawei.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

qemu-img.c | 2 ++

1 file changed, 2 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static int img_check(int argc, char **argv)

check->corruptions_fixed);

}

+ qapi_free_ImageCheck(check);

+ check = g_new0(ImageCheck, 1);

ret = collect_image_check(bs, check, filename, fmt, 0);

check->leaks_fixed = leaks_fixed;

--

2.24.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

On success path we return what inflate() returns instead of 0. And it

most probably works for Z_STREAM_END as it is positive, but is

definitely broken for Z_BUF_ERROR.

While being here, switch to errno return code, to be closer to

qcow2_compress API (and usual expectations).

Revert condition in if to be more positive. Drop dead initialization of

ret.

Cc: qemu-stable@nongnu.org # v4.0

Fixes: 341926ab83e2b

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Message-Id: <20200302150930.16218-1-vsementsov@virtuozzo.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2-threads.c | 12 +++++++-----

1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-threads.c

+++ b/block/qcow2-threads.c

@@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, size_t dest_size,

* @src - source buffer, @src_size bytes

*

* Returns: 0 on success

- * -1 on fail

+ * -EIO on fail

*/

static ssize_t qcow2_decompress(void *dest, size_t dest_size,

const void *src, size_t src_size)

{

- int ret = 0;

+ int ret;

z_stream strm;

memset(&strm, 0, sizeof(strm));

@@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_decompress(void *dest, size_t dest_size,

ret = inflateInit2(&strm, -12);

if (ret != Z_OK) {

- return -1;

+ return -EIO;

}

ret = inflate(&strm, Z_FINISH);

- if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || strm.avail_out != 0) {

+ if ((ret == Z_STREAM_END || ret == Z_BUF_ERROR) && strm.avail_out == 0) {

/*

* We approve Z_BUF_ERROR because we need @dest buffer to be filled, but

* @src buffer may be processed partly (because in qcow2 we know size of

* compressed data with precision of one sector)

*/

- ret = -1;

+ ret = 0;

+ } else {

+ ret = -EIO;

}

inflateEnd(&strm);

--

2.24.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

We need it in separate to pass to the block-copy object in the next

commit.

Cc: qemu-stable@nongnu.org

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200311103004.7649-2-vsementsov@virtuozzo.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

blockjob.c | 16 +++++-----

include/qemu/job.h | 11 ++-----

include/qemu/progress_meter.h | 58 +++++++++++++++++++++++++++++++++++

job-qmp.c | 4 +--

job.c | 6 ++--

qemu-img.c | 6 ++--

6 files changed, 76 insertions(+), 25 deletions(-)

create mode 100644 include/qemu/progress_meter.h

diff --git a/blockjob.c b/blockjob.c

index XXXXXXX..XXXXXXX 100644

--- a/blockjob.c

+++ b/blockjob.c

@@ -XXX,XX +XXX,XX @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)

info->device = g_strdup(job->job.id);

info->busy = atomic_read(&job->job.busy);

info->paused = job->job.pause_count > 0;

- info->offset = job->job.progress_current;

- info->len = job->job.progress_total;

+ info->offset = job->job.progress.current;

+ info->len = job->job.progress.total;

info->speed = job->speed;

info->io_status = job->iostatus;

info->ready = job_is_ready(&job->job),

@@ -XXX,XX +XXX,XX @@ static void block_job_event_cancelled(Notifier *n, void *opaque)

qapi_event_send_block_job_cancelled(job_type(&job->job),

job->job.id,

- job->job.progress_total,

- job->job.progress_current,

+ job->job.progress.total,

+ job->job.progress.current,

job->speed);

}

@@ -XXX,XX +XXX,XX @@ static void block_job_event_completed(Notifier *n, void *opaque)

qapi_event_send_block_job_completed(job_type(&job->job),

job->job.id,

- job->job.progress_total,

- job->job.progress_current,

+ job->job.progress.total,

+ job->job.progress.current,

job->speed,

!!msg,

msg);

@@ -XXX,XX +XXX,XX @@ static void block_job_event_ready(Notifier *n, void *opaque)

qapi_event_send_block_job_ready(job_type(&job->job),

job->job.id,

- job->job.progress_total,

- job->job.progress_current,

+ job->job.progress.total,

+ job->job.progress.current,

job->speed);

}

diff --git a/include/qemu/job.h b/include/qemu/job.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/job.h

+++ b/include/qemu/job.h

@@ -XXX,XX +XXX,XX @@

#include "qapi/qapi-types-job.h"

#include "qemu/queue.h"

+#include "qemu/progress_meter.h"

#include "qemu/coroutine.h"

#include "block/aio.h"

@@ -XXX,XX +XXX,XX @@ typedef struct Job {

/** True if this job should automatically dismiss itself */

bool auto_dismiss;

- /**

- * Current progress. The unit is arbitrary as long as the ratio between

- * progress_current and progress_total represents the estimated percentage

- * of work already done.

- */

- int64_t progress_current;

-

- /** Estimated progress_current value at the completion of the job */

- int64_t progress_total;

+ ProgressMeter progress;

/**

* Return code from @run and/or @prepare callback(s).

diff --git a/include/qemu/progress_meter.h b/include/qemu/progress_meter.h

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/include/qemu/progress_meter.h

@@ -XXX,XX +XXX,XX @@

+/*

+ * Helper functionality for some process progress tracking.

+ *

+ * Copyright (c) 2011 IBM Corp.

+ * Copyright (c) 2012, 2018 Red Hat, Inc.

+ * Copyright (c) 2020 Virtuozzo International GmbH

+ *

+ * Permission is hereby granted, free of charge, to any person obtaining a copy

+ * of this software and associated documentation files (the "Software"), to deal

+ * in the Software without restriction, including without limitation the rights

+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

+ * copies of the Software, and to permit persons to whom the Software is

+ * furnished to do so, subject to the following conditions:

+ *

+ * The above copyright notice and this permission notice shall be included in

+ * all copies or substantial portions of the Software.

+ *

+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL

+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN

+ * THE SOFTWARE.

+ */

+

+#ifndef QEMU_PROGRESS_METER_H

+#define QEMU_PROGRESS_METER_H

+

+typedef struct ProgressMeter {

+ /**

+ * Current progress. The unit is arbitrary as long as the ratio between

+ * current and total represents the estimated percentage

+ * of work already done.

+ */

+ uint64_t current;

+

+ /** Estimated current value at the completion of the process */

+ uint64_t total;

+} ProgressMeter;

+

+static inline void progress_work_done(ProgressMeter *pm, uint64_t done)

+{

+ pm->current += done;

+}

+

+static inline void progress_set_remaining(ProgressMeter *pm, uint64_t remaining)

+{

+ pm->total = pm->current + remaining;

+}

+

+static inline void progress_increase_remaining(ProgressMeter *pm,

+ uint64_t delta)

+{

+ pm->total += delta;

+}

+

+#endif /* QEMU_PROGRESS_METER_H */

diff --git a/job-qmp.c b/job-qmp.c

index XXXXXXX..XXXXXXX 100644

--- a/job-qmp.c

+++ b/job-qmp.c

@@ -XXX,XX +XXX,XX @@ static JobInfo *job_query_single(Job *job, Error **errp)

.id = g_strdup(job->id),

.type = job_type(job),

.status = job->status,

- .current_progress = job->progress_current,

- .total_progress = job->progress_total,

+ .current_progress = job->progress.current,

+ .total_progress = job->progress.total,

.has_error = !!job->err,

.error = job->err ? \

g_strdup(error_get_pretty(job->err)) : NULL,

diff --git a/job.c b/job.c

index XXXXXXX..XXXXXXX 100644

--- a/job.c

+++ b/job.c

@@ -XXX,XX +XXX,XX @@ void job_unref(Job *job)

void job_progress_update(Job *job, uint64_t done)

{

- job->progress_current += done;

+ progress_work_done(&job->progress, done);

}

void job_progress_set_remaining(Job *job, uint64_t remaining)

{

- job->progress_total = job->progress_current + remaining;

+ progress_set_remaining(&job->progress, remaining);

}

void job_progress_increase_remaining(Job *job, uint64_t delta)

{

- job->progress_total += delta;

+ progress_increase_remaining(&job->progress, delta);

}

void job_event_cancelled(Job *job)

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static void run_block_job(BlockJob *job, Error **errp)

do {

float progress = 0.0f;

aio_poll(aio_context, true);

- if (job->job.progress_total) {

- progress = (float)job->job.progress_current /

- job->job.progress_total * 100.f;

+ if (job->job.progress.total) {

+ progress = (float)job->job.progress.current /

+ job->job.progress.total * 100.f;

}

qemu_progress_print(progress, 0);

} while (!job_is_ready(&job->job) && !job_is_completed(&job->job));

--

2.24.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Assume we have two regions, A and B, and region B is in-flight now,

region A is not yet touched, but it is unallocated and should be

skipped.

Correspondingly, as progress we have

total = A + B

current = 0

If we reset unallocated region A and call progress_reset_callback,

it will calculate 0 bytes dirty in the bitmap and call

job_progress_set_remaining, which will set

total = current + 0 = 0 + 0 = 0

So, B bytes are actually removed from total accounting. When job

finishes we'll have

total = 0

current = B

, which doesn't sound good.

This is because we didn't considered in-flight bytes, actually when

calculating remaining, we should have set (in_flight + dirty_bytes)

as remaining, not only dirty_bytes.

To fix it, let's refactor progress calculation, moving it to block-copy

itself instead of fixing callback. And, of course, track in_flight

bytes count.

We still have to keep one callback, to maintain backup job bytes_read

calculation, but it will go on soon, when we turn the whole backup

process into one block_copy call.

Cc: qemu-stable@nongnu.org

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Message-Id: <20200311103004.7649-3-vsementsov@virtuozzo.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/backup.c | 13 ++-----------

block/block-copy.c | 16 ++++++++++++----

include/block/block-copy.h | 15 +++++----------

3 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/block/backup.c b/block/backup.c

index XXXXXXX..XXXXXXX 100644

--- a/block/backup.c

+++ b/block/backup.c

@@ -XXX,XX +XXX,XX @@ static void backup_progress_bytes_callback(int64_t bytes, void *opaque)

BackupBlockJob *s = opaque;

s->bytes_read += bytes;

- job_progress_update(&s->common.job, bytes);

-}

-

-static void backup_progress_reset_callback(void *opaque)

-{

- BackupBlockJob *s = opaque;

- uint64_t estimate = bdrv_get_dirty_count(s->bcs->copy_bitmap);

-

- job_progress_set_remaining(&s->common.job, estimate);

}

static int coroutine_fn backup_do_cow(BackupBlockJob *job,

@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,

job->cluster_size = cluster_size;

job->len = len;

- block_copy_set_callbacks(bcs, backup_progress_bytes_callback,

- backup_progress_reset_callback, job);

+ block_copy_set_progress_callback(bcs, backup_progress_bytes_callback, job);

+ block_copy_set_progress_meter(bcs, &job->common.job.progress);

/* Required permissions are already taken by backup-top target */

block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,

diff --git a/block/block-copy.c b/block/block-copy.c

index XXXXXXX..XXXXXXX 100644

--- a/block/block-copy.c

+++ b/block/block-copy.c

@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,

return s;

}

-void block_copy_set_callbacks(

+void block_copy_set_progress_callback(

BlockCopyState *s,

ProgressBytesCallbackFunc progress_bytes_callback,

- ProgressResetCallbackFunc progress_reset_callback,

void *progress_opaque)

{

s->progress_bytes_callback = progress_bytes_callback;

- s->progress_reset_callback = progress_reset_callback;

s->progress_opaque = progress_opaque;

}

+void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)

+{

+ s->progress = pm;

+}

+

/*

* block_copy_do_copy

*

@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,

if (!ret) {

bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes);

- s->progress_reset_callback(s->progress_opaque);

+ progress_set_remaining(s->progress,

+ bdrv_get_dirty_count(s->copy_bitmap) +

+ s->in_flight_bytes);

}

*count = bytes;

@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,

trace_block_copy_process(s, start);

bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);

+ s->in_flight_bytes += chunk_end - start;