On Mon, Dec 18, 2023 at 10:15:34PM +0800, Yong Huang wrote:
> On Mon, Dec 18, 2023 at 7:16 PM Daniel P. Berrangé <berra...@redhat.com>
> wrote:
> > > @@ -903,6 +928,17 @@ block_crypto_child_perms(BlockDriverState *bs,
> > BdrvChild *c,
> > >
> > > BlockCrypto *crypto = bs->opaque;
> > >
> > > + if (role == (role & BDRV_CHILD_METADATA)) {
> > > + /* Assign read permission only */
> > > + perm |= BLK_PERM_CONSISTENT_READ;
> > > + /* Share all permissions */
> > > + shared |= BLK_PERM_ALL;
> > > +
> > > + *nperm = perm;
> > > + *nshared = shared;
> > > + return;
> > > + }
> >
> > What is code doing ? You've set BDRV_CHILD_METADATA role on the
> > crypto->header object, but how does this block_crypto_child_perms
> > method get run against crypto->header ?
> >
> This paragraph originally aims to provide a function that multiple disks
> could share
> the same LUKS header (with read-only permission).
> But I've found that it may not work when updating the detached header after
> reviewing the patch recently :(.
>
> Should it be a separate commit, Since it kind of has nothing to do with the
> basic
> function?
Yes, that would be better as a separate commit.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
