Re: [PATCH v3 7/7] hw/virtio: generalise CHR_EVENT_CLOSED handling

2022-11-28 Thread Michael S. Tsirkin
On Tue, Nov 29, 2022 at 05:18:58AM +, Raphael Norwitz wrote: > > On Nov 28, 2022, at 11:41 AM, Alex Bennée wrote: > > > > ..and use for both virtio-user-blk and virtio-user-gpio. This avoids > > the circular close by deferring shutdown due to disconnection until a > > later point.

Re: [PATCH v3 7/7] hw/virtio: generalise CHR_EVENT_CLOSED handling

2022-11-28 Thread Raphael Norwitz
> On Nov 28, 2022, at 11:41 AM, Alex Bennée wrote: > > ..and use for both virtio-user-blk and virtio-user-gpio. This avoids > the circular close by deferring shutdown due to disconnection until a > later point. virtio-user-blk already had this mechanism in place so The mechanism was originally

Re: [PATCH v3 12/17] vfio/migration: Implement VFIO migration protocol v2

2022-11-28 Thread Alex Williamson
On Mon, 28 Nov 2022 16:56:39 -0400 Jason Gunthorpe wrote: > On Mon, Nov 28, 2022 at 01:36:30PM -0700, Alex Williamson wrote: > > On Mon, 28 Nov 2022 15:40:23 -0400 > > Jason Gunthorpe wrote: > > > > > On Mon, Nov 28, 2022 at 11:50:03AM -0700, Alex Williamson wrote: > > > > > > > There's a

Re: [PATCH v3 12/17] vfio/migration: Implement VFIO migration protocol v2

2022-11-28 Thread Jason Gunthorpe
On Mon, Nov 28, 2022 at 01:36:30PM -0700, Alex Williamson wrote: > On Mon, 28 Nov 2022 15:40:23 -0400 > Jason Gunthorpe wrote: > > > On Mon, Nov 28, 2022 at 11:50:03AM -0700, Alex Williamson wrote: > > > > > There's a claim here about added complexity that I'm not really seeing. > > > It looks

Re: [PATCH v3 12/17] vfio/migration: Implement VFIO migration protocol v2

2022-11-28 Thread Alex Williamson
On Mon, 28 Nov 2022 15:40:23 -0400 Jason Gunthorpe wrote: > On Mon, Nov 28, 2022 at 11:50:03AM -0700, Alex Williamson wrote: > > > There's a claim here about added complexity that I'm not really seeing. > > It looks like we simply make an ioctl call here and scale our buffer > > based on the

Re: [PATCH v3 12/17] vfio/migration: Implement VFIO migration protocol v2

2022-11-28 Thread Jason Gunthorpe
On Mon, Nov 28, 2022 at 11:50:03AM -0700, Alex Williamson wrote: > There's a claim here about added complexity that I'm not really seeing. > It looks like we simply make an ioctl call here and scale our buffer > based on the minimum of the returned device estimate or our upper > bound. I'm not

Re: [PATCH v3 12/17] vfio/migration: Implement VFIO migration protocol v2

2022-11-28 Thread Alex Williamson
On Thu, 24 Nov 2022 14:41:00 +0200 Avihai Horon wrote: > On 20/11/2022 11:34, Avihai Horon wrote: > > > > On 17/11/2022 19:38, Alex Williamson wrote: > >> External email: Use caution opening links or attachments > >> > >> > >> On Thu, 17 Nov 2022 19:07:10 +0200 > >> Avihai Horon wrote: >

[PATCH v3 5/7] vhost: enable vrings in vhost_dev_start() for vhost-user devices

2022-11-28 Thread Alex Bennée
From: Stefano Garzarella Commit 02b61f38d3 ("hw/virtio: incorporate backend features in features") properly negotiates VHOST_USER_F_PROTOCOL_FEATURES with the vhost-user backend, but we forgot to enable vrings as specified in docs/interop/vhost-user.rst: If

[PATCH v3 7/7] hw/virtio: generalise CHR_EVENT_CLOSED handling

