On Tue, Dec 5, 2023 at 12:51 AM Daniel P. Berrangé <berra...@redhat.com> wrote:
> On Tue, Dec 05, 2023 at 12:41:16AM +0800, Yong Huang wrote: > > On Tue, Dec 5, 2023 at 12:24 AM Daniel P. Berrangé <berra...@redhat.com> > > wrote: > > > > > On Tue, Dec 05, 2023 at 12:06:17AM +0800, Hyman Huang wrote: > > > > This functionality was motivated by the following to-do list seen > > > > in crypto documents: > > > > https://wiki.qemu.org/Features/Block/Crypto > > > > > > > > The last chapter says we should "separate header volume": > > > > > > > > The LUKS format has ability to store the header in a separate volume > > > > from the payload. We should extend the LUKS driver in QEMU to support > > > > this use case. > > > > > > > > As a proof-of-concept, I've created this patchset, which I've named > > > > the Gluks: generic luks. As their name suggests, they offer > encryption > > > > for any format that QEMU theoretically supports. > > > > > > I don't see the point in creating a new driver. > > > > > > I would expect detached header support to be implemented via an > > > optional new 'header' field in the existing driver. ie > > > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > > index ca390c5700..48d1f2a974 100644 > > > --- a/qapi/block-core.json > > > +++ b/qapi/block-core.json > > > @@ -3352,11 +3352,15 @@ > > > # decryption key (since 2.6). Mandatory except when doing a > > > # metadata-only probe of the image. > > > # > > > +# @header: optional reference to the location of a blockdev > > > +# storing a detached LUKS heaer > > > +# > > > # Since: 2.9 > > > ## > > > { 'struct': 'BlockdevOptionsLUKS', > > > 'base': 'BlockdevOptionsGenericFormat', > > > - 'data': { '*key-secret': 'str' } } > > > + 'data': { '*key-secret': 'str', > > > + "*header-file': 'BlockdevRef'} } > > > > > > ## > > > # @BlockdevOptionsGenericCOWFormat: > > > @@ -4941,9 +4945,18 @@ > > > # > > > # Driver specific image creation options for LUKS. > > > # > > > -# @file: Node to create the image format on > > > +# @file: Node to create the image format on. Mandatory > > > +# unless a detached header file is specified using > > > +# @header. > > > # > > > -# @size: Size of the virtual disk in bytes > > > +# @size: Size of the virtual disk in bytes. Mandatory > > > +# unless a detached header file is specified using > > > +# @header. > > > +# > > > +# @header: optional reference to the location of a blockdev > > > +# storing a detached LUKS heaer. The @file option is > > > +# is optional when this is given, unless it is desired > > > +# to perform pre-allocation > > > # > > > # @preallocation: Preallocation mode for the new image (since: 4.2) > > > # (default: off; allowed values: off, metadata, falloc, full) > > > @@ -4952,8 +4965,9 @@ > > > ## > > > { 'struct': 'BlockdevCreateOptionsLUKS', > > > 'base': 'QCryptoBlockCreateOptionsLUKS', > > > - 'data': { 'file': 'BlockdevRef', > > > - 'size': 'size', > > > + 'data': { '*file': 'BlockdevRef', > > > + '*size': 'size', > > > + '*header': 'BlockdevRef' > > > '*preallocation': 'PreallocMode' } } > > > > > > ## > > > > > > It ends up giving basicallly the same workflow as you outline, > > > without needing the new block driver > > > > > > > How about the design and usage, could it be simpler? Any advice? :) > > > > > > As you can see below, the Gluks format block layer driver's design is > > quite simple. > > > > virtio-blk/vhost-user-blk...(front-end device) > > ^ > > | > > Gluks (format-like disk node) > > / \ > > file header (blockdev reference) > > / \ > > file file (protocol node) > > | | > > disk data Luks data > > What I suggested above ends up with the exact same block driver > graph, unless I'm missing something. > I could overlook something or fail to adequately convey the goal of the patchset. :( Indeed, utilizing the same block driver might be effective if our only goal is to divide the header volume, giving us an additional way to use Luks. While supporting encryption for any disk format that QEMU is capable of supporting is another feature of this patchset. This implies that we might link the Luks header to other blockdev references, which might alter how the Luks are used and make them incompatible with it. It's not user-friendly in my opinion, and I'm not aware of a more elegant solution. > 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 :| > > -- Best regards
