Re: iotests failing on gitlab CI check-system-fedora job

2021-03-17 Thread Paolo Bonzini
On 17/03/21 23:23, Peter Maydell wrote: The check-system-fedora job in the gitlab CI seems to have started reliably failing on iotests 040 041 127 256 267: https://gitlab.com/qemu-project/qemu/-/jobs/1106977551 Could somebody have a look at what's happening, please? (This is probably a

Re: [PATCH V3] file-posix: allow -EBUSY -EINVAL errors during write zeros on block

2021-03-17 Thread John Snow
On 3/9/21 7:16 PM, ChangLimin wrote: Since Linux 5.10, write zeros to a multipath device using ioctl(fd, BLKZEROOUT, range) with cache none or directsync return -EBUSY permanently. When do we get -EINVAL? Both of the commits referenced below don't specifically mention it, so I am not sure in

Re: Can not use hmp block_resize command with -blockdev option

2021-03-17 Thread John Snow
On 3/16/21 11:43 PM, zhao xiaojun wrote: Hi, I use -blockdev option to specify a drive when qemu boot and i want to resize it with hmp block_resize command. The hmp block_resize comand's arguments: block_resize device new_size. So I query the device by qmp query_block command, but the device

Re: "make check" broken with everything but tools disabled

2021-03-17 Thread John Snow
On 3/16/21 9:28 AM, Markus Armbruster wrote: Watch this: $ mkdir bld-tools $ cd bld-tools $ ../configure --disable-system --disable-user --enable-tools $ make check [...] make: *** No rule to make target 'tests/qemu-iotests/socket_scm_helper', needed by

iotests failing on gitlab CI check-system-fedora job

2021-03-17 Thread Peter Maydell
The check-system-fedora job in the gitlab CI seems to have started reliably failing on iotests 040 041 127 256 267: https://gitlab.com/qemu-project/qemu/-/jobs/1106977551 Could somebody have a look at what's happening, please? (This is probably a regression that's got into master because I

Re: [PULL v3 00/42] Block layer patches and object-add QAPIfication

2021-03-17 Thread Peter Maydell
On Tue, 16 Mar 2021 at 18:12, Kevin Wolf wrote: > > The following changes since commit 6e31b3a5c34c6e5be7ef60773e607f189eaa15f3: > > Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into > staging (2021-03-16 10:53:47 +) > > are available in the Git repository at: > >

Re: [PULL 00/13] misc patches removing deprecated features

2021-03-17 Thread Peter Maydell
On Tue, 16 Mar 2021 at 16:55, Daniel P. Berrangé wrote: > > The following changes since commit 6e31b3a5c34c6e5be7ef60773e607f189eaa15f3: > > Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into > staging (2021-03-16 10:53:47 +) > > are available in the Git repository

Re: [PATCH 5/6] test-coroutine: add rwlock upgrade test

2021-03-17 Thread David Edmondson
On Wednesday, 2021-03-17 at 19:00:12 +01, Paolo Bonzini wrote: > Test that rwlock upgrade is fair, and readers go back to sleep if a writer > is in line. > > Signed-off-by: Paolo Bonzini Reviewed-by: David Edmondson > --- > tests/unit/test-coroutine.c | 62

[PATCH 1/6] block/vdi: When writing new bmap entry fails, don't leak the buffer

2021-03-17 Thread Paolo Bonzini
From: David Edmondson If a new bitmap entry is allocated, requiring the entire block to be written, avoiding leaking the buffer allocated for the block should the write fail. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Edmondson Message-Id:

[PATCH 4/6] coroutine-lock: reimplement CoRwlock to fix downgrade bug

2021-03-17 Thread Paolo Bonzini
An invariant of the current rwlock is that if multiple coroutines hold a reader lock, all must be runnable. The unlock implementation relies on this, choosing to wake a single coroutine when the final read lock holder exits the critical section, assuming that it will wake a coroutine attempting to

[PATCH 2/6] block/vdi: Don't assume that blocks are larger than VdiHeader

2021-03-17 Thread Paolo Bonzini
From: David Edmondson Given that the block size is read from the header of the VDI file, a wide variety of sizes might be seen. Rather than re-using a block sized memory region when writing the VDI header, allocate an appropriately sized buffer. Signed-off-by: David Edmondson Message-Id:

[PATCH 6/6] test-coroutine: Add rwlock downgrade test