2022-11-28 Thread Alex Bennée
..and use for both virtio-user-blk and virtio-user-gpio. This avoids the circular close by deferring shutdown due to disconnection until a later point. virtio-user-blk already had this mechanism in place so generalise it as a vhost-user helper function and use for both blk and gpio devices. While

Re: [PATCH v3 2/3] block/vmdk: Simplify vmdk_co_create() to return directly

2022-11-28 Thread Juan Quintela
Markus Armbruster wrote: > Cc: Fam Zheng > Cc: Kevin Wolf > Cc: Hanna Reitz > Cc: qemu-block@nongnu.org > Signed-off-by: Markus Armbruster > Reviewed-by: Peter Maydell > Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela goto, uninitialized variable at declaraton and can use

[PATCH v7 13/14] block: convert bdrv_create to co_wrapper

2022-11-28 Thread Emanuele Giuseppe Esposito
This function is never called in coroutine context, therefore instead of manually creating a new coroutine, delegate it to the block-coroutine-wrapper script, defining it as co_wrapper. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy

[PATCH v7 05/14] block/vmdk: add coroutine_fn annotations

2022-11-28 Thread Emanuele Giuseppe Esposito
These functions end up calling bdrv_create() implemented as generated_co_wrapper functions. In addition, they also happen to be always called in coroutine context, meaning all callers are coroutine_fn. This means that the g_c_w function will enter the qemu_in_coroutine() case and eventually

[PATCH v7 12/14] block-coroutine-wrapper.py: support also basic return types

2022-11-28 Thread Emanuele Giuseppe Esposito
Extend the regex to cover also return type, pointers included. This implies that the value returned by the function cannot be a simple "int" anymore, but the custom return type. Therefore remove poll_state->ret and instead use a per-function custom "ret" field. Signed-off-by: Emanuele Giuseppe

[PATCH v7 09/14] block: rename generated_co_wrapper in co_wrapper_mixed

2022-11-28 Thread Emanuele Giuseppe Esposito
In preparation to the incoming new function specifiers, rename g_c_w with a more meaningful name and document it. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy --- docs/devel/block-coroutine-wrapper.rst | 6 +-- block/coroutines.h | 4

[PATCH v7 14/14] block/dirty-bitmap: convert coroutine-only functions to co_wrapper

2022-11-28 Thread Emanuele Giuseppe Esposito
bdrv_can_store_new_dirty_bitmap and bdrv_remove_persistent_dirty_bitmap check if they are running in a coroutine, directly calling the coroutine callback if it's the case. Except that no coroutine calls such functions, therefore that check can be removed, and function creation can be offloaded to

[PATCH v7 11/14] block-coroutine-wrapper.py: support functions without bs arg

2022-11-28 Thread Emanuele Giuseppe Esposito
Right now, we take the first parameter of the function to get the BlockDriverState to pass to bdrv_poll_co(), that internally calls functions that figure in which aiocontext the coroutine should run. However, it is useless to pass a bs just to get its own AioContext, so instead pass it directly,

[PATCH v7 06/14] block: avoid duplicating filename string in bdrv_create

2022-11-28 Thread Emanuele Giuseppe Esposito
We know that the string will stay around until the function returns, and the parameter of drv->bdrv_co_create_opts is const char*, so it must not be modified either. Suggested-by: Kevin Wolf Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Kevin Wolf Reviewed-by: Vladimir

[PATCH v7 08/14] block: bdrv_create_file is a coroutine_fn

2022-11-28 Thread Emanuele Giuseppe Esposito
It is always called in coroutine_fn callbacks, therefore it can directly call bdrv_co_create(). Rename it to bdrv_co_create_file too. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/block/block-global-state.h | 3 ++-

[PATCH v7 07/14] block: distinguish between bdrv_create running in coroutine and not

2022-11-28 Thread Emanuele Giuseppe Esposito
Call two different functions depending on whether bdrv_create is in coroutine or not, following the same pattern as generated_co_wrapper functions. This allows to also call the coroutine function directly, without using CreateCo or relying in bdrv_create(). Signed-off-by: Emanuele Giuseppe

