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 p
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 Esposi
This new annotation creates just a function wrapper that creates
a new coroutine. It assumes the caller is not a coroutine.
This is much better as g_c_w, because it is clear if the caller
is a coroutine or not, and provides the advantage of automating
the code creation. In the future all g_c_w fun
Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts
for bdrv_block_status_above and bdrv_is_allocated_above.
Note that since bdrv_block_status_above only calls the g_c_w function
bdrv_common_block_status_above and is marked as coroutine_fn, call
directly bdrv_co_common_block_sta
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 take
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 s
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
---
block.c| 5 +++--
block/crypto.c | 2 +-
block/parallels.c
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 Esp
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, a
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
g
Am 18/11/2022 um 11:57 schrieb Paolo Bonzini:
> On 11/16/22 15:07, Emanuele Giuseppe Esposito wrote:
>> Here we introduce generated_co_wrapper_simple, a simplification of
>> g_c_w that
>> only considers the case where the caller is not in a coroutine.
>> This simplifies and clarifies a lot when
Basically BdrvPollCo->bs is only used by bdrv_poll_co(), and the
functions that it uses are both using bdrv_get_aio_context, that
defaults to qemu_get_aio_context() if bs is NULL.
Therefore pass NULL to BdrvPollCo to automatically generate a function
that create and runs a coroutine in the main lo
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 g_c_w_simple.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Kevin Wolf
---
block.c|
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 suspend
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
This macro will be used to mark all coroutine_fn functions.
Right now, it will be used for the newly introduced coroutine_fn, since
we know the callers.
As a TODO, in the future we might want to add this macro to all
corotuine_fn functions, to be sure that they are only called in
coroutines contex
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
---
block.c | 7 ++-
1 file changed, 2 insertions(+), 5 de
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 ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, t
Am 23.11.2022 um 12:45 hat Emanuele Giuseppe Esposito geschrieben:
>
>
> Am 18/11/2022 um 11:57 schrieb Paolo Bonzini:
> > On 11/16/22 15:07, Emanuele Giuseppe Esposito wrote:
> >> Here we introduce generated_co_wrapper_simple, a simplification of
> >> g_c_w that
> >> only considers the case wher
On Tue, Nov 22, 2022 at 07:01:38PM +0100, Eugenio Perez Martin wrote:
On Tue, Nov 22, 2022 at 4:13 AM Jason Wang wrote:
On Mon, Nov 21, 2022 at 6:11 PM Stefano Garzarella wrote:
>
> Commit 69e1c14aa2 ("virtio: core: vq reset feature negotation support")
> enabled VIRTIO_F_RING_RESET by defaul
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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 co
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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 tha
* Juan Quintela (quint...@redhat.com) wrote:
> Until previous commit, save_live_pending() was used for ram. Now with
> the split into state_pending_estimate() and state_pending_exact() it
> is not needed anymore, so remove them.
>
> Signed-off-by: Juan Quintela
> ---
> include/migration/registe
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts
> for bdrv_block_status_above and bdrv_is_allocated_above.
>
> Note that since bdrv_block_status_above only calls the g_c_w function
> bdrv_common_block_statu
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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 Giu
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> This macro will be used to mark all coroutine_fn functions.
> Right now, it will be used for the newly introduced coroutine_fn, since
> we know the callers.
>
> As a TODO, in the future we might want to add this macro to all
> co
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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
On 11/23/22 14:45, Kevin Wolf wrote:
I think this means that if we clean up everything, in the end we'll have
coroutine_wrapper and coroutine_wrapper_bdrv (the fourth version not in
the above list, but that Paolo mentioned we may want to have).
Yes, I agree.
The only thing I'm unsure about is
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> This new annotation creates just a function wrapper that creates
> a new coroutine. It assumes the caller is not a coroutine.
>
> This is much better as g_c_w, because it is clear if the caller
> is a coroutine or not, and provid
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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 useles
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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 u
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/migration.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> ind
Am 23.11.2022 um 18:04 hat Paolo Bonzini geschrieben:
> On 11/23/22 14:45, Kevin Wolf wrote:
> > I think this means that if we clean up everything, in the end we'll have
> > coroutine_wrapper and coroutine_wrapper_bdrv (the fourth version not in
> > the above list, but that Paolo mentioned we may w
* Avihai Horon (avih...@nvidia.com) wrote:
>
> On 08/11/2022 19:52, Vladimir Sementsov-Ogievskiy wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On 11/3/22 19:16, Avihai Horon wrote:
> > > From: Juan Quintela
> > >
> > > It was only used for RAM, and in that cas
* Avihai Horon (avih...@nvidia.com) wrote:
> +ret = qemu_file_get_to_fd(f, migration->data_fd, data_size);
> +if (!ret) {
> +trace_vfio_load_state_device_data(vbasedev->name, data_size);
> +
> +}
I notice you had a few cases like that; I wouldn't bother making that
condition
> On Nov 23, 2022, at 8:16 AM, Stefano Garzarella wrote:
>
> 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
On Thu, Nov 24, 2022 at 12:19:25AM +, Raphael Norwitz wrote:
>
> > On Nov 23, 2022, at 8:16 AM, Stefano Garzarella wrote:
> >
> > Commit 02b61f38d3 ("hw/virtio: incorporate backend features in features")
> > properly negotiates VHOST_USER_F_PROTOCOL_FEATURES with the vhost-user
> > backend,
Il mer 23 nov 2022, 17:49 Kevin Wolf ha scritto:
> I already asked about other opinions on this in patch 1.
>
> These assertions are runtime checks and I don't feel they are the right
> tool to verify coroutine_fn consistency. Asserting in tricky places
> makes sense to me, especially as long as
Il mer 23 nov 2022, 17:29 Kevin Wolf ha scritto:
> As I said, personally, I don't feel like putting QEMU_IN_COROUTINE()
> assertions into every coroutine_fn is a useful thing to do. Static
> analysis (maybe even with something vrc based in 'make check'? Paolo,
> would this be realistic?)
Yes, u
On Thu, Nov 24, 2022 at 01:50:19AM -0500, Michael S. Tsirkin wrote:
On Thu, Nov 24, 2022 at 12:19:25AM +, Raphael Norwitz wrote:
> On Nov 23, 2022, at 8:16 AM, Stefano Garzarella wrote:
>
> Commit 02b61f38d3 ("hw/virtio: incorporate backend features in features")
> properly negotiates VHOS
41 matches
Mail list logo
