Re: [PULL 30/39] block: bdrv_reopen_multiple: refresh permissions on updated graph

2021-04-30 Thread Peter Maydell
On Fri, 30 Apr 2021 at 11:53, Kevin Wolf wrote: > > From: 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 >

Re: [PULL 18/39] block: add bdrv_attach_child_common() transaction action

2021-04-30 Thread Peter Maydell
On Fri, 30 Apr 2021 at 11:53, Kevin Wolf wrote: > > From: 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 > perm

Re: [PULL 37/64] block/snapshot: Fix fallback

2021-04-30 Thread Peter Maydell
On Mon, 7 Sept 2020 at 12:11, Kevin Wolf wrote: > > From: Max Reitz > > If the top node's driver does not provide snapshot functionality and we > want to fall back to a node down the chain, we need to snapshot all > non-COW children. For simplicity's sake, just do not fall back if there > is mor

[PATCH] Add missing coroutine_fn function signature to functions

2021-04-30 Thread cennedee
>From 447601c28d5ed0b1208a0560390f760e75ce5613 Mon Sep 17 00:00:00 2001 From: Cenne Dee Date: Fri, 30 Apr 2021 15:52:28 -0400 Subject: [PATCH] Add missing coroutine_fn function signature to functions Patch adds the signature for all relevant functions ending with _co or those that use them. Sign

Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout

2021-04-30 Thread Emanuele Giuseppe Esposito
On 30/04/2021 15:50, Max Reitz wrote: On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Using the flag -p, allow the qemu binary to print to stdout. This helps especially when doing print-debugging. I think this shouldn’t refer to prints but to qemu’s stdout/stderr in general, i.e

Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