[PATCH v7 02/14] block-copy: add coroutine_fn annotations

2022-11-28 Thread Emanuele Giuseppe Esposito
These functions end up calling bdrv_common_block_status_above(), a generated_co_wrapper function. In addition, they also happen to be always called in coroutine context, meaning all callers are coroutine_fn. This means that the g_c_w function will enter the qemu_in_coroutine() case and eventually

[PATCH v7 01/14] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers

2022-11-28 Thread Emanuele Giuseppe Esposito
bdrv_common_block_status_above() is a g_c_w, and it is being called by many "wrapper" functions like bdrv_is_allocated(), bdrv_is_allocated_above() and bdrv_block_status_above(). Because we want to eventually split the coroutine from non-coroutine case in g_c_w, create duplicate wrappers that

[PATCH v7 10/14] block-coroutine-wrapper.py: introduce co_wrapper

2022-11-28 Thread Emanuele Giuseppe Esposito
This new annotation starts just a function wrapper that creates a new coroutine. It assumes the caller is not a coroutine. It will be the default annotation to be used in the future. This is much better as c_w_mixed, because it is clear if the caller is a coroutine or not, and provides the

[PATCH v7 04/14] block-backend: replace bdrv_*_above with blk_*_above

2022-11-28 Thread Emanuele Giuseppe Esposito
Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts for bdrv_block_status_above and bdrv_is_allocated_above. Note that since blk_co_block_status_above only calls the g_c_w function bdrv_common_block_status_above and is marked as coroutine_fn, call directly

[PATCH v7 03/14] nbd/server.c: add coroutine_fn annotations

