[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] scsi: add block job opblockers
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] scsi: add block job opblockers for scsi-block |
Date: |
Thu, 5 Apr 2018 14:43:17 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 05.04.2018 um 13:59 hat Paolo Bonzini geschrieben:
> On 12/03/2018 12:58, Kevin Wolf wrote:
> >> - users of dirty bitmaps would call use perm/shared_perm as in your
> >> message above
> >>
> >> - dirty bitmaps creation calls bdrv_get_cumulative_perm (which should
> >> now become public) and checks that it doesn't have BLK_PERM_BYPASS in
> >> shared_perm
> >
> > My proposal was really that users of dirty bitmaps don't change
> > anything, but we do everything in the dirty bitmap implementation. Dirty
> > bitmap creation would add a BdrvChild with the above permissions.
> > Deleting a dirty bitmap would remove the BdrvChild again.
>
> This is also better because it works if somebody requests
> BLK_PERM_BYPASS after dirty bitmap creation.
>
> However, is there any better way than also creating a dummy BlockDriver
> and BlockDriverState? I first thought of a root role similar to
> BlockBackend's, but BdrvChildRole doesn't have a way to inject its own
> permissions. I then tried moving bdrv_child_perm to BdrvChildRole, and
> that almost works except that child_backing has special requirements
> (mostly due to commit_top and mirror_top's special block drivers).
Have a look at block_job_add_bdrv(), which does the same thing to
restrict permissions while a block job is working on the subchain.
Essentially, yes, you'll probably have a new BdrvChildRole (I suppose
with .stay_at_node = true and .get_parent_desc only), but the place
where you specify the permissions is the bdrv_root_attach_child() call.
When you're done, you call bdrv_root_unref_child().
Kevin
> Perhaps child_perm should be in BdrvChildRole and BlockDriverState
> should only have a bdrv_get_backing_perm (called by
> child_backing.child_perm). This makes sense to me since those
> permissions are specific to the driver, e.g. whether it has metadata at
> all. But this becomes 2.13 material.
>
> Do you still object to the two opblocker patches?
>
> Paolo