[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine |
Date: |
Fri, 17 Jan 2020 06:44:53 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Am 16.01.2020 um 16:13 hat Markus Armbruster geschrieben:
>> Kevin Wolf <address@hidden> writes:
>>
>> > Am 16.01.2020 um 10:45 hat Markus Armbruster geschrieben:
>> >> Kevin Wolf <address@hidden> writes:
>> >> > block_resize is safe to run in a coroutine, so use it as an example for
>> >> > the new 'coroutine': true annotation in the QAPI schema.
>> >> >
>> >> > Signed-off-by: Kevin Wolf <address@hidden>
>> >> > Reviewed-by: Marc-André Lureau <address@hidden>
>> >
>> >> > diff --git a/blockdev.c b/blockdev.c
>> >> > index 8e029e9c01..b5e5d1e072 100644
>> >> > --- a/blockdev.c
>> >> > +++ b/blockdev.c
>> >> > @@ -3161,9 +3161,9 @@ void hmp_drive_del(Monitor *mon, const QDict
>> >> > *qdict)
>> >> > aio_context_release(aio_context);
>> >> > }
>> >> >
>> >> > -void qmp_block_resize(bool has_device, const char *device,
>> >> > - bool has_node_name, const char *node_name,
>> >> > - int64_t size, Error **errp)
>> >> > +void coroutine_fn qmp_block_resize(bool has_device, const char *device,
>> >> > + bool has_node_name, const char
>> >> > *node_name,
>> >> > + int64_t size, Error **errp)
>> >> > {
>> >> > Error *local_err = NULL;
>> >> > BlockBackend *blk = NULL;
>> >>
>> >> Pardon my ignorant question: what exactly makes a function a
>> >> coroutine_fn?
>> >
>> > When Stefan requested adding the coroutine_fn marker, it seemed to make
>> > sense to me because the QMP dispatcher will always call it from
>> > coroutine context now, and being always run in coroutine context makes a
>> > function a coroutine_fn.
>> >
>> > However, it's also called from hmp_block_resize(), so at least for now
>> > coroutine_fn is actually wrong.
>>
>> This answers the question when we mark a function a coroutine_fn. I
>> meant to ask what conditions the function itself must satisfy to be
>> eligible for this mark.
>
> The requirement is actually not about the function itself, it's about
> the callers, as stated above.
>
> But being a coroutine_fn allows the function to call other functions
> that only work in coroutine context (other coroutine_fns). In the end
> the reason why a function only works in coroutine context is usually
> that it (or any other coroutine_fns called by it) could yield, which
> obviously doesn't work outside of coroutine contest.
Thanks.
I think "being always run in coroutine context makes a function a
coroutine_fn" is inaccurate. It's "calling a coroutine_fn without
switching to coroutine context first when not already in coroutine
context". The induction terminates at basic coroutine_fn like
qemu_coroutine_yield().
Pertinent:
/**
* Return whether or not currently inside a coroutine
*
* This can be used to write functions that work both when in coroutine
context
* and when not in coroutine context. Note that such functions cannot use
the
* coroutine_fn annotation since they work outside coroutine context.
*/
bool qemu_in_coroutine(void);
For qmp_block_resize(), it's used like this, in bdrv_truncate():
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_truncate_co_entry(&tco);
} else {
co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco);
bdrv_coroutine_enter(child->bs, co);
BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE);
}
where bdrv_truncate_co_entry() is a coroutine_fn.
- Re: [PATCH v3 1/4] qapi: Add a 'coroutine' flag for commands, (continued)
- [PATCH v3 2/4] vl: Initialise main loop earlier, Kevin Wolf, 2020/01/15
- [PATCH v3 3/4] qmp: Move dispatcher to a coroutine, Kevin Wolf, 2020/01/15
- [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Kevin Wolf, 2020/01/15
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Markus Armbruster, 2020/01/16
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Kevin Wolf, 2020/01/16
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Markus Armbruster, 2020/01/16
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Kevin Wolf, 2020/01/16
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine,
Markus Armbruster <=
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Kevin Wolf, 2020/01/17
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Markus Armbruster, 2020/01/17
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Markus Armbruster, 2020/01/17
- Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine, Kevin Wolf, 2020/01/17