[Qemu-block] [PATCH v2 2/3] qcow2: Don't allow overflow during cluster allocation

Our code was already checking that we did not attempt to allocate more clusters than what would fit in an INT64 (the physical maximimum if we can access a full off_t's worth of data). But this does not catch smaller limits enforced by various spots in the qcow2 image description: L1 and normal

[Qemu-block] [PATCH v2 3/3] qcow2: Avoid memory over-allocation on compressed images

When reading a compressed image, we were allocating s->cluster_data to 32*cluster_size + 512 (possibly over 64 megabytes, for an image with 2M clusters). Let's check out the history: Back when qcow2 was first written, we used s->cluster_data for everything, including copy_sectors() and

[Qemu-block] [PATCH v2 0/3] qcow2: minor compression improvements

Updates to v1: - fix whitespace [Berto] - fix g_try_malloc usage [Berto, Kevin] - improve comments [Berto, Kevin] - add a patch to avoid overflow on 512TB images with 2M clusters Eric Blake (3): qcow2: Prefer byte-based calls into bs->file qcow2: Don't allow overflow during cluster allocation

Re: [Qemu-block] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

On 02/21/2018 08:08 AM, Alberto Garcia wrote: This patch fixes several mistakes in the documentation of the compressed cluster descriptor: More things to consider, as followup patches: Note that both the L1 table, and the standard L2 descriptors, have a cap on bit 55 as the maximum host

Re: [Qemu-block] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

On 02/21/2018 11:39 AM, Kevin Wolf wrote: See my commit message comment - we have other spots in the code base that blindly g_malloc(2 * s->cluster_size). Though is that a reason to do the same in new code or to phase out such allocations whenever you touch them? Touché. And I intended

Re: [Qemu-block] [PATCH v2 10/36] test-qemu-opts: Test qemu_opts_to_qdict_filtered()

On 02/21/2018 07:53 AM, Kevin Wolf wrote: Signed-off-by: Kevin Wolf --- tests/test-qemu-opts.c | 125 + 1 file changed, 125 insertions(+) diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index

Re: [Qemu-block] [PATCH v2 09/36] test-qemu-opts: Test qemu_opts_append()

On 02/21/2018 07:53 AM, Kevin Wolf wrote: Basic test for merging two QemuOptsLists. Signed-off-by: Kevin Wolf --- tests/test-qemu-opts.c | 128 + 1 file changed, 128 insertions(+) Reviewed-by: Eric Blake

Re: [Qemu-block] [PATCH] docs: document how to use the l2-cache-entry-size parameter

Am 19.02.2018 um 15:54 hat Alberto Garcia geschrieben: > This patch updates docs/qcow2-cache.txt explaining how to use the new > l2-cache-entry-size parameter. > > Here's a more detailed technical description of this feature: > >

Re: [Qemu-block] [PATCH v2 08/36] util: Add qemu_opts_to_qdict_filtered()

On 02/21/2018 07:53 AM, Kevin Wolf wrote: This allows, given a QemuOpts for a QemuOptsList that was merged from multiple QemuOptsList, to only consider those options that exist in one specific list. Block drivers need this to separate format-layer create options from protocol-level options.

Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

On 02/21/2018 10:59 AM, Eric Blake wrote: > On 02/21/2018 09:00 AM, Eric Blake wrote: >> On 02/21/2018 04:04 AM, Alberto Garcia wrote: >>> On Tue 20 Feb 2018 11:24:59 PM CET, Eric Blake wrote: >>> >>> I was also preparing a patch to change this, but you arrived first :-) >>> So, it's time

Re: [Qemu-block] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

Am 21.02.2018 um 17:59 hat Eric Blake geschrieben: > On 02/21/2018 10:51 AM, Kevin Wolf wrote: > > Am 20.02.2018 um 23:24 hat Eric Blake geschrieben: > > > When reading a compressed image, we were allocating s->cluster_data > > > to 32*cluster_size + 512 (possibly over 64 megabytes, for an image >

Re: [Qemu-block] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

Am 21.02.2018 um 15:08 hat Alberto Garcia geschrieben: > This patch fixes several mistakes in the documentation of the > compressed cluster descriptor: > > 1) the documentation claims that the cluster descriptor contains the >number of sectors used to store the compressed data, but what it >

Re: [Qemu-block] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

On 02/21/2018 10:51 AM, Kevin Wolf wrote: Am 20.02.2018 um 23:24 hat Eric Blake geschrieben: When reading a compressed image, we were allocating s->cluster_data to 32*cluster_size + 512 (possibly over 64 megabytes, for an image with 2M clusters). Let's check out the history: Much later, in

Re: [Qemu-block] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

