ImageInfo oddities regarding compression

ImageInfo has an optional member @compressed: ## # @ImageInfo: [...] # @compressed: true if the image is compressed (Since 1.7) Doc bug: neglects to specify the default. I guess it's false. The only user appears to be vmdk_get_extent_info(). Goes back to v1.7.0's commits

Re: [PATCH v2 6/6] Rename arch_init.h to arch_type.h

On Wed, 25 Nov 2020 15:56:36 -0500 Eduardo Habkost wrote: > The only declarations in arch_init.h are related to the arch_type > variable (which is a useful feature that allows us to simplify > command line option handling). Rename the header to reflect its > purpose. > > Signed-off-by: Eduardo

Re: ImageInfo oddities regarding compression

On Fri, Nov 27, 2020 at 11:06:36AM +0100, Markus Armbruster wrote: > ImageInfo has an optional member @compressed: > > ## > # @ImageInfo: > [...] > # @compressed: true if the image is compressed (Since 1.7) > > Doc bug: neglects to specify the default. I guess it's false. > >

Re: ImageInfo oddities regarding compression

Kevin Wolf writes: > Am 27.11.2020 um 11:14 hat Daniel P. Berrangé geschrieben: >> On Fri, Nov 27, 2020 at 11:06:36AM +0100, Markus Armbruster wrote: >> > ImageInfo has an optional member @compressed: >> > >> > ## >> > # @ImageInfo: >> > [...] >> > # @compressed: true if the

Re: ImageInfo oddities regarding compression

Am 27.11.2020 um 11:14 hat Daniel P. Berrangé geschrieben: > On Fri, Nov 27, 2020 at 11:06:36AM +0100, Markus Armbruster wrote: > > ImageInfo has an optional member @compressed: > > > > ## > > # @ImageInfo: > > [...] > > # @compressed: true if the image is compressed (Since 1.7)

[PATCH v3 0/5] Increase amount of data for monitor to read

The subject was discussed here: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00206.html https://patchew.org/QEMU/20190610105906.28524-1-dplotni...@virtuozzo.com/# Message-ID: <31dd78ba-bd64-2ed6-3c8f-eed4e904d...@virtuozzo.com> and v2: Message-Id:

[PATCH v3 4/5] iotests: 129 don't check backup "busy"

From: Vladimir Sementsov-Ogievskiy Busy is racy, job has it's "pause-points" when it's not busy. Drop this check. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- tests/qemu-iotests/129 | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/qemu-iotests/129

[PATCH v3 3/5] monitor: let QMP monitor track JSON message content

We are going to allow the QMP monitor reading data from input channel more than one byte at once to increase the performance. With the OOB compatibility disabled, the monitor queues one QMP command at most. It was done for the backward compatibility as stated in the comment before pushing a

[PATCH v2 03/36] block: bdrv_append(): don't consume reference

We have too much comments for this feature. It seems better just don't do it. Most of real users (tests don't count) have to create additional reference. Drop also comment in external_snapshot_prepare: - bdrv_append doesn't "remove" old bs in common sense, it sounds strange - the fact that

[PATCH v2 25/36] block: introduce bdrv_drop_filter()

Using bdrv_replace_node() for removing filter is not good enough: it keeps child reference of the filter, which may conflict with original top node during permission update. Instead let's create new interface, which will do all graph modifications first and then update permissions. Let's modify

Re: [PATCH v2 2/2] monitor: increase amount of data for monitor to read

On 24.11.2020 14:03, Vladimir Sementsov-Ogievskiy wrote: 23.11.2020 18:44, Andrey Shinkevich wrote: QMP and HMP monitors read one byte at a time from the socket or stdin, which is very inefficient. With 100+ VMs on the host, this results in multiple extra system calls and CPU overuse. This

[PATCH v3 2/5] monitor: drain requests queue with 'channel closed' event

When CHR_EVENT_CLOSED comes, the QMP requests queue may still contain unprocessed commands. It can happen with QMP capability OOB enabled. Let the dispatcher complete handling requests rest in the monitor queue. Signed-off-by: Andrey Shinkevich --- monitor/qmp.c | 46

[PATCH v3 5/5] monitor: increase amount of data for monitor to read

QMP and HMP monitors read one byte at a time from the socket or stdin, which is very inefficient. With 100+ VMs on the host, this results in multiple extra system calls and CPU overuse. This patch increases the amount of read data up to 4096 bytes that fits the buffer size on the channel level.

[PATCH v2 20/36] block: add bdrv_attach_child_common() transaction action

Split out no-perm part of bdrv_root_attach_child() into separate transaction action. bdrv_root_attach_child() now moves to new permission update paradigm: first update graph relations then update permissions. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 162

[PATCH v2 21/36] block: add bdrv_attach_child_noperm() transaction action

Split no-perm part of bdrv_attach_child as separate transaction action. It will be used in later commits. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 71 ++--- 1 file changed, 58 insertions(+), 13 deletions(-) diff --git

[PATCH v2 10/36] util: add transactions.c