2021-04-30 Thread Emanuele Giuseppe Esposito
On 30/04/2021 15:02, Max Reitz wrote: On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: As with gdbserver, valgrind delays the test execution, so the default QMP socket timeout timeout too soon. I’m curious: The default timeouts should be long enough for slow systems, too, though (e.g.

Re: [PATCH v3 04/15] qemu-iotests: add option to attach gdbserver

2021-04-30 Thread Emanuele Giuseppe Esposito
On 30/04/2021 13:38, Max Reitz wrote: On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Add -gdb flag and GDB_QEMU environmental variable to python tests to attach a gdbserver to each qemu instance. Well, this patch doesn’t do this, but OK. Maybe "define" rather than "add"? In the sens

Re: [PATCH v3 05/15] qemu-iotests: delay QMP socket timers

2021-04-30 Thread Emanuele Giuseppe Esposito
On 30/04/2021 13:59, Max Reitz wrote: On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Attaching a gdbserver implies that the qmp socket should wait indefinitely for an answer from QEMU. Signed-off-by: Emanuele Giuseppe Esposito ---   python/qemu/machine.py    |  3 +++   tests/qemu-

[PATCH 3/4] block/vvfat: Fix leak of mapping_t::path

2021-04-30 Thread Philippe Mathieu-Daudé
read_directory() keeps pointers to alloc'ed data in path ...: 743 static int read_directory(BDRVVVFATState* s, int mapping_index) 744 { ... 792 buffer = g_malloc(length); ... 828 /* create mapping for this file */ 829 if(!is_dot && !is_dotdot && (S_ISDIR(st.st_mode)

[PATCH] xen-block: Use specific blockdev driver

2021-04-30 Thread Anthony PERARD via
From: Anthony PERARD ... when a xen-block backend instance is created via xenstore. Following 8d17adf34f50 ("block: remove support for using "file" driver with block/char devices"), using the "file" blockdev driver for everything doesn't work anymore, we need to use the "host_device" driver when

[PATCH 4/4] block/vvfat: Avoid out of bounds write in create_long_filename()

2021-04-30 Thread Philippe Mathieu-Daudé
The direntry_t::name holds 11 bytes: typedef struct direntry_t { uint8_t name[8 + 3]; ... However create_long_filename() writes up to 31 bytes into it: 421 for(i=0;i<26*number_of_entries;i++) { 422 int offset=(i%26); 423 if(offset<10) offset=1+offset; 424

[PATCH 0/4] block/vvfat: Fix leaks and out of bounds writes

2021-04-30 Thread Philippe Mathieu-Daudé
The first 3 patches are trivial leak fixes, the last one is a RFC since I have no clue about this code. Johannes, you wrote this 18 years ago, do you still remember? =) Philippe Mathieu-Daudé (4): block/vvfat: Fix leak of BDRVVVFATState::qcow_filename block/vvfat: Fix leak of BDRVVVFATState::

[PATCH 2/4] block/vvfat: Fix leak of BDRVVVFATState::used_clusters

2021-04-30 Thread Philippe Mathieu-Daudé
used_clusters is allocated in enable_write_target(), called by vvfat_open(), but never free'd. Allocate it using GLib API, and free it in vvfat_close(). This fixes (QEMU built with --enable-sanitizers): Direct leak of 64508 byte(s) in 1 object(s) allocated from: #0 0x55d7a36378f7 in callo

Re: [ANNOUNCE] libblkio v0.1.0 preview release

2021-04-30 Thread Stefan Hajnoczi
On Thu, Apr 29, 2021 at 04:00:38PM +0100, Richard W.M. Jones wrote: > On Thu, Apr 29, 2021 at 03:41:29PM +0100, Stefan Hajnoczi wrote: > > On Thu, Apr 29, 2021 at 03:22:59PM +0100, Richard W.M. Jones wrote: > > > This is where I get confused about what this library actually does. > > > It's not jus

[PATCH 1/4] block/vvfat: Fix leak of BDRVVVFATState::qcow_filename

2021-04-30 Thread Philippe Mathieu-Daudé
qcow_filename is allocated in enable_write_target(), called by vvfat_open(), but never free'd. Free it in vvfat_close(). This fixes (QEMU built with --enable-sanitizers): Direct leak of 4096 byte(s) in 1 object(s) allocated from: #0 0x55d7a363773f in malloc (/mnt/scratch/qemu/sanitizer/q

Re: [ANNOUNCE] libblkio v0.1.0 preview release

2021-04-30 Thread Stefan Hajnoczi
On Thu, Apr 29, 2021 at 05:51:16PM +0200, Kevin Wolf wrote: > Am 29.04.2021 um 16:05 hat Stefan Hajnoczi geschrieben: > > Hi, > > A preview release of libblkio, a library for high-performance block I/O, > > is now available: > > > > https://gitlab.com/libblkio/libblkio/-/tree/v0.1.0 > > > > App

Re: [PULL 00/39] Block layer patches

2021-04-30 Thread Peter Maydell
On Fri, 30 Apr 2021 at 11:51, Kevin Wolf wrote: > > The following changes since commit ccdf06c1db192152ac70a1dd974c624f566cb7d4: > > Open 6.1 development tree (2021-04-30 11:15:40 +0100) > > are available in the Git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for y

Re: [PATCH v2] Document qemu-img options data_file and data_file_raw

2021-04-30 Thread Max Reitz
On 30.04.21 15:34, Connor Kuehl wrote: The contents of this patch were initially developed and posted by Han Han[1], however, it appears the original patch was not applied. Since then, the relevant documentation has been moved and adapted to a new format. I've taken most of the original wording

Re: [PATCH v3 15/15] docs/devel/testing: add -p option to the debug section of QEMU iotests

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 4 1 file changed, 4 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 62902cfd2d..0c18fc4571 100644 --- a/docs/devel/testing.rst ++

[PATCH v2] Document qemu-img options data_file and data_file_raw

2021-04-30 Thread Connor Kuehl
The contents of this patch were initially developed and posted by Han Han[1], however, it appears the original patch was not applied. Since then, the relevant documentation has been moved and adapted to a new format. I've taken most of the original wording and tweaked it according to some of the f

Re: [PATCH v3 03/15] docs/devel/testing: add debug section to the QEMU iotests chapter

2021-04-30 Thread Paolo Bonzini
On 30/04/21 13:23, Max Reitz wrote: +--- +QEMU iotests offers some options to debug a failing test, that can be +given as options to the ``check`` script: -, Even better: "The following options to the ``check`` script can be useful when debugging a failing test:". Pao

Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Using the flag -p, allow the qemu binary to print to stdout. This helps especially when doing print-debugging. I think this shouldn’t refer to prints but to qemu’s stdout/stderr in general, i.e Signed-off-by: Emanuele Giuseppe Esposit

Re: [PATCH v3 13/15] docs/devel/testing: add -valgrind option to the debug section of QEMU iotests

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 2ee77a057b..62902cfd2d 100644 --- a/docs/devel/testing.rst

Re: [PATCH v3 06/15] qemu_iotests: insert gdbserver command line as wrapper for qemu binary

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/iotests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Max Reitz

Re: [PATCH v3 12/15] qemu_iotests: insert valgrind command line as wrapper for qemu binary

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: The priority will be given to gdb command line, meaning if the -gdb parameter and -valgrind are given, gdb will be wrapped around the qemu binary. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/iotests.py | 3 ++- 1 file

Re: [PATCH 2/3] hw/ide: Add Kconfig dependency MICRODRIVE -> PCMCIA

2021-04-30 Thread Laurent Vivier
Le 25/04/2021 à 00:20, Philippe Mathieu-Daudé a écrit : > The Microdrive Compact Flash can be plugged on a PCMCIA bus. > Express the dependency using the 'depends on' Kconfig expression. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/ide/Kconfig | 1 + > 1 file changed, 1 insertion(+) > >

Re: [PATCH v3 11/15] qemu_iotests: allow valgrind to read/delete the generated log file

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: When using -valgrind on the script tests, it generates a log file in $TEST_DIR that is either read (if valgrind finds problems) or otherwise deleted. Provide the same exact behavior when using -valgrind on the python tests. Signed-off-by: Eman

Re: [PATCH 3/3] hw/pcmcia: Do not register PCMCIA type if not required

2021-04-30 Thread Laurent Vivier
Le 25/04/2021 à 00:20, Philippe Mathieu-Daudé a écrit : > If the Kconfig 'PCMCIA' value is not selected, it is pointless > to build the PCMCIA core components. > > (Currently only one machine of the ARM targets requires this). > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/pcmcia/meson.bu

Re: [PATCH 1/3] hw/arm/pxa2xx: Declare PCMCIA bus with Kconfig

2021-04-30 Thread Laurent Vivier
Le 25/04/2021 à 00:20, Philippe Mathieu-Daudé a écrit : > The Intel XScale PXA chipsets provide a PCMCIA controller, > which expose a PCMCIA (IDE) bus. Express this dependency using > the Kconfig 'select' expression. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/arm/Kconfig | 1 + > 1 fil

Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: As with gdbserver, valgrind delays the test execution, so the default QMP socket timeout timeout too soon. I’m curious: The default timeouts should be long enough for slow systems, too, though (e.g. heavily-loaded CI systems). I would expec

Re: [PATCH v3 05/15] qemu-iotests: delay QMP socket timers

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Attaching a gdbserver implies that the qmp socket should wait indefinitely for an answer from QEMU. Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/machine.py| 3 +++ tests/qemu-iotests/iotests.py | 10 +- 2 fil

Re: [PATCH v3 09/15] qemu_iotests: extend the check script to support valgrind for python tests

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Currently, the check script only parses the option and sets the VALGRIND_QEMU environmental variable to "y". Add another local python variable that prepares the command line, identical to the one provided in the test scripts. Because the pytho

Re: [PATCH v3 08/15] docs/devel/testing: add -gdb option to the debugging section of QEMU iotests

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index b7e2370e7e..2ee77a057b 100644 --- a/docs/devel/testing.rst

Re: [PATCH v3 07/15] qemu-iotests: add gdbserver option to script tests too

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: The only limitation here is that running a script with gdbserver will make the test output mismatch with the expected results, making the test fail. Signed-off-by: Emanuele Giuseppe Esposito --- tests/qemu-iotests/common.rc | 8 +++- 1

Re: [PATCH v3 04/15] qemu-iotests: add option to attach gdbserver

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Add -gdb flag and GDB_QEMU environmental variable to python tests to attach a gdbserver to each qemu instance. if -gdb is not provided but $GDB_QEMU is set, ignore the environmental variable. Signed-off-by: Emanuele Giuseppe Esposito --- t

Re: [PATCH v3 04/15] qemu-iotests: add option to attach gdbserver

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Add -gdb flag and GDB_QEMU environmental variable to python tests to attach a gdbserver to each qemu instance. Well, this patch doesn’t do this, but OK. Out of interest: Why gdbserver and not “just” gdb? On Fedora, those are separate packa

Re: [PATCH v3 03/15] docs/devel/testing: add debug section to the QEMU iotests chapter

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Introduce the "Debugging a test case" section, in preparation to the additional flags that will be added in the next patches. Signed-off-by: Emanuele Giuseppe Esposito --- docs/devel/testing.rst | 8 1 file changed, 8 insertions(+

Re: [PATCH v3 02/15] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/qtest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py index 39a0cf62fe..c18eae96c6 100644 --- a/python/qemu/qtes

Re: [PATCH v3 01/15] python: qemu: add timer parameter for qmp.accept socket

2021-04-30 Thread Max Reitz
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Add a new _qmp_timer field to the QEMUMachine class. The default timer is 15 sec, as per the default in the qmp accept() function. Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/machine.py | 3 ++- 1 file changed, 2 insertions(+

Re: [PATCH] mirror: stop cancelling in-flight requests on non-force cancel in READY

2021-04-30 Thread Max Reitz
On 21.04.21 09:58, Vladimir Sementsov-Ogievskiy wrote: If mirror is READY than cancel operation is not discarding the whole result of the operation, but instead it's a documented way get a point-in-time snapshot of source disk. So, we should not cancel any requests if mirror is READ and force=fa

[PULL 37/39] block: Add BDRV_O_NO_SHARE for blk_new_open()

2021-04-30 Thread Kevin Wolf
Normally, blk_new_open() just shares all permissions. This was fine originally when permissions only protected against uses in the same process because no other part of the code would actually get to access the block nodes opened with blk_new_open(). However, since we use it for file locking now, u

[PULL 36/39] block: refactor bdrv_node_check_perm()

2021-04-30 Thread Kevin Wolf
From: 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 kno

[PULL 34/39] block: refactor bdrv_child_set_perm_safe() transaction action

2021-04-30 Thread Kevin Wolf
From: 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-o

[PULL 27/39] block: make bdrv_refresh_limits() to be a transaction action

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy To be used in further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-28-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- include/block/block.h | 3 ++- block.c | 9 --

[PULL 29/39] block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare

2021-04-30 Thread Kevin Wolf
From: 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-ori

[PULL 35/39] block: rename bdrv_replace_child_safe() to bdrv_replace_child()

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-36-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --

[PULL 32/39] block: inline bdrv_check_perm_common()

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy bdrv_check_perm_common() has only one caller, so no more sense in "common". Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-33-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 32 +++---

[PULL 31/39] block: drop unused permission update functions

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-32-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 103 1 file changed, 103 dele

[PULL 38/39] qemu-img convert: Unshare write permission for source

2021-04-30 Thread Kevin Wolf
For a successful conversion of an image, we must make sure that its content doesn't change during the conversion. A special case of this is using the same image file both as the source and as the destination. If both input and output format are raw, the operation would just be useless work, with o

[PULL 39/39] vhost-user-blk: Fail gracefully on too large queue size

2021-04-30 Thread Kevin Wolf
virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so vhost_user_blk_device_realize() should check this before calling it. Simple reproducer: qemu-system-x86_64 \ -chardev null,id=foo \ -device vhost-user-blk-pci,queue-size=4096,chardev=foo Fixes: https://bugzilla.redhat.com

[PULL 33/39] block: inline bdrv_replace_child()

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-34-vsement...

[PULL 26/39] block: make bdrv_unset_inherits_from to be a transaction action

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy To be used in the further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-27-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 46 ++

[PULL 30/39] block: bdrv_reopen_multiple: refresh permissions on updated graph

2021-04-30 Thread Kevin Wolf
From: 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

[PULL 25/39] block: drop ignore_children for permission update functions

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy This argument is always NULL. Drop it. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-26-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 38 +++--- 1 f

[PULL 20/39] block: split out bdrv_replace_node_noperm()

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy Split part of bdrv_replace_node_common() to be used separately. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-21-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 50 ++

[PULL 19/39] block: add bdrv_attach_child_noperm() transaction action

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-20-vsement...@virtuozzo.com> Signed-off-by: Kevin

[PULL 18/39] block: add bdrv_attach_child_common() transaction action

2021-04-30 Thread Kevin Wolf
From: 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. qsd-jobs test output updated. Seems now permission u

[PULL 28/39] block: add bdrv_set_backing_noperm() transaction action

2021-04-30 Thread Kevin Wolf
From: 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

[PULL 15/39] block: add bdrv_list_* permission update functions

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-16-vsement...@virtuozzo.com> Signed

[PULL 24/39] block/backup-top: drop .active

2021-04-30 Thread Kevin Wolf
From: 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 theoretica

[PULL 21/39] block: adapt bdrv_append() for inserting filters

2021-04-30 Thread Kevin Wolf
From: 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 permis

[PULL 11/39] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-12-vsement...@virtuozzo.com> Signed-of

[PULL 10/39] block: refactor bdrv_child* permission functions

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy Split out non-recursive parts, and refactor as block graph transaction action. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-11-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 79 +++

[PULL 22/39] block: add bdrv_remove_filter_or_cow transaction action

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-23-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 84 +++-- 1 file changed, 82 inser

[PULL 14/39] block: add bdrv_drv_set_perm transaction action

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy Refactor calling driver callbacks to a separate transaction action to be used later. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-15-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c |

[PULL 23/39] block: introduce bdrv_drop_filter()

2021-04-30 Thread Kevin Wolf
From: 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 the

[PULL 16/39] block: add bdrv_replace_child_safe() transaction action

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy To be used in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-17-vsement...@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 54 ++

[PULL 17/39] block: fix bdrv_replace_node_common

2021-04-30 Thread Kevin Wolf
From: 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 sub

[PULL 08/39] util: add transactions.c

2021-04-30 Thread Kevin Wolf
From: 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 ro

[PULL 03/39] tests/test-bdrv-graph-mod: add test_append_greedy_filter

2021-04-30 Thread Kevin Wolf
From: 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 h

[PULL 00/39] Block layer patches

2021-04-30 Thread Kevin Wolf
The following changes since commit ccdf06c1db192152ac70a1dd974c624f566cb7d4: Open 6.1 development tree (2021-04-30 11:15:40 +0100) are available in the Git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 68bf7336533faa6aa90fdd4558edddbf5d8ef81

[PULL 13/39] block: use topological sort for permission update

2021-04-30 Thread Kevin Wolf
From: 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

[PULL 12/39] block: inline bdrv_child_*() permission functions calls

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-13-vsement...@virtuozzo.com>

[PULL 09/39] block: bdrv_refresh_perms: check for parents permissions conflict

2021-04-30 Thread Kevin Wolf
From: 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 substitut

[PULL 02/39] tests/test-bdrv-graph-mod: add test_parallel_perm_update

2021-04-30 Thread Kevin Wolf
From: 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 o

[PULL 04/39] block: bdrv_append(): don't consume reference

2021-04-30 Thread Kevin Wolf
From: 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

[PULL 07/39] block: make bdrv_reopen_{prepare,commit,abort} private

2021-04-30 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy 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 Message-Id: <20210428151804.439460-8-vsement...@virtuozzo.com>

[PULL 06/39] block: drop ctx argument from bdrv_root_attach_child

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Alberto Garcia R

[PULL 05/39] block: BdrvChildClass: add .get_parent_aio_context handler

2021-04-30 Thread Kevin Wolf
From: 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 Reviewed-by: Alberto Garcia Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-6

[PULL 01/39] tests/test-bdrv-graph-mod: add test_parallel_exclusive_write

2021-04-30 Thread Kevin Wolf
From: 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 don

Re: [PATCH] block: simplify write-threshold and drop write notifiers

2021-04-30 Thread Max Reitz
On 22.04.21 00:09, Vladimir Sementsov-Ogievskiy wrote: write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's handle write-threshold simply in generic code and drop write notifiers at all. Also move part of write-threshold API that is used only

Re: [PATCH v2] vhost-user-blk: Fail gracefully on too large queue size

2021-04-30 Thread Kevin Wolf
Am 13.04.2021 um 18:56 hat Kevin Wolf geschrieben: > virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so > vhost_user_blk_device_realize() should check this before calling it. > > Simple reproducer: > > qemu-system-x86_64 \ > -chardev null,id=foo \ > -device vhost-user-blk-

Re: [PATCH v2] monitor: hmp_qemu_io: acquire aio contex, fix crash

2021-04-30 Thread Max Reitz
On 23.04.21 15:42, Vladimir Sementsov-Ogievskiy wrote: Max reported the following bug: $ ./qemu-img create -f raw src.img 1G $ ./qemu-img create -f raw dst.img 1G $ (echo ' {"execute":"qmp_capabilities"} {"execute":"blockdev-mirror", "arguments":{"job-id":"mirror",

Re: [PATCH] virtio-blk: drop deprecated scsi=on|off property

2021-04-30 Thread Peter Krempa
On Fri, Apr 30, 2021 at 09:42:05 +0200, Markus Armbruster wrote: > Eduardo Habkost writes: > > > On Thu, Apr 29, 2021 at 04:52:21PM +0100, Stefan Hajnoczi wrote: > >> The scsi=on|off property was deprecated in QEMU 5.0 and can be removed > >> completely at this point. > >> > >> Drop the scsi=on|

Re: [PATCH] virtio-blk: drop deprecated scsi=on|off property

2021-04-30 Thread Markus Armbruster
Eduardo Habkost writes: > On Thu, Apr 29, 2021 at 04:52:21PM +0100, Stefan Hajnoczi wrote: >> The scsi=on|off property was deprecated in QEMU 5.0 and can be removed >> completely at this point. >> >> Drop the scsi=on|off option. It was only available on Legacy virtio-blk >> devices. Linux v5.6 a