qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 04/21] block/commit: utilize job_exit shim


From: Kevin Wolf
Subject: Re: [Qemu-block] [PATCH 04/21] block/commit: utilize job_exit shim
Date: Thu, 16 Aug 2018 08:41:28 +0200
User-agent: Mutt/1.9.1 (2017-09-22)

Am 15.08.2018 um 22:52 hat John Snow geschrieben:
> On 08/08/2018 12:29 PM, Kevin Wolf wrote:
> > Am 07.08.2018 um 06:33 hat John Snow geschrieben:
> >> Change the manual deferment to commit_complete into the implicit
> >> callback to job_exit.
> >>
> >> Signed-off-by: John Snow <address@hidden>
> > 
> > There is one tricky thing in this patch that the commit message could be
> > a bit more explicit about, which is moving job_completed() to a later
> > point.
> > 
> > This is the code that happens between the old call of job_completed()
> > and the new one:
> > 
> >     /* If bdrv_drop_intermediate() didn't already do that, remove the commit
> >      * filter driver from the backing chain. Do this as the final step so 
> > that
> >      * the 'consistent read' permission can be granted.  */
> >     if (remove_commit_top_bs) {
> >         bdrv_child_try_set_perm(commit_top_bs->backing, 0, BLK_PERM_ALL,
> >                                 &error_abort);
> >         bdrv_replace_node(commit_top_bs, backing_bs(commit_top_bs),
> >                           &error_abort);
> >     }
> > 
> >     bdrv_unref(commit_top_bs);
> >     bdrv_unref(top);
> > 
> > As the comment states, bdrv_replace_node() requires that the permission
> > restrictions that the commit job made are already lifted. The most
> > important part is done by the explicit block_job_remove_all_bdrv() call
> > right before this hunk. It still leaves bjob->blk around, which could
> > have implications, but luckily we didn't take any permissions for that
> > one:
> > 
> >     s = block_job_create(job_id, &commit_job_driver, NULL, bs, 0, 
> > BLK_PERM_ALL,
> >                          speed, JOB_DEFAULT, NULL, NULL, errp);
> > 
> > So I think we got everything out of the way and bdrv_replace_node() can
> > do what it wants to do.
> > 
> > Kevin
> > 
> 
> I suppose it will be up to the author of a job to be aware of any
> permissions they pick up at creation time that might have an effect on
> the cleanup they wish to do during completion time.

I'm really only talking about the commit job, the specific conversion
in this patch and the terse commit message.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]