Add simple transaction API to use in further update of block graph operations. Supposed usage is: - "prepare" is main function of the action and it should make the main effect of the action to be visible for the following actions, keeping possibility of roll-back, saving necessary things in

[PATCH v2 13/36] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()

We are going to drop recursive bdrv_child_* functions, so stop use them in bdrv_child_try_set_perm() as a first step. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/block.c b/block.c index

[PATCH v2 15/36] block: use topological sort for permission update

Rewrite bdrv_check_perm(), bdrv_abort_perm_update() and bdrv_set_perm() to update nodes in topological sort order instead of simple DFS. With topologically sorted nodes, we update a node only when all its parents already updated. With DFS it's not so. Consider the following example: A -+

[PATCH v2 32/36] block: inline bdrv_check_perm_common()

bdrv_check_perm_common() has only one caller, so no more sense in "common". Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 32 +++- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/block.c b/block.c index 3ea04bbd8f..6c87ad0287 100644 ---

[PATCH v2 16/36] block: add bdrv_drv_set_perm transaction action

Refactor calling driver callbacks to a separate transaction action to be used later. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 70 - 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index

[PATCH v2 34/36] block: refactor bdrv_child_set_perm_safe() transaction action

Old interfaces dropped, nobody directly calls bdrv_child_set_perm_abort() and bdrv_child_set_perm_commit(), so we can use personal state structure for the action and stop exploiting BdrvChild structure. Also, drop "_safe" suffix which is redundant now. Signed-off-by: Vladimir Sementsov-Ogievskiy

[PATCH v2 02/36] tests/test-bdrv-graph-mod: add test_parallel_perm_update

Add test to show that simple DFS recursion order is not correct for permission update. Correct order is topological-sort order, which will be introduced later. Consider the block driver which has two filter children: one active with exclusive write access and one inactive with no specific

[PATCH v2 22/36] block: split out bdrv_replace_node_noperm()

Split part of bdrv_replace_node_common() to be used separately. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 47 ++- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/block.c b/block.c index 162a247579..02da1a90bc 100644 ---

[PATCH v2 28/36] block: add bdrv_set_backing_noperm() transaction action

Split out no-perm part of bdrv_set_backing_hd() as a separate transaction action. Note the in case of existing BdrvChild we reuse it, not recreate, just to do less actions. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 111 +--- 1

[PATCH v2 26/36] block/backup-top: drop .active

We don't need this workaround anymore: bdrv_append is already smart enough and we can use new bdrv_drop_filter(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/backup-top.c | 38 +- tests/qemu-iotests/283.out | 2 +- 2 files changed, 2

[PATCH v2 29/36] blockdev: qmp_x_blockdev_reopen: acquire all contexts

During reopen we may add backing bs from other aio context, which may lead to changing original context of top bs. We are going to move graph modification to prepare stage. So, it will be possible that bdrv_flush() in bdrv_reopen_prepare called on bs in non-original aio context, which we didn't

[PATCH v2 23/36] block: adapt bdrv_append() for inserting filters

bdrv_append is not very good for inserting filters: it does extra permission update as part of bdrv_set_backing_hd(). During this update filter may conflict with other parents of top_bs. Instead, let's first do all graph modifications and after it update permissions. Note: bdrv_append() is still

[PATCH v2 33/36] block: inline bdrv_replace_child()

bdrv_replace_child() has only one caller, the second argument is unused. Inline it now. This triggers deletion of some more unused interfaces. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 101 ++-- 1 file changed, 18

[PATCH v2 05/36] block: add bdrv_parent_try_set_aio_context

We already have bdrv_parent_can_set_aio_context(). Add corresponding bdrv_parent_set_aio_context_ignore() and bdrv_parent_try_set_aio_context() and use them instead of open-coding. Make bdrv_parent_try_set_aio_context() public, as it will be used in further commit. Signed-off-by: Vladimir

[PATCH v2 12/36] block: refactor bdrv_child* permission functions

Split out non-recursive parts, and refactor as block graph transaction action. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 79 ++--- 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/block.c b/block.c index

[PATCH v2 30/36] block: bdrv_reopen_multiple: refresh permissions on updated graph

Move bdrv_reopen_multiple to new paradigm of permission update: first update graph relations, then do refresh the permissions. We have to modify reopen process in file-posix driver: with new scheme we don't have prepared permissions in raw_reopen_prepare(), so we should reconfigure fd in

[PATCH v2 36/36] block: refactor bdrv_node_check_perm()

Now, bdrv_node_check_perm() is called only with fresh cumulative permissions, so its actually "refresh_perm". Move permission calculation to the function. Also, drop unreachable error message. Add also Virtuozzo copyright, as big work is done at this point. Signed-off-by: Vladimir

[PATCH v2 35/36] block: rename bdrv_replace_child_safe() to bdrv_replace_child()

We don't have bdrv_replace_child(), so it's time for bdrv_replace_child_safe() to take its place. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 1fde22e4f4..20b1cf59f7 100644 ---