Am 20.02.2018 um 23:24 hat Eric Blake geschrieben: > When reading a compressed image, we were allocating s->cluster_data > to 32*cluster_size + 512 (possibly over 64 megabytes, for an image > with 2M clusters). Let's check out the history: > > Back when qcow2 was first written, we used

Re: [Qemu-block] [PATCH 11/27] block: x-blockdev-create QMP command

On 02/21/2018 04:29 AM, Kevin Wolf wrote: +++ b/include/block/block_int.h @@ -130,6 +130,8 @@ struct BlockDriver { int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags, Error **errp); void (*bdrv_close)(BlockDriverState *bs); +int

Re: [Qemu-block] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

On Wed 21 Feb 2018 05:10:30 PM CET, Eric Blake wrote: >> Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)): >> >> -Bit 0 - x:Host cluster offset. This is usually _not_ aligned to a >> -cluster boundary! >> +Bit 0 - x-1: Host cluster offset. This

Re: [Qemu-block] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

CC: qemu-sta...@nongnu.org Incorrect docs make for difficult interoperability. On 02/21/2018 08:08 AM, Alberto Garcia wrote: This patch fixes several mistakes in the documentation of the compressed cluster descriptor: 1) the documentation claims that the cluster descriptor contains the

Re: [Qemu-block] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

On 02/21/2018 08:08 AM, Alberto Garcia wrote: This patch fixes several mistakes in the documentation of the compressed cluster descriptor: 1) the documentation claims that the cluster descriptor contains the number of sectors used to store the compressed data, but what it actually

Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

On 02/21/2018 09:00 AM, Eric Blake wrote: On 02/21/2018 04:04 AM, Alberto Garcia wrote: On Tue 20 Feb 2018 11:24:59 PM CET, Eric Blake wrote: I was also preparing a patch to change this, but you arrived first :-) So, it's time to cut back on the waste.  A compressed cluster will NEVER occupy

Re: [Qemu-block] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

On Wed 21 Feb 2018 04:00:54 PM CET, Eric Blake wrote: >> - Solution b: the width of the 'compressed cluster size' field is >>(cluster_bits - 8), that's (cluster_size / 256) sectors. > > Not true. It is (cluster_bits - 9) or (cluster_size / 512). It's not, it's (cluster_bits - 8), the

Re: [Qemu-block] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

On 02/21/2018 04:04 AM, Alberto Garcia wrote: On Tue 20 Feb 2018 11:24:59 PM CET, Eric Blake wrote: I was also preparing a patch to change this, but you arrived first :-) So, it's time to cut back on the waste. A compressed cluster will NEVER occupy more than an uncompressed cluster (okay,

[Qemu-block] [PATCH v2 36/36] qemu-iotests: Test ssh image creation over QMP

Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- tests/qemu-iotests/207 | 261 + tests/qemu-iotests/207.out | 75 + tests/qemu-iotests/group | 1 + 3 files changed, 337 insertions(+)

[Qemu-block] [PATCH v2 29/36] ssh: Use QAPI BlockdevOptionsSsh object

Create a BlockdevOptionsSsh object in connect_to_ssh() and take the options from there. 'host_key_check' is still processed separately because it's not in the schema yet. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block/ssh.c | 136

[Qemu-block] [PATCH v2 32/36] ssh: Support .bdrv_co_create

This adds the .bdrv_co_create driver callback to ssh, which enables image creation over QMP. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- qapi/block-core.json | 16 - block/ssh.c | 92

[Qemu-block] [PATCH v2 26/36] nfs: Support .bdrv_co_create

This adds the .bdrv_co_create driver callback to nfs, which enables image creation over QMP. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- qapi/block-core.json | 16 +++- block/nfs.c | 74

[Qemu-block] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

This patch fixes several mistakes in the documentation of the compressed cluster descriptor: 1) the documentation claims that the cluster descriptor contains the number of sectors used to store the compressed data, but what it actually contains is the number of sectors *minus one* or, in

[Qemu-block] [PATCH v2 25/36] nfs: Use QAPI options in nfs_client_open()

Using the QAPI visitor to turn all options into QAPI BlockdevOptionsNfs simplifies the code a lot. It will also be useful for implementing the QAPI based .bdrv_co_create callback. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block/nfs.c | 176

[Qemu-block] [PATCH v2 35/36] qemu-iotests: Test qcow2 over file image creation with QMP

Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- tests/qemu-iotests/206 | 436 + tests/qemu-iotests/206.out | 209 ++ tests/qemu-iotests/group | 1 + 3 files changed, 646

[Qemu-block] [PATCH v2 33/36] file-posix: Fix no-op bdrv_truncate() with falloc preallocation

If bdrv_truncate() is called, but the requested size is the same as before, don't call posix_fallocate(), which returns -EINVAL for length zero and would therefore make bdrv_truncate() fail. The problem can be triggered by creating a zero-sized raw image with 'falloc' preallocation mode.

