On 2018-08-17 21:18, John Snow wrote:
>
>
> On 08/17/2018 03:04 PM, John Snow wrote:
>> Change the manual deferment to commit_complete into the implicit
>> callback to job_exit, renaming commit_complete to commit_exit.
>>
>> This conversion does change the timing of when job_completed is
>> called to after the bdrv_replace_node and bdrv_unref calls, which
>> could have implications for bjob->blk which will now be put down
>> after this cleanup.
>>
>> Kevin highlights that we did not take any permissions for that backend
>> at job creation time, so it is safe to reorder these operations.
>>
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>> block/commit.c | 20 ++++----------------
>> 1 file changed, 4 insertions(+), 16 deletions(-)
>>
>> diff --git a/block/commit.c b/block/commit.c
>> index 4a17bb73ec..93c3b6a39e 100644
>> --- a/block/commit.c
>> +++ b/block/commit.c
>> @@ -68,19 +68,13 @@ static int coroutine_fn commit_populate(BlockBackend
>> *bs, BlockBackend *base,
>> return 0;
>> }
>>
>> -typedef struct {
>> - int ret;
>> -} CommitCompleteData;
>> -
>> -static void commit_complete(Job *job, void *opaque)
>> +static void commit_exit(Job *job)
>> {
>> CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
>> BlockJob *bjob = &s->common;
>> - CommitCompleteData *data = opaque;
>> BlockDriverState *top = blk_bs(s->top);
>> BlockDriverState *base = blk_bs(s->base);
>> BlockDriverState *commit_top_bs = s->commit_top_bs;
>> - int ret = data->ret;
>> bool remove_commit_top_bs = false;
>>
>> /* Make sure commit_top_bs and top stay around until
>> bdrv_replace_node() */
>> @@ -93,8 +87,8 @@ static void commit_complete(Job *job, void *opaque)
>>
>> if (!job_is_cancelled(job) && ret == 0) {
>
> forgot to actually squash the change in here that replaces `ret` with
> `job->ret`.A better interface would be one function that is called when .run() was successful, and one that is called when it was not. (They can still resolve into a single function in the job that is just called with a boolean that's either true or false, but accessing *job to find out whether .run() was successful or not seems kind of convoluted to me.) Max
signature.asc
Description: OpenPGP digital signature