2021-03-17 Thread Paolo Bonzini
From: David Edmondson Test that downgrading an rwlock does not result in a failure to schedule coroutines queued on the rwlock. The diagram associated with test_co_rwlock_downgrade() describes the intended behaviour, but what was observed previously corresponds to: | c1 | c2 | c3

[PATCH 5/6] test-coroutine: add rwlock upgrade test

2021-03-17 Thread Paolo Bonzini
Test that rwlock upgrade is fair, and readers go back to sleep if a writer is in line. Signed-off-by: Paolo Bonzini --- tests/unit/test-coroutine.c | 62 + 1 file changed, 62 insertions(+) diff --git a/tests/unit/test-coroutine.c

[PATCH v5 0/6] coroutine rwlock downgrade fix, minor VDI changes

2021-03-17 Thread Paolo Bonzini
This is a resubmit of David Edmondson's series at https://patchew.org/QEMU/20210309144015.557477-1-david.edmond...@oracle.com/. After closer analysis on IRC, the CoRwlock's attempt to ensure fairness turned out to be flawed. Therefore, this series reimplements CoRwlock without using a CoQueue.

[PATCH 3/6] coroutine/mutex: Store the coroutine in the CoWaitRecord only once

2021-03-17 Thread Paolo Bonzini
From: David Edmondson When taking the slow path for mutex acquisition, set the coroutine value in the CoWaitRecord in push_waiter(), rather than both there and in the caller. Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Edmondson Message-Id:

Re: [RFC PATCH] curl: Allow reading after EOF

2021-03-17 Thread Eric Blake
On 3/17/21 11:43 AM, Kevin Wolf wrote: >>> It is not entirely clear to me if this is something we want to do. If we >>> do care about consistency between protocol drivers, something like this >>> should probably be done in block/io.c eventually - but that would >>> require converting

Re: [PATCH 4/6] coroutine-lock: reimplement CoRwLock to fix downgrade bug