[Qemu-block] [PATCH v2 31/36] ssh: Pass BlockdevOptionsSsh to connect_to_ssh()

Move the parsing of the QDict options up to the callers, in preparation for the .bdrv_co_create implementation that directly gets a QAPI type. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block/ssh.c | 34 +- 1 file

[Qemu-block] [PATCH v2 21/36] rbd: Pass BlockdevOptionsRbd to qemu_rbd_connect()

With the conversion to a QAPI options object, the function is now prepared to be used in a .bdrv_co_create implementation. Signed-off-by: Kevin Wolf --- block/rbd.c | 102 +++- 1 file changed, 52 insertions(+), 50

[Qemu-block] [PATCH v2 34/36] block: Fail bdrv_truncate() with negative size

Most callers have their own checks, but something like this should also be checked centrally. As it happens, x-blockdev-create can pass negative image sizes to format drivers (because there is no QAPI type that would reject negative numbers) and triggers the check added by this patch.

[Qemu-block] [PATCH v2 30/36] ssh: QAPIfy host-key-check option

This makes the host-key-check option available in blockdev-add. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- qapi/block-core.json | 63 +++-- block/ssh.c | 88

[Qemu-block] [PATCH v2 24/36] rbd: Use qemu_rbd_connect() in qemu_rbd_do_create()

This is almost exactly the same code. The differences are that qemu_rbd_connect() supports BlockdevOptionsRbd.server and that the cache mode is set explicitly. Supporting 'server' is a welcome new feature for image creation. Caching is disabled by default, so leave it that way. Signed-off-by:

[Qemu-block] [PATCH v2 18/36] rbd: Fix use after free in qemu_rbd_set_keypairs() error path

If we want to include the invalid option name in the error message, we can't free the string earlier than that. Signed-off-by: Kevin Wolf --- block/rbd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/rbd.c b/block/rbd.c index

[Qemu-block] [PATCH v2 28/36] sheepdog: Support .bdrv_co_create

This adds the .bdrv_co_create driver callback to sheepdog, which enables image creation over QMP. Signed-off-by: Kevin Wolf --- qapi/block-core.json | 24 +- block/sheepdog.c | 240 +++ 2 files changed, 189

[Qemu-block] [PATCH v2 23/36] rbd: Assing s->snap/image_name in qemu_rbd_open()

Now that the options are already available in qemu_rbd_open() and not only parsed in qemu_rbd_connect(), we can assign s->snap and s->image_name there instead of passing the fields by reference to qemu_rbd_connect(). Signed-off-by: Kevin Wolf --- block/rbd.c | 14

[Qemu-block] [PATCH v2 16/36] file-win32: Support .bdrv_co_create

This adds the .bdrv_co_create driver callback to file-win32, which enables image creation over QMP. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block/file-win32.c | 45 + 1 file changed, 37 insertions(+),

[Qemu-block] [PATCH v2 27/36] sheepdog: QAPIfy "redundacy" create option

The "redundacy" option for Sheepdog image creation is currently a string that can encode one or two integers depending on its format, which at the same time implicitly selects a mode. This patch turns it into a QAPI union and converts the string into such a QAPI object before interpreting the

[Qemu-block] [PATCH v2 15/36] file-posix: Support .bdrv_co_create

This adds the .bdrv_co_create driver callback to file, which enables image creation over QMP. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- qapi/block-core.json | 20 +- block/file-posix.c | 77

[Qemu-block] [PATCH v2 13/36] block: Make bdrv_is_whitelisted() public

We'll use a separate source file for image creation, and we need to check there whether the requested driver is whitelisted. Signed-off-by: Kevin Wolf --- include/block/block.h | 1 + block.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git

[Qemu-block] [PATCH v2 17/36] gluster: Support .bdrv_co_create

This adds the .bdrv_co_create driver callback to gluster, which enables image creation over QMP. Signed-off-by: Kevin Wolf --- qapi/block-core.json | 18 ++- block/gluster.c | 135 ++- 2 files changed, 108

[Qemu-block] [PATCH v2 20/36] rbd: Remove non-schema options from runtime_opts

Instead of the QemuOpts in qemu_rbd_connect(), we want to use QAPI objects. As a preparation, fetch those options directly from the QDict that .bdrv_open() supports in the rbd driver and that are not in the schema. Signed-off-by: Kevin Wolf --- block/rbd.c | 55

[Qemu-block] [PATCH v2 11/36] qdict: Introduce qdict_rename_keys()

A few block drivers will need to rename .bdrv_create options for their QAPIfication, so let's have a helper function for that. Signed-off-by: Kevin Wolf --- include/qapi/qmp/qdict.h | 6 +++ qobject/qdict.c | 34 ++ tests/check-qdict.c | 113