[PATCH v2 04/36] block: bdrv_append(): return status

Return int status to avoid extra error propagation schemes. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block.h | 4 ++-- block.c | 15 --- block/commit.c | 6 ++ block/mirror.c | 6 ++ blockdev.c

[PATCH v2 06/36] block: BdrvChildClass: add .get_parent_aio_context handler

Add new handler to get aio context and implement it in all child classes. Add corresponding public interface to be used soon. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block.h | 3 +++ include/block/block_int.h | 2 ++ block.c | 13 +

[PATCH v2 01/36] tests/test-bdrv-graph-mod: add test_parallel_exclusive_write

Add the test that shows that concept of ignore_children is incomplete. Actually, when we want to update something, ignoring permission of some existing BdrvChild, we should ignore also the propagated effect of this child to the other children. But that's not done. Better approach (update

[PATCH v2 07/36] block: drop ctx argument from bdrv_root_attach_child

Passing parent aio context is redundant, as child_class and parent opaque pointer are enough to retrieve it. Drop the argument and use new bdrv_child_get_parent_aio_context() interface. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block_int.h | 1 - block.c |

[PATCH v2 00/36] block: update graph permissions update

Hi all! Here is a proposal of updating graph changing procedures. The thing brought me here is a question about "activating" filters after insertion, which is done in mirror_top and backup_top. The problem is that we can't simply avoid permission conflict when inserting the filter: during

[PATCH v2 19/36] block: fix bdrv_replace_node_common

inore_children thing doesn't help to track all propagated permissions of children we want to ignore. The simplest way to correctly update permissions is update graph first and then do permission update. In this case we just referesh permissions for the whole subgraph (in topological-sort defined

[PATCH v2 31/36] block: drop unused permission update functions

Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 103 1 file changed, 103 deletions(-) diff --git a/block.c b/block.c index 474e624152..3ea04bbd8f 100644 --- a/block.c +++ b/block.c @@ -1933,11 +1933,6 @@ static int

Re: ImageInfo oddities regarding compression

Am 27.11.2020 um 13:21 hat Markus Armbruster geschrieben: > >> I fell down this (thankfully shallow) rabbit hole because we also have > >> > >> { 'enum': 'MultiFDCompression', > >> 'data': [ 'none', 'zlib', > >> { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] } > >>

Re: ImageInfo oddities regarding compression

Kevin Wolf writes: > Am 27.11.2020 um 13:21 hat Markus Armbruster geschrieben: >> >> I fell down this (thankfully shallow) rabbit hole because we also have >> >> >> >> { 'enum': 'MultiFDCompression', >> >> 'data': [ 'none', 'zlib', >> >> { 'name': 'zstd', 'if':

[PATCH v3 1/5] monitor: change function obsolete name in comments

The function name monitor_qmp_bh_dispatcher() has been changed to monitor_qmp_dispatcher_co() since the commit 9ce44e2c. Let's amend the comments. Signed-off-by: Andrey Shinkevich --- monitor/qmp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monitor/qmp.c

[PATCH v2 17/36] block: add bdrv_list_* permission update functions

Add new interface, allowing use of existing node list. It will be used to fix bdrv_replace_node() in the further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 106 +--- 1 file changed, 71 insertions(+), 35 deletions(-)

[PATCH v2 09/36] block: return value from bdrv_replace_node()

Functions with errp argument are not recommened to be void-functions. Improve bdrv_replace_node(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block.h | 4 ++-- block.c | 14 -- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git

[PATCH v2 14/36] block: inline bdrv_child_*() permission functions calls

Each of them has only one caller. Open-coding simplifies further pemission-update system changes. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 59 + 1 file changed, 17 insertions(+), 42 deletions(-) diff --git a/block.c

[PATCH v2 11/36] block: bdrv_refresh_perms: check parents compliance

Add additional check that node parents do not interfere with each other. This should not hurt existing callers and allows in further patch use bdrv_refresh_perms() to update a subtree of changed BdrvChild (check that change is correct). New check will substitute bdrv_check_update_perm() in

[PATCH v2 08/36] block: make bdrv_reopen_{prepare, commit, abort} private

These functions are called only from bdrv_reopen_multiple() in block.c. No reason to publish them. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block.h | 4 block.c | 13 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git

[PATCH v2 18/36] block: add bdrv_replace_child_safe() transaction action

To be used in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 53 + 1 file changed, 53 insertions(+) diff --git a/block.c b/block.c index 6996aee1cf..f24bd60c2f 100644 --- a/block.c +++ b/block.c @@ -84,6 +84,8

[PATCH v2 24/36] block: add bdrv_remove_backing transaction action

Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 42 -- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index 7094922509..b1394b721c 100644 --- a/block.c +++ b/block.c @@ -2973,12 +2973,19 @@ out: return

[PATCH v2 27/36] block: drop ignore_children for permission update functions

This argument is always NULL. Drop it. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 36 +++- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/block.c b/block.c index e835a78f06..54fb6d24bd 100644 --- a/block.c +++ b/block.c @@ -1934,7