[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 4/7] block/commit: utilize job_exit
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 4/7] block/commit: utilize job_exit shim |
Date: |
Wed, 22 Aug 2018 13:58:51 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
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 <address@hidden>
>> ---
>> 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
[Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, John Snow, 2018/08/17