2021-03-17 Thread Paolo Bonzini
On 17/03/21 16:17, David Edmondson wrote: +if (tkt) { +if (tkt->read) { +if (lock->owners >= 0) { +lock->owners++; +co = tkt->co; +} +} else { +if (lock->owners == 0) { +lock->owners = -1; +

Re: [PATCH 4/6] coroutine-lock: reimplement CoRwLock to fix downgrade bug

2021-03-17 Thread David Edmondson
On Wednesday, 2021-03-17 at 18:19:58 +01, Paolo Bonzini wrote: > On 17/03/21 16:17, David Edmondson wrote: >>> +if (tkt) { >>> +if (tkt->read) { >>> +if (lock->owners >= 0) { >>> +lock->owners++; >>> +co = tkt->co; >>> +} >>> +

[PATCH v4 2/6] block: Allow changing bs->file on reopen

2021-03-17 Thread Alberto Garcia
When the x-blockdev-reopen was added it allowed reconfiguring the graph by replacing backing files, but changing the 'file' option was forbidden. Because of this restriction some operations are not possible, notably inserting and removing block filters. This patch adds support for replacing the

Re: [PATCH v3 00/36] block: update graph permissions update

2021-03-17 Thread Eric Blake
On 3/17/21 12:33 PM, Eric Blake wrote: > On 3/17/21 10:38 AM, Vladimir Sementsov-Ogievskiy wrote: > >>> 6/36 Checking commit 5780b805277e (block: drop ctx argument from >>> bdrv_root_attach_child) >>> 7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare, >>> commit, abort} private)

[PATCH v4 3/6] iotests: Test replacing files with x-blockdev-reopen

2021-03-17 Thread Alberto Garcia
This patch adds new tests in which we use x-blockdev-reopen to change bs->file Signed-off-by: Alberto Garcia --- tests/qemu-iotests/245 | 109 - tests/qemu-iotests/245.out | 11 +++- 2 files changed, 117 insertions(+), 3 deletions(-) diff --git

Re: [PATCH v3 00/36] block: update graph permissions update

2021-03-17 Thread Eric Blake
On 3/17/21 10:38 AM, Vladimir Sementsov-Ogievskiy wrote: >> 6/36 Checking commit 5780b805277e (block: drop ctx argument from >> bdrv_root_attach_child) >> 7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare, >> commit, abort} private) >> ERROR: Author email address is mangled by

[PATCH v4 5/6] iotests: Test reopening multiple devices at the same time

2021-03-17 Thread Alberto Garcia
This test swaps the images used by two active block devices. This is now possible thanks to the new ability to run x-blockdev-reopen on multiple devices at the same time. Signed-off-by: Alberto Garcia --- tests/qemu-iotests/245 | 41 ++

[PATCH v4 0/6] Allow changing bs->file on reopen

2021-03-17 Thread Alberto Garcia
Based-on: <20210317143529.615584-1-vsement...@virtuozzo.com> Hello, this is the same as v3, but rebased on top of Vladimir's "block: update graph permissions update v3", which you can get here: git: https://src.openvz.org/scm/~vsementsov/qemu.git tag: up-block-topologic-perm-v3 Tip: you may

[PATCH v4 6/6] block: Make blockdev-reopen stable API

2021-03-17 Thread Alberto Garcia
This patch drops the 'x-' prefix from x-blockdev-reopen. Signed-off-by: Alberto Garcia --- qapi/block-core.json | 6 +++--- blockdev.c | 2 +- tests/qemu-iotests/155 | 2 +- tests/qemu-iotests/165 | 2 +- tests/qemu-iotests/245 | 10 +-

[PATCH v4 4/6] block: Support multiple reopening with x-blockdev-reopen

2021-03-17 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- qapi/block-core.json | 18 + blockdev.c | 78 +++--- tests/qemu-iotests/155 | 9 +++-- tests/qemu-iotests/165 | 4 +- tests/qemu-iotests/245 | 27 +++-- tests/qemu-iotests/248

[PATCH v4 1/6] block: Add bdrv_reopen_queue_free()

2021-03-17 Thread Alberto Garcia
Move the code to free a BlockReopenQueue to a separate function. It will be used in a subsequent patch. Signed-off-by: Alberto Garcia --- include/block/block.h | 1 + block.c | 16 2 files changed, 13 insertions(+), 4 deletions(-) diff --git

Re: [RFC PATCH] curl: Allow reading after EOF

2021-03-17 Thread Kevin Wolf
Am 17.03.2021 um 17:12 hat Daniel P. Berrangé geschrieben: > On Wed, Mar 17, 2021 at 04:17:34PM +0100, Kevin Wolf wrote: > > This makes the curl driver more consistent with file-posix in that it > > doesn't return errors any more for reading after the end of the remote > > file. Instead, zeros are

Re: [PATCH v3 13/36] block: use topological sort for permission update

2021-03-17 Thread Alberto Garcia
On Wed 17 Mar 2021 03:35:06 PM CET, Vladimir Sementsov-Ogievskiy wrote: > 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 >

Re: [RFC PATCH] curl: Allow reading after EOF

2021-03-17 Thread Kevin Wolf
Am 17.03.2021 um 16:46 hat Eric Blake geschrieben: > On 3/17/21 10:32 AM, Eric Blake wrote: > > On 3/17/21 10:17 AM, Kevin Wolf wrote: > >> This makes the curl driver more consistent with file-posix in that it > >> doesn't return errors any more for reading after the end of the remote > >> file.

[PATCH] iotests: add test for removing persistent bitmap from backing file

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
Just demonstrate one of x-blockdev-reopen usecases. We can't simply remove persistent bitmap from RO node (for example from backing file), as we need to remove it from the image too. So, we should reopen the node first. Signed-off-by: Vladimir Sementsov-Ogievskiy ---

Re: [RFC PATCH] curl: Allow reading after EOF

2021-03-17 Thread Eric Blake
On 3/17/21 10:32 AM, Eric Blake wrote: > On 3/17/21 10:17 AM, Kevin Wolf wrote: >> This makes the curl driver more consistent with file-posix in that it >> doesn't return errors any more for reading after the end of the remote >> file. Instead, zeros are returned for these areas. >> >> This

Re: [PATCH v3 00/36] block: update graph permissions update

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
17.03.2021 18:21, no-re...@patchew.org wrote: Patchew URL: https://patchew.org/QEMU/20210317143529.615584-1-vsement...@virtuozzo.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id:

Re: [RFC PATCH] curl: Allow reading after EOF

2021-03-17 Thread Daniel P . Berrangé
On Wed, Mar 17, 2021 at 04:17:34PM +0100, Kevin Wolf wrote: > This makes the curl driver more consistent with file-posix in that it > doesn't return errors any more for reading after the end of the remote > file. Instead, zeros are returned for these areas. > > This inconsistency was reported in:

Re: [PATCH v3 00/36] block: update graph permissions update

2021-03-17 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20210317143529.615584-1-vsement...@virtuozzo.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210317143529.615584-1-vsement...@virtuozzo.com Subject: [PATCH v3 00/36] block:

Re: [PATCH 4/6] coroutine-lock: reimplement CoRwLock to fix downgrade bug

2021-03-17 Thread David Edmondson
On Wednesday, 2021-03-17 at 13:16:39 +01, Paolo Bonzini wrote: > An invariant of the current rwlock is that if multiple coroutines hold a > reader lock, all must be runnable. The unlock implementation relies on > this, choosing to wake a single coroutine when the final read lock > holder exits

[RFC PATCH] curl: Allow reading after EOF

2021-03-17 Thread Kevin Wolf
This makes the curl driver more consistent with file-posix in that it doesn't return errors any more for reading after the end of the remote file. Instead, zeros are returned for these areas. This inconsistency was reported in: https://bugzilla.redhat.com/show_bug.cgi?id=1935061 Note that the

Re: [RFC PATCH] curl: Allow reading after EOF

2021-03-17 Thread Eric Blake
On 3/17/21 10:17 AM, Kevin Wolf wrote: > This makes the curl driver more consistent with file-posix in that it > doesn't return errors any more for reading after the end of the remote > file. Instead, zeros are returned for these areas. > > This inconsistency was reported in: >

Re: [PATCH 5/6] test-coroutine: add rwlock upgrade test

2021-03-17 Thread David Edmondson
On Wednesday, 2021-03-17 at 13:16:40 +01, Paolo Bonzini wrote: > Test that rwlock upgrade is fair, and readers go back to sleep if a writer > is in line. > > Signed-off-by: Paolo Bonzini Reviewed-by: David Edmondson > --- > tests/unit/test-coroutine.c | 62

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

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 29/36] block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 35/36] block: rename bdrv_replace_child_safe() to bdrv_replace_child()

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 b7cded9826..b61ac9ff19 100644 ---

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

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 15/36] block: add bdrv_list_* permission update functions

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 16/36] block: add bdrv_replace_child_safe() transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
To be used in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 54 ++ 1 file changed, 54 insertions(+) diff --git a/block.c b/block.c index 31a4e4fa5c..4f9d67a6a2 100644 --- a/block.c +++ b/block.c @@ -83,6

