[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: |
Sat, 25 Aug 2018 15:07:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-08-22 23:55, John Snow wrote:
>
>
> On 08/22/2018 07:58 AM, Max Reitz wrote:
>> 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
>>
>
> Sorry, I need a better cover letter.
My mistake, I need to read more than the first few lines of a cover letter.
> .exit() is going away, and either .prepare() or .abort() will be called
> after .run(), so what you're asking for will be true, just not all at
> once in this series.
Yay!
Max
signature.asc
Description: OpenPGP digital signature
[Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, John Snow, 2018/08/17
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, Max Reitz, 2018/08/22
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, Max Reitz, 2018/08/22
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, John Snow, 2018/08/22
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, Max Reitz, 2018/08/25
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, Max Reitz, 2018/08/25
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, John Snow, 2018/08/28
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, Max Reitz, 2018/08/29
- Re: [Qemu-block] [PATCH 5/7] block/mirror: utilize job_exit shim, John Snow, 2018/08/28