2022-11-28 Thread Emanuele Giuseppe Esposito
These functions end up calling bdrv_*() implemented as generated_co_wrapper functions. In addition, they also happen to be always called in coroutine context, meaning all callers are coroutine_fn. This means that the g_c_w function will enter the qemu_in_coroutine() case and eventually suspend (or

[PATCH v7 00/14] Still more coroutine and various fixes in block layer

2022-11-28 Thread Emanuele Giuseppe Esposito
This is a dump of all minor coroutine-related fixes found while looking around and testing various things in the QEMU block layer. Patches aim to: - add missing coroutine_fn annotation to the functions - simplify to avoid the typical "if in coroutine: fn() // else create_coroutine(fn)" already

[PATCH v2 5/5] qemu-img: Speed up checksum

2022-11-28 Thread Nir Soffer
Add coroutine based loop inspired by `qemu-img convert` design. Changes compared to `qemu-img convert`: - State for the entire image is kept in ImgChecksumState - State for single worker coroutine is kept in ImgChecksumworker. - "Writes" are always in-order, ensured using a queue. - Calling

[PATCH v2 1/5] qemu-img.c: Move IO_BUF_SIZE to the top of the file

2022-11-28 Thread Nir Soffer
This macro is used by various commands (compare, convert, rebase) but it is defined somewhere in the middle of the file. I'm going to use it in the new checksum command so lets clean up a bit before that. --- qemu-img.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git

[PATCH v2 4/5] iotests: Test qemu-img checksum

2022-11-28 Thread Nir Soffer
Add simple tests computing a checksum for image with all kinds of extents in raw and qcow2 formats. The test can be extended later for other formats, format options (e..g compressed qcow2), protocols (e.g. nbd), and image with a backing chain, but I'm not sure this is really needed. To help

[PATCH v2 2/5] Support format or cache specific out file

2022-11-28 Thread Nir Soffer
Extend the test finder to find tests with format (*.out.qcow2) or cache specific (*.out.nocache) out file. This worked before only for the numbered tests. --- tests/qemu-iotests/findtests.py | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git

[PATCH v2 0/5] Add qemu-img checksum command using blkhash

2022-11-28 Thread Nir Soffer
Since blkhash is available only via copr now, the new command is added as optional feature, built only if blkhash-devel package is installed. Changes since v1 (Hanna): - Move IO_BUF_SIZE to top of the file - Extend TestFinder to support format or cache specific out files - Improve online help

[PATCH v2 3/5] qemu-img: Add checksum command

2022-11-28 Thread Nir Soffer
The checksum command compute a checksum for disk image content using the blkhash library[1]. The blkhash library is not packaged yet, but it is available via copr[2]. Example run: $ ./qemu-img checksum -p fedora-35.qcow2 6e5c00c995056319d52395f8d91c7f84725ae3da69ffcba4de4c7d22cff713a5

Re: [PATCH v2 14/15] block: Don't poll in bdrv_replace_child_noperm()

2022-11-28 Thread Kevin Wolf
Am 25.11.2022 um 17:07 hat Vladimir Sementsov-Ogievskiy geschrieben: > On 11/18/22 20:41, Kevin Wolf wrote: > > In order to make sure that bdrv_replace_child_noperm() doesn't have to > > poll any more, get rid of the bdrv_parent_drained_begin_single() call. > > > > This is possible now because we

Re: [PATCH v2 00/15] block: Simplify drain

2022-11-28 Thread Kevin Wolf
Am 18.11.2022 um 18:40 hat Kevin Wolf geschrieben: > I'm aware that exactly nobody has been looking forward to a series with > this title, but it has to be. The way drain works means that we need to > poll in bdrv_replace_child_noperm() and that makes things rather messy > with Emanuele's

Re: [PATCH v2 11/15] block: Call drain callbacks only once

2022-11-28 Thread Kevin Wolf
Am 25.11.2022 um 15:59 hat Vladimir Sementsov-Ogievskiy geschrieben: > On 11/18/22 20:41, Kevin Wolf wrote: > > We only need to call both the BlockDriver's callback and the parent > > callbacks when going from undrained to drained or vice versa. A second > > drain section doesn't make a difference

Re: [PATCH 2/3] iotests: Test qemu-img checksum

2022-11-28 Thread Nir Soffer
On Mon, Nov 7, 2022 at 1:41 PM Hanna Reitz wrote: > On 30.10.22 18:38, Nir Soffer wrote: > > On Wed, Oct 26, 2022 at 4:31 PM Hanna Reitz wrote: > > > > On 01.09.22 16:32, Nir Soffer wrote: > > > Add simple tests creating an image with all kinds of extents, > > different > > >

Re: [PATCH 1/3] qemu-img: Add checksum command

2022-11-28 Thread Nir Soffer
On Mon, Nov 7, 2022 at 12:20 PM Hanna Reitz wrote: > On 30.10.22 18:37, Nir Soffer wrote: > > On Wed, Oct 26, 2022 at 4:00 PM Hanna Reitz wrote: > > > > On 01.09.22 16:32, Nir Soffer wrote: > [...] > > > --- > > > docs/tools/qemu-img.rst | 22 + > > > meson.build

[PATCH v3 2/3] block/vmdk: Simplify vmdk_co_create() to return directly

2022-11-28 Thread Markus Armbruster
Cc: Fam Zheng Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé --- block/vmdk.c | 28 +++- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git

Re: [PATCH v6 10/14] block-coroutine-wrapper.py: introduce co_wrapper

2022-11-28 Thread Emanuele Giuseppe Esposito
Am 25/11/2022 um 21:32 schrieb Vladimir Sementsov-Ogievskiy: > >>     class FuncDecl: >> -    def __init__(self, return_type: str, name: str, args: str) -> None: >> +    def __init__(self, return_type: str, name: str, args: str, >> + variant: str) -> None: > > I'd prefer

Re: [PATCH v6 07/14] block: distinguish between bdrv_create running in coroutine and not

2022-11-28 Thread Emanuele Giuseppe Esposito
Am 25/11/2022 um 19:03 schrieb Vladimir Sementsov-Ogievskiy: > On 11/25/22 16:35, Emanuele Giuseppe Esposito wrote: >> Call two different functions depending on whether bdrv_create >> is in coroutine or not, following the same pattern as >> generated_co_wrapper functions. >> >> This allows to