[Qemu-block] [PATCH v2 12/36] qcow2: Use visitor for options in qcow2_create()

Instead of manually creating the BlockdevCreateOptions object, use a visitor to parse the given options into the QAPI object. This involves translation from the old command line syntax to the syntax mandated by the QAPI schema. Option names are still checked against qcow2_create_opts, so only the

[Qemu-block] [PATCH v2 09/36] test-qemu-opts: Test qemu_opts_append()

Basic test for merging two QemuOptsLists. Signed-off-by: Kevin Wolf --- tests/test-qemu-opts.c | 128 + 1 file changed, 128 insertions(+) diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 5d5a3daa7b..6c3183390b

[Qemu-block] [PATCH v2 05/36] qcow2: Use BlockdevRef in qcow2_create2()

Instead of passing a separate BlockDriverState* into qcow2_create2(), make use of the BlockdevRef that is included in BlockdevCreateOptions. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max Reitz ---

[Qemu-block] [PATCH v2 07/36] qcow2: Handle full/falloc preallocation in qcow2_create2()

Once qcow2_create2() can be called directly on an already existing node, we must provide the 'full' and 'falloc' preallocation modes outside of creating the image on the protocol layer. Fortunately, we have preallocated truncate now which can provide this functionality. Signed-off-by: Kevin Wolf

[Qemu-block] [PATCH v2 08/36] util: Add qemu_opts_to_qdict_filtered()

This allows, given a QemuOpts for a QemuOptsList that was merged from multiple QemuOptsList, to only consider those options that exist in one specific list. Block drivers need this to separate format-layer create options from protocol-level options. Signed-off-by: Kevin Wolf

[Qemu-block] [PATCH v2 01/36] block/qapi: Introduce BlockdevCreateOptions

This creates a BlockdevCreateOptions union type that will contain all of the options for image creation. We'll start out with an empty struct type BlockdevCreateNotSupported for all drivers. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max

[Qemu-block] [PATCH v2 02/36] block/qapi: Add qcow2 create options to schema

Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Max Reitz --- qapi/block-core.json | 45 - 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/qapi/block-core.json

[Qemu-block] [PATCH v2 03/36] qcow2: Let qcow2_create() handle protocol layer

Currently, qcow2_create() only parses the QemuOpts and then calls qcow2_create2() for the actual image creation, which includes both the creation of the actual file on the file system and writing a valid empty qcow2 image into that file. The plan is that qcow2_create2() becomes the function that

[Qemu-block] [PATCH v2 00/36] x-blockdev-create for protocols and qcow2

This series implements a minimal QMP command that allows to create an image file on the protocol level or an image format on a given block node. Eventually, the interface is going to change to some kind of an async command (possibly a (non-)block job), but that will require more work on the job

Re: [Qemu-block] [PATCH v2] specs/qcow2: Fix documentation of the compressed cluster descriptor

On Tue 20 Feb 2018 08:40:43 PM CET, Eric Blake wrote: >> Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)): > > I'm looking at how this works for different cluster sizes. If we have > 512-byte clusters, x is 61, and we DON'T have the 'number sectors' > field at all! Well, you can

Re: [Qemu-block] [PATCH] migration: do not transfer ram during bulk storage migration

On Tue, Feb 20, 2018 at 04:10:03PM +0100, Peter Lieven wrote: > this patch makes the bulk phase of a block migration to take > place before we start transferring ram. As the bulk block migration > can take a long time its pointless to transfer ram during that phase. > > Signed-off-by: Peter

Re: [Qemu-block] [PATCH 11/27] block: x-blockdev-create QMP command

Am 15.02.2018 um 20:58 hat Eric Blake geschrieben: > On 02/08/2018 01:23 PM, Kevin Wolf wrote: > > This adds a synchronous x-blockdev-create QMP command that can create > > qcow2 images on a given node name. > > > > We don't want to block while creating an image, so this is not the final > >

Re: [Qemu-block] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

On Tue 20 Feb 2018 11:24:59 PM CET, Eric Blake wrote: I was also preparing a patch to change this, but you arrived first :-) > So, it's time to cut back on the waste. A compressed cluster > will NEVER occupy more than an uncompressed cluster (okay, gzip > DOES document that because the

Re: [Qemu-block] [PATCH 1/2] qcow2: Prefer byte-based calls into bs->file

On Tue 20 Feb 2018 11:24:58 PM CET, Eric Blake wrote: > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > index d46b69d7f34..3fefeb3dc50 100644 > --- a/block/qcow2-refcount.c > +++ b/block/qcow2-refcount.c > @@ -2310,8 +2310,8 @@ write_refblocks: > on_disk_refblock = (void