[PATCH v3 26/36] block: make bdrv_unset_inherits_from to be a transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
To be used in the further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 46 ++ 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index 922df82952..3b9681a738 100644 --- a/block.c +++ b/block.c @@

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

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 b39e6455b2..29e00c4708 100644 ---

[PATCH v3 24/36] block/backup-top: drop .active

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
We don't need this workaround anymore: bdrv_append is already smart enough and we can use new bdrv_drop_filter(). This commit efficiently reverts also recent 705dde27c6c53b73, which checked .active on io path. Still it said that the problem should be theoretical. And the logic of filter

[PATCH v3 27/36] block: make bdrv_refresh_limits() to be a transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
To be used in further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block.h | 3 ++- block.c | 9 - block/io.c| 31 +-- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/include/block/block.h

[PATCH v3 23/36] block: introduce bdrv_drop_filter()

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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

[PATCH v3 25/36] block: drop ignore_children for permission update functions

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
This argument is always NULL. Drop it. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 38 +++--- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/block.c b/block.c index bf60f1ea2c..922df82952 100644 --- a/block.c +++ b/block.c @@

[PATCH v3 14/36] block: add bdrv_drv_set_perm transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 22/36] block: add bdrv_remove_filter_or_cow transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 78 +++-- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index 11f7ad0818..2fca1f2ad5 100644 --- a/block.c +++ b/block.c @@ -2929,12 +2929,19 @@ static

[PATCH v3 20/36] block: split out bdrv_replace_node_noperm()

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
Split part of bdrv_replace_node_common() to be used separately. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 50 +++--- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/block.c b/block.c index d810915684..433cae1181 100644

[PATCH v3 18/36] block: add bdrv_attach_child_common() transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 | 189

[PATCH v3 13/36] block: use topological sort for permission update

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 21/36] block: adapt bdrv_append() for inserting filters

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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. append-greedy-filter

