[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/21] block/commit: utilize job_exit shim
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [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
- [Qemu-devel] [PATCH 20/21] qapi/block-mirror: expose new job properties, (continued)
- [Qemu-devel] [PATCH 20/21] qapi/block-mirror: expose new job properties, John Snow, 2018/08/07
- [Qemu-devel] [PATCH 12/21] jobs: allow entrypoints to return status code, John Snow, 2018/08/07
- [Qemu-devel] [PATCH 07/21] block/create: utilize job_exit shim, John Snow, 2018/08/07
- [Qemu-devel] [PATCH 06/21] block/stream: utilize job_exit shim, John Snow, 2018/08/07
- [Qemu-devel] [PATCH 04/21] block/commit: utilize job_exit shim, John Snow, 2018/08/07
- [Qemu-devel] [PATCH 01/21] jobs: canonize Error object, John Snow, 2018/08/07
[Qemu-devel] [PATCH 11/21] jobs: remove job_defer_to_main_loop, John Snow, 2018/08/07
[Qemu-devel] [PATCH 09/21] tests/test-blockjob: utilize job_exit shim, John Snow, 2018/08/07
[Qemu-devel] [PATCH 08/21] tests/test-blockjob-txn: utilize job_exit shim, John Snow, 2018/08/07
[Qemu-devel] [PATCH 13/21] block/commit: add block job creation flags, John Snow, 2018/08/07
[Qemu-devel] [PATCH 05/21] block/mirror: utilize job_exit shim, John Snow, 2018/08/07