Re: [v2 3/4] crypto: Support generic LUKS encryption

["Daniel P. Berrangé" <berrange@redhat.com>]

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é <berrange@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 :|