[PATCH v3 19/36] block: add bdrv_attach_child_noperm() transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 11/36] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 10/36] block: refactor bdrv_child* permission functions

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 05/36] block: BdrvChildClass: add .get_parent_aio_context handler

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 | 2 ++ include/block/block_int.h | 2 ++ block.c | 13 +

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

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 and rewrite the remaining one to be more generic (as now we don't know which node is added and which was

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

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 103 1 file changed, 103 deletions(-) diff --git a/block.c b/block.c index 53a8af301a..b39e6455b2 100644 --- a/block.c +++ b/block.c @@ -1953,11 +1953,6 @@ static int

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

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 28/36] block: add bdrv_set_backing_noperm() transaction action

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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. We don't need to create extra reference to backing_hd as we don't lose it in bdrv_attach_child(). Signed-off-by: Vladimir

[PATCH v3 17/36] block: fix bdrv_replace_node_common

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 12/36] block: inline bdrv_child_*() permission functions calls

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
Each of them has only one caller. Open-coding simplifies further pemission-update system changes. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia --- block.c | 59 + 1 file changed, 17 insertions(+), 42

[PATCH v3 09/36] block: bdrv_refresh_perms: check for parents permissions conflict

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 08/36] util: add transactions.c

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 04/36] block: bdrv_append(): don't consume reference

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 02/36] tests/test-bdrv-graph-mod: add test_parallel_perm_update

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 07/36] block: make bdrv_reopen_{prepare, commit, abort} private

2021-03-17 Thread Vladimir Sementsov-Ogievskiy via
These functions are called only from bdrv_reopen_multiple() in block.c. No reason to publish them. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Reviewed-by: Kevin Wolf --- include/block/block.h | 4 block.c | 13 + 2 files changed, 9

[PATCH v3 06/36] block: drop ctx argument from bdrv_root_attach_child

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 03/36] tests/test-bdrv-graph-mod: add test_append_greedy_filter

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
bdrv_append() is not quite good for inserting filters: it does extra permission update in intermediate state, where filter get it filtered child but is not yet replace it in a backing chain. Some filters (for example backup-top) may want permissions even when have no parents. And described

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

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
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 v3 00/36] block: update graph permissions update

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
Hi all! Finally, I finished v3. Phew. Missed a soft-freeze. Should we consider it a bugfix? There are bugfixes here but they are mostly theoretical. So, up to Kevin, should it go to current release or to the next.. The main point of the series is fixing some permission update problems (see

Re: [PULL 00/11] pflash patches for 2021-03-16

2021-03-17 Thread Philippe Mathieu-Daudé
On 3/17/21 2:37 PM, Peter Maydell wrote: > On Mon, 15 Mar 2021 at 23:37, Philippe Mathieu-Daudé > wrote: >> >> The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339: >> >> Merge remote-tracking branch >> 'remotes/stefanha-gitlab/tags/block-pull-request' into staging

[PATCH 2/2] hw/block/nvme: align reserved fields declarations

2021-03-17 Thread Gollu Appalanaidu
Align the Reserved fields declaration in NvmeBar Signed-off-by: Gollu Appalanaidu --- include/block/nvme.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/block/nvme.h b/include/block/nvme.h index fc65cfcb01..e5bd00bb85 100644 --- a/include/block/nvme.h +++

[PATCH 1/2] hw/block/nvme: align with existing style

2021-03-17 Thread Gollu Appalanaidu
Make uniform hexadecimal numbers format. Signed-off-by: Gollu Appalanaidu --- hw/block/nvme.c | 30 +++--- include/block/nvme.h | 10 +- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

Re: [PULL 00/11] pflash patches for 2021-03-16

2021-03-17 Thread Peter Maydell
On Mon, 15 Mar 2021 at 23:37, Philippe Mathieu-Daudé wrote: > > The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339: > > Merge remote-tracking branch > 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-03-15 > 19:23:00 +) > > are available in the

[PATCH 2/6] block/vdi: Don't assume that blocks are larger than VdiHeader

2021-03-17 Thread Paolo Bonzini
From: David Edmondson Given that the block size is read from the header of the VDI file, a wide variety of sizes might be seen. Rather than re-using a block sized memory region when writing the VDI header, allocate an appropriately sized buffer. Signed-off-by: David Edmondson Message-Id:

[PATCH 6/6] test-coroutine: Add rwlock downgrade test

