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

Make it more obvious how to add new fields to the version 3 header and

how to interpret them.

The specification is adjusted so that for new defined optional fields:

1. Software may support some of these optional fields and ignore the

others, which means that features may be backported to downstream

Qemu independently.

2. If we want to add incompatible field (or a field, for which some of

its values would be incompatible), it must be accompanied by

incompatible feature bit.

Also the concept of "default is zero" is clarified, as it's strange to

say that the value of the field is assumed to be zero for the software

version which don't know about the field at all and don't know how to

treat it be it zero or not.

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

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

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

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

[mreitz: s/some its/some of its/]

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

---

docs/interop/qcow2.txt | 45 +++++++++++++++++++++++++++++++++++++++---

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

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt

index XXXXXXX..XXXXXXX 100644

--- a/docs/interop/qcow2.txt

+++ b/docs/interop/qcow2.txt

@@ -XXX,XX +XXX,XX @@ The first cluster of a qcow2 image contains the file header:

Offset into the image file at which the snapshot table

starts. Must be aligned to a cluster boundary.

-If the version is 3 or higher, the header has the following additional fields.

-For version 2, the values are assumed to be zero, unless specified otherwise

-in the description of a field.

+For version 2, the header is exactly 72 bytes in length, and finishes here.

+For version 3 or higher, the header length is at least 104 bytes, including

+the next fields through header_length.

72 - 79: incompatible_features

Bitmask of incompatible features. An implementation must

@@ -XXX,XX +XXX,XX @@ in the description of a field.

100 - 103: header_length

Length of the header structure in bytes. For version 2

images, the length is always assumed to be 72 bytes.

+ For version 3 it's at least 104 bytes and must be a multiple

+ of 8.

+

+

+=== Additional fields (version 3 and higher) ===

+

+In general, these fields are optional and may be safely ignored by the software,

+as well as filled by zeros (which is equal to field absence), if software needs

+to set field B, but does not care about field A which precedes B. More

+formally, additional fields have the following compatibility rules:

+

+1. If the value of the additional field must not be ignored for correct

+handling of the file, it will be accompanied by a corresponding incompatible

+feature bit.

+

+2. If there are no unrecognized incompatible feature bits set, an unknown

+additional field may be safely ignored other than preserving its value when

+rewriting the image header.

+

+3. An explicit value of 0 will have the same behavior as when the field is not

+present*, if not altered by a specific incompatible bit.

+

+*. A field is considered not present when header_length is less than or equal

+to the field's offset. Also, all additional fields are not present for

+version 2.

+

+ < ... No additional fields in the header currently ... >

+

+

+=== Header padding ===

+

+@header_length must be a multiple of 8, which means that if the end of the last

+additional field is not aligned, some padding is needed. This padding must be

+zeroed, so that if some existing (or future) additional field will fall into

+the padding, it will be interpreted accordingly to point [3.] of the previous

+paragraph, i.e. in the same manner as when this field is not present.

+

+

+=== Header extensions ===

Directly after the image header, optional sections called header extensions can

be stored. Each extension has a structure like the following:

--

2.24.1

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

The patch adds a new additional field to the qcow2 header: compression_type,

which specifies compression type. If field is absent or zero, default

compression type is set: ZLIB, which corresponds to current behavior.

New compression type (ZSTD) is to be added in further commit.

Suggested-by: Denis Plotnikov <dplotnikov@virtuozzo.com>

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

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

[mreitz: s/Bits 3-63: Reserved/Bits 4-63: Reserved/]

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

---

docs/interop/qcow2.txt | 21 +++++++++++++++++++--

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

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt

index XXXXXXX..XXXXXXX 100644

--- a/docs/interop/qcow2.txt

+++ b/docs/interop/qcow2.txt

@@ -XXX,XX +XXX,XX @@ the next fields through header_length.

An External Data File Name header extension may

be present if this bit is set.

- Bits 3-63: Reserved (set to 0)

+ Bit 3: Compression type bit. If this bit is set,

+ a non-default compression is used for compressed

+ clusters. The compression_type field must be

+ present and not zero.

+

+ Bits 4-63: Reserved (set to 0)

80 - 87: compatible_features

Bitmask of compatible features. An implementation can

@@ -XXX,XX +XXX,XX @@ present*, if not altered by a specific incompatible bit.

to the field's offset. Also, all additional fields are not present for

version 2.

- < ... No additional fields in the header currently ... >

+ 104: compression_type

+

+ Defines the compression method used for compressed clusters.

+ All compressed clusters in an image use the same compression

+ type.

+

+ If the incompatible bit "Compression type" is set: the field

+ must be present and non-zero (which means non-zlib

+ compression type). Otherwise, this field must not be present

+ or must be zero (which means zlib).

+

+ Available compression type values:

+ 0: zlib <https://www.zlib.net/>

=== Header padding ===

--

2.24.1

From: Thomas Huth <thuth@redhat.com>

Commit d9df28e7b07 ("iotests: check whitelisted formats") added the

modern @iotests.skip_if_unsupported() to the functions in this test,

so we don't need the old explicit test here anymore.

Signed-off-by: Thomas Huth <thuth@redhat.com>

Message-Id: <20200129141751.32652-1-thuth@redhat.com>

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

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

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

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

---

tests/qemu-iotests/139 | 3 ---

1 file changed, 3 deletions(-)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/139

+++ b/tests/qemu-iotests/139

@@ -XXX,XX +XXX,XX @@ class TestBlockdevDel(iotests.QMPTestCase):

@iotests.skip_if_unsupported(['quorum'])

def testQuorum(self):

- if not iotests.supports_quorum():

- return

-

self.addQuorum('quorum0', 'node0', 'node1')

# We cannot remove the children of a Quorum device

self.delBlockDriverState('node0', expect_error = True)

--

2.24.1

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

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_find_node(const char *node_name)

}

/* Put this QMP function here so it can access the static graph_bdrv_states. */

-BlockDeviceInfoList *bdrv_named_nodes_list(Error **errp)

+BlockDeviceInfoList *bdrv_named_nodes_list(bool flat,

+ Error **errp)

{

BlockDeviceInfoList *list, *entry;

BlockDriverState *bs;

list = NULL;

QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {

- BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, errp);

+ BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, flat, errp);

if (!info) {

qapi_free_BlockDeviceInfoList(list);

return NULL;

diff --git a/docs/interop/qemu-img.rst b/docs/interop/qemu-img.rst

index XXXXXXX..XXXXXXX 100644

--- a/docs/interop/qemu-img.rst

+++ b/docs/interop/qemu-img.rst

@@ -XXX,XX +XXX,XX @@ Parameters to convert subcommand:

will still be printed. Areas that cannot be read from the source will be

treated as containing only zeroes.

+.. option:: --target-is-zero

+

+ Assume that reading the destination image will always return

+ zeros. This parameter is mutually exclusive with a destination image

+ that has a backing file. It is required to also use the ``-n``

+ parameter to skip image creation.

+

Parameters to dd subcommand:

.. program:: qemu-img-dd

@@ -XXX,XX +XXX,XX @@ Command description:

4

Error on reading data

-.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME

+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME

Convert the disk image *FILENAME* or a snapshot *SNAPSHOT_PARAM*

to disk image *OUTPUT_FILENAME* using format *OUTPUT_FMT*. It can

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img-cmds.hx

+++ b/qemu-img-cmds.hx

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

ERST

DEF("convert", img_convert,

- "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")

+ "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")

SRST

-.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] [--salvage] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME

+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] [--salvage] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME

ERST

DEF("create", img_create,