2021-03-17 Thread Paolo Bonzini
From: David Edmondson Test that downgrading an rwlock does not result in a failure to schedule coroutines queued on the rwlock. The diagram associated with test_co_rwlock_downgrade() describes the intended behaviour, but what was observed previously corresponds to: | c1 | c2 | c3

[PATCH 5/6] test-coroutine: add rwlock upgrade test

2021-03-17 Thread Paolo Bonzini
Test that rwlock upgrade is fair, and readers go back to sleep if a writer is in line. Signed-off-by: Paolo Bonzini --- tests/unit/test-coroutine.c | 62 + 1 file changed, 62 insertions(+) diff --git a/tests/unit/test-coroutine.c

[PATCH 4/6] coroutine-lock: reimplement CoRwLock to fix downgrade bug

2021-03-17 Thread Paolo Bonzini
An invariant of the current rwlock is that if multiple coroutines hold a reader lock, all must be runnable. The unlock implementation relies on this, choosing to wake a single coroutine when the final read lock holder exits the critical section, assuming that it will wake a coroutine attempting to

[PATCH 1/6] block/vdi: When writing new bmap entry fails, don't leak the buffer

2021-03-17 Thread Paolo Bonzini
From: David Edmondson If a new bitmap entry is allocated, requiring the entire block to be written, avoiding leaking the buffer allocated for the block should the write fail. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Edmondson Message-Id:

[PATCH 3/6] coroutine/mutex: Store the coroutine in the CoWaitRecord only once

2021-03-17 Thread Paolo Bonzini
From: David Edmondson When taking the slow path for mutex acquisition, set the coroutine value in the CoWaitRecord in push_waiter(), rather than both there and in the caller. Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Edmondson Message-Id:

[PATCH v4 0/6] coroutine rwlock downgrade fix, minor VDI changes

2021-03-17 Thread Paolo Bonzini
This is a resubmit of David Edmondson's series at https://patchew.org/QEMU/20210309144015.557477-1-david.edmond...@oracle.com/. After closer analysis on IRC, the CoRwLock's attempt to ensure fairness turned out to be flawed. Therefore, this series reimplements CoRwLock without using a CoQueue.

Re: [PATCH 4/5] coroutine-lock: reimplement CoRwLock to fix downgrade bug

2021-03-17 Thread Paolo Bonzini
On 17/03/21 11:40, David Edmondson wrote: Isn't this... * ... Also, @qemu_co_rwlock_upgrade * only overrides CoRwlock fairness if there are no concurrent readers, so * another writer might run while @qemu_co_rwlock_upgrade blocks. ...now incorrect? Maybe, but for sure the comment was

Re: [PATCH 4/5] coroutine-lock: reimplement CoRwLock to fix downgrade bug

2021-03-17 Thread David Edmondson
On Tuesday, 2021-03-16 at 17:00:06 +01, Paolo Bonzini wrote: > A feature of the current rwlock is that if multiple coroutines hold a > reader lock, all must be runnable. The unlock implementation relies on > this, choosing to wake a single coroutine when the final read lock > holder exits the

Re: [PATCH 0/7] block/nbd: decouple reconnect from drain

2021-03-17 Thread Vladimir Sementsov-Ogievskiy
15.03.2021 09:06, Roman Kagan wrote: The reconnection logic doesn't need to stop while in a drained section. Moreover it has to be active during the drained section, as the requests that were caught in-flight with the connection to the server broken can only usefully get drained if the

Re: [PATCH v3 6/6] block/qcow2: use seqcache for compressed writes

2021-03-17 Thread Max Reitz
On 16.03.21 18:48, Vladimir Sementsov-Ogievskiy wrote: 16.03.2021 15:25, Max Reitz wrote: On 15.03.21 15:40, Vladimir Sementsov-Ogievskiy wrote: 15.03.2021 12:58, Max Reitz wrote: [...] The question is whether it really makes sense to even have a seqcache_read() path when in reality it’s

[PATCH v2] piix: fix regression during unplug in Xen HVM domUs

2021-03-17 Thread Olaf Hering
Commit ee358e919e385fdc79d59d0d47b4a81e349cd5c9 causes a regression in Xen HVM domUs which run xenlinux based kernels. If the domU has an USB device assigned, for example with "usbdevice=['tablet']" in domU.cfg, the late unplug of devices will kill the emulated USB host. As a result the khubd