Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
On Wed, Feb 28, 2024 at 7:58 PM Kevin Wolf wrote: > Am 28.02.2024 um 12:30 hat Daniel P. Berrangé geschrieben: > > On Wed, Feb 28, 2024 at 11:17:37AM +0100, Kevin Wolf wrote: > > > Until now, @size has been mandatory for creating images with every > > > driver. Maybe we should even have put it into BlockdevCreateOptions's > > > base, because without a size, you're not really creating an image. > > > > NB, @size isn't mandatory for creating images. It isn't required > > when creating qcow2 files with backing stores, as the size is > > acquired from the backing file instead. > > > > $ qemu-img create demo.raw 1g > > Formatting 'demo.raw', fmt=raw size=1073741824 > > $ qemu-img create -o backing_file=demo.raw -o backing_fmt=raw -f qcow2 > demo.qcow2 > > Formatting 'demo.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off > compression_type=zlib size=1073741824 backing_file=demo.raw backing_fmt=raw > lazy_refcounts=off refcount_bits=16 > > Yes, 'qemu-img create' is different, it adds some convenience magic like > this. But for 'blockdev-create', it's mandatory for every driver. I > double checked the QAPI schema. > > Kevin > > IMHO, LUKS detached header creation is not the case as qcow2 backing store creation in aspect of allowing @size absent, since qemu-img creation process fetch the @size from backing file as Daniel said above actually, while contrastively LUKS detached header creation with no need for @size. Making @size optional feels better for me. Yong -- Best regards
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Am 28.02.2024 um 12:30 hat Daniel P. Berrangé geschrieben: > On Wed, Feb 28, 2024 at 11:17:37AM +0100, Kevin Wolf wrote: > > Until now, @size has been mandatory for creating images with every > > driver. Maybe we should even have put it into BlockdevCreateOptions's > > base, because without a size, you're not really creating an image. > > NB, @size isn't mandatory for creating images. It isn't required > when creating qcow2 files with backing stores, as the size is > acquired from the backing file instead. > > $ qemu-img create demo.raw 1g > Formatting 'demo.raw', fmt=raw size=1073741824 > $ qemu-img create -o backing_file=demo.raw -o backing_fmt=raw -f qcow2 > demo.qcow2 > Formatting 'demo.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off > compression_type=zlib size=1073741824 backing_file=demo.raw backing_fmt=raw > lazy_refcounts=off refcount_bits=16 Yes, 'qemu-img create' is different, it adds some convenience magic like this. But for 'blockdev-create', it's mandatory for every driver. I double checked the QAPI schema. Kevin
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
On Wed, Feb 28, 2024 at 11:17:37AM +0100, Kevin Wolf wrote: > Until now, @size has been mandatory for creating images with every > driver. Maybe we should even have put it into BlockdevCreateOptions's > base, because without a size, you're not really creating an image. NB, @size isn't mandatory for creating images. It isn't required when creating qcow2 files with backing stores, as the size is acquired from the backing file instead. $ qemu-img create demo.raw 1g Formatting 'demo.raw', fmt=raw size=1073741824 $ qemu-img create -o backing_file=demo.raw -o backing_fmt=raw -f qcow2 demo.qcow2 Formatting 'demo.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1073741824 backing_file=demo.raw backing_fmt=raw lazy_refcounts=off refcount_bits=16 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 :|
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
On Wed, Feb 28, 2024 at 12:21:02PM +0100, Kevin Wolf wrote: > Am 28.02.2024 um 11:23 hat Daniel P. Berrangé geschrieben: > > On Wed, Feb 28, 2024 at 11:17:37AM +0100, Kevin Wolf wrote: > > > Am 28.02.2024 um 07:43 hat Markus Armbruster geschrieben: > > > > Yong Huang writes: > > > > > > > > > On Wed, Feb 21, 2024 at 4:26 PM Markus Armbruster > > > > > wrote: > > > > > > > > > >> Yong Huang writes: > > > > >> > > > > >> > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster > > > > >> > > > > > >> wrote: > > > > >> > > > > > >> >> Hyman Huang writes: > > > > >> >> > > > > >> >> > Add comment in detail for commit 433957bb7f (qapi: > > > > >> >> > Make parameter 'file' optional for > > > > >> >> > BlockdevCreateOptionsLUKS). > > > > >> >> > > > > > >> >> > Signed-off-by: Hyman Huang > > > > >> >> > --- > > > > >> >> > qapi/block-core.json | 20 +++- > > > > >> >> > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > >> >> > > > > > >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json > > > > >> >> > index ab5a93a966..42b0840d43 100644 > > > > >> >> > --- a/qapi/block-core.json > > > > >> >> > +++ b/qapi/block-core.json > > > > >> >> > @@ -4973,7 +4973,25 @@ > > > > >> >> > ## > > > > >> >> > # @BlockdevCreateOptionsLUKS: > > > > >> >> > # > > > > >> >> > -# Driver specific image creation options for LUKS. > > > > >> >> > +# Driver specific image creation options for LUKS. Note that > > > > >> >> > +# @file is required if @preallocation is specified and equals > > > > >> >> > +# PREALLOC_MODE_ON. The following three scenarios determine how > > > > >> >> > +# creation logic behaves when @preallocation is either equal to > > > > >> >> > +# PREALLOC_MODE_OFF or is not given: > > > > >> >> > +# > > > > >> >> > +# 1) When @file is given only, format the block device > > > > >> >> > referenced > > > > >> >> > +# by @file as the LUKS specification and trunk it to the > > > > >> >> > @size. > > > > >> >> > > > > >> >> Do you mean "truncate it to @size"? > > > > >> >> > > > > >> > Yes, :( sorry for the spelling mistake. > > > > >> > > > > >> Writing good documentation in a second language is *hard*. All we > > > > >> can > > > > >> reasonably expect from contributors to try their best. And then we > > > > >> improve the text together in review. Just like we do for code :) > > > > >> > > > > >> >> > +# In this case, the @size should reflect amount of space > > > > >> >> > made > > > > >> >> > +# available to the guest, so the trunk size must take > > > > >> >> > account > > > > >> >> > +# of that which will be used by the crypto header. > > > > >> >> > +# > > > > >> >> > +# 2) When @header is given only, just format the block device > > > > >> >> > +# referenced by @header as the LUKS specification. > > > > >> >> > +# > > > > >> >> > +# 3) When both @file and @header are given, block device > > > > >> >> > +# referenced by @file should be trunked to @size, and block > > > > >> >> > +# device referenced by @header should be formatted as the > > > > >> >> > LUKS > > > > >> >> > +# specification. > > > > >> >> > # > > > > >> >> > # @file: Node to create the image format on, mandatory except > > > > >> >> > when > > > > >> >> > #'preallocation' is not requested > > > > >> >> > > > > >> >> Let's see whether I understand. > > > > >> >> > > > > >> >> blockdev-create with "driver": "luks" can work in three different > > > > >> >> ways: > > > > >> >> > > > > >> >> 1. Create an image with a LUKS header > > > > >> >> > > > > >> >> 2. Create just a detached LUKS header > > > > >> >> > > > > >> >> 3. Create an image and a detached LUKS header > > > > >> >> > > > > >> >> Correct? > > > > >> >> > > > > >> > > > > > >> > Yes > > > > >> > > > > > >> > > > > > >> >> @file and @header are BlockdevRef, which means they refer to > > > > >> >> existing > > > > >> >> images with arbitrary driver. Could be "file", "qcow2", or > > > > >> >> anything. > > > > >> >> > > > > >> >> Correct? > > > > >> >> > > > > >> > Yes > > > > >> > > > > > >> > > > > > >> >> > > > > >> >> To get 1., specify @file, but not @header. > > > > >> >> > > > > >> >> To get 2., specify @header, but not @file. > > > > >> >> > > > > >> >> To get 3., specify both. > > > > >> >> > > > > >> >> Specifying neither is an error. > > > > >> >> > > > > >> >> Correct? > > > > >> >> > > > > >> > > > > > >> > Yes > > > > >> > > > > > >> > > > > > >> >> In any case, @size is the logical size of the image (how much > > > > >> >> data it > > > > >> >> can hold). > > > > >> >> > > > > >> > > > > > >> > Yes > > > > >> > > > > > >> > > > > > >> >> > > > > >> >> With 1., the actual image size is a bit larger due to the LUKS > > > > >> >> header. > > > > >> >> The @file image is resized to that size: if it's shorter, it's > > > > >> >> grown, if > > > > >> >> it's longer, it's truncated. > > > > >> >> > > > > >> > > > > > >> > Yes > > > > >> > > > > > >> > > > > > >> >> With 2., @size is
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Am 28.02.2024 um 11:23 hat Daniel P. Berrangé geschrieben: > On Wed, Feb 28, 2024 at 11:17:37AM +0100, Kevin Wolf wrote: > > Am 28.02.2024 um 07:43 hat Markus Armbruster geschrieben: > > > Yong Huang writes: > > > > > > > On Wed, Feb 21, 2024 at 4:26 PM Markus Armbruster > > > > wrote: > > > > > > > >> Yong Huang writes: > > > >> > > > >> > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster > > > >> wrote: > > > >> > > > > >> >> Hyman Huang writes: > > > >> >> > > > >> >> > Add comment in detail for commit 433957bb7f (qapi: > > > >> >> > Make parameter 'file' optional for > > > >> >> > BlockdevCreateOptionsLUKS). > > > >> >> > > > > >> >> > Signed-off-by: Hyman Huang > > > >> >> > --- > > > >> >> > qapi/block-core.json | 20 +++- > > > >> >> > 1 file changed, 19 insertions(+), 1 deletion(-) > > > >> >> > > > > >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json > > > >> >> > index ab5a93a966..42b0840d43 100644 > > > >> >> > --- a/qapi/block-core.json > > > >> >> > +++ b/qapi/block-core.json > > > >> >> > @@ -4973,7 +4973,25 @@ > > > >> >> > ## > > > >> >> > # @BlockdevCreateOptionsLUKS: > > > >> >> > # > > > >> >> > -# Driver specific image creation options for LUKS. > > > >> >> > +# Driver specific image creation options for LUKS. Note that > > > >> >> > +# @file is required if @preallocation is specified and equals > > > >> >> > +# PREALLOC_MODE_ON. The following three scenarios determine how > > > >> >> > +# creation logic behaves when @preallocation is either equal to > > > >> >> > +# PREALLOC_MODE_OFF or is not given: > > > >> >> > +# > > > >> >> > +# 1) When @file is given only, format the block device > > > >> >> > referenced > > > >> >> > +# by @file as the LUKS specification and trunk it to the > > > >> >> > @size. > > > >> >> > > > >> >> Do you mean "truncate it to @size"? > > > >> >> > > > >> > Yes, :( sorry for the spelling mistake. > > > >> > > > >> Writing good documentation in a second language is *hard*. All we can > > > >> reasonably expect from contributors to try their best. And then we > > > >> improve the text together in review. Just like we do for code :) > > > >> > > > >> >> > +# In this case, the @size should reflect amount of space made > > > >> >> > +# available to the guest, so the trunk size must take account > > > >> >> > +# of that which will be used by the crypto header. > > > >> >> > +# > > > >> >> > +# 2) When @header is given only, just format the block device > > > >> >> > +# referenced by @header as the LUKS specification. > > > >> >> > +# > > > >> >> > +# 3) When both @file and @header are given, block device > > > >> >> > +# referenced by @file should be trunked to @size, and block > > > >> >> > +# device referenced by @header should be formatted as the > > > >> >> > LUKS > > > >> >> > +# specification. > > > >> >> > # > > > >> >> > # @file: Node to create the image format on, mandatory except > > > >> >> > when > > > >> >> > #'preallocation' is not requested > > > >> >> > > > >> >> Let's see whether I understand. > > > >> >> > > > >> >> blockdev-create with "driver": "luks" can work in three different > > > >> >> ways: > > > >> >> > > > >> >> 1. Create an image with a LUKS header > > > >> >> > > > >> >> 2. Create just a detached LUKS header > > > >> >> > > > >> >> 3. Create an image and a detached LUKS header > > > >> >> > > > >> >> Correct? > > > >> >> > > > >> > > > > >> > Yes > > > >> > > > > >> > > > > >> >> @file and @header are BlockdevRef, which means they refer to > > > >> >> existing > > > >> >> images with arbitrary driver. Could be "file", "qcow2", or > > > >> >> anything. > > > >> >> > > > >> >> Correct? > > > >> >> > > > >> > Yes > > > >> > > > > >> > > > > >> >> > > > >> >> To get 1., specify @file, but not @header. > > > >> >> > > > >> >> To get 2., specify @header, but not @file. > > > >> >> > > > >> >> To get 3., specify both. > > > >> >> > > > >> >> Specifying neither is an error. > > > >> >> > > > >> >> Correct? > > > >> >> > > > >> > > > > >> > Yes > > > >> > > > > >> > > > > >> >> In any case, @size is the logical size of the image (how much data > > > >> >> it > > > >> >> can hold). > > > >> >> > > > >> > > > > >> > Yes > > > >> > > > > >> > > > > >> >> > > > >> >> With 1., the actual image size is a bit larger due to the LUKS > > > >> >> header. > > > >> >> The @file image is resized to that size: if it's shorter, it's > > > >> >> grown, if > > > >> >> it's longer, it's truncated. > > > >> >> > > > >> > > > > >> > Yes > > > >> > > > > >> > > > > >> >> With 2., @size is merely recorded in the detached LUKS header. > > > >> >> > > > >> > > > > >> > In LUKS1 specification, payload data size is not contained in the > > > >> > header, > > > >> > so in this case, @size is not recorded in the detached LUKS header. > > > >> > The creation logic just does the LUKS header formatting only. > > > >> > > > >> Is @size unused then? > > > >> > > > > > > > >
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
On Wed, Feb 28, 2024 at 11:17:37AM +0100, Kevin Wolf wrote:
> Am 28.02.2024 um 07:43 hat Markus Armbruster geschrieben:
> > Yong Huang writes:
> >
> > > On Wed, Feb 21, 2024 at 4:26 PM Markus Armbruster
> > > wrote:
> > >
> > >> Yong Huang writes:
> > >>
> > >> > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster
> > >> wrote:
> > >> >
> > >> >> Hyman Huang writes:
> > >> >>
> > >> >> > Add comment in detail for commit 433957bb7f (qapi:
> > >> >> > Make parameter 'file' optional for
> > >> >> > BlockdevCreateOptionsLUKS).
> > >> >> >
> > >> >> > Signed-off-by: Hyman Huang
> > >> >> > ---
> > >> >> > qapi/block-core.json | 20 +++-
> > >> >> > 1 file changed, 19 insertions(+), 1 deletion(-)
> > >> >> >
> > >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > >> >> > index ab5a93a966..42b0840d43 100644
> > >> >> > --- a/qapi/block-core.json
> > >> >> > +++ b/qapi/block-core.json
> > >> >> > @@ -4973,7 +4973,25 @@
> > >> >> > ##
> > >> >> > # @BlockdevCreateOptionsLUKS:
> > >> >> > #
> > >> >> > -# Driver specific image creation options for LUKS.
> > >> >> > +# Driver specific image creation options for LUKS. Note that
> > >> >> > +# @file is required if @preallocation is specified and equals
> > >> >> > +# PREALLOC_MODE_ON. The following three scenarios determine how
> > >> >> > +# creation logic behaves when @preallocation is either equal to
> > >> >> > +# PREALLOC_MODE_OFF or is not given:
> > >> >> > +#
> > >> >> > +# 1) When @file is given only, format the block device referenced
> > >> >> > +# by @file as the LUKS specification and trunk it to the @size.
> > >> >>
> > >> >> Do you mean "truncate it to @size"?
> > >> >>
> > >> > Yes, :( sorry for the spelling mistake.
> > >>
> > >> Writing good documentation in a second language is *hard*. All we can
> > >> reasonably expect from contributors to try their best. And then we
> > >> improve the text together in review. Just like we do for code :)
> > >>
> > >> >> > +# In this case, the @size should reflect amount of space made
> > >> >> > +# available to the guest, so the trunk size must take account
> > >> >> > +# of that which will be used by the crypto header.
> > >> >> > +#
> > >> >> > +# 2) When @header is given only, just format the block device
> > >> >> > +# referenced by @header as the LUKS specification.
> > >> >> > +#
> > >> >> > +# 3) When both @file and @header are given, block device
> > >> >> > +# referenced by @file should be trunked to @size, and block
> > >> >> > +# device referenced by @header should be formatted as the LUKS
> > >> >> > +# specification.
> > >> >> > #
> > >> >> > # @file: Node to create the image format on, mandatory except when
> > >> >> > #'preallocation' is not requested
> > >> >>
> > >> >> Let's see whether I understand.
> > >> >>
> > >> >> blockdev-create with "driver": "luks" can work in three different
> > >> >> ways:
> > >> >>
> > >> >> 1. Create an image with a LUKS header
> > >> >>
> > >> >> 2. Create just a detached LUKS header
> > >> >>
> > >> >> 3. Create an image and a detached LUKS header
> > >> >>
> > >> >> Correct?
> > >> >>
> > >> >
> > >> > Yes
> > >> >
> > >> >
> > >> >> @file and @header are BlockdevRef, which means they refer to existing
> > >> >> images with arbitrary driver. Could be "file", "qcow2", or anything.
> > >> >>
> > >> >> Correct?
> > >> >>
> > >> > Yes
> > >> >
> > >> >
> > >> >>
> > >> >> To get 1., specify @file, but not @header.
> > >> >>
> > >> >> To get 2., specify @header, but not @file.
> > >> >>
> > >> >> To get 3., specify both.
> > >> >>
> > >> >> Specifying neither is an error.
> > >> >>
> > >> >> Correct?
> > >> >>
> > >> >
> > >> > Yes
> > >> >
> > >> >
> > >> >> In any case, @size is the logical size of the image (how much data it
> > >> >> can hold).
> > >> >>
> > >> >
> > >> > Yes
> > >> >
> > >> >
> > >> >>
> > >> >> With 1., the actual image size is a bit larger due to the LUKS header.
> > >> >> The @file image is resized to that size: if it's shorter, it's grown,
> > >> >> if
> > >> >> it's longer, it's truncated.
> > >> >>
> > >> >
> > >> > Yes
> > >> >
> > >> >
> > >> >> With 2., @size is merely recorded in the detached LUKS header.
> > >> >>
> > >> >
> > >> > In LUKS1 specification, payload data size is not contained in the
> > >> > header,
> > >> > so in this case, @size is not recorded in the detached LUKS header.
> > >> > The creation logic just does the LUKS header formatting only.
> > >>
> > >> Is @size unused then?
> > >>
> > >
> > > IIUC, yes. Creation logic will ignore the @size. See the following code
> > > in function block_crypto_co_create_luks:
> > >
> > > if (luks_opts->header) {
> > > /* LUKS volume with detached header */
> > > hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
> > > if (hdr_bs == NULL) {
> > > return -EIO;
> > > }
> > >
> > > cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
> > >
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Am 28.02.2024 um 07:43 hat Markus Armbruster geschrieben:
> Yong Huang writes:
>
> > On Wed, Feb 21, 2024 at 4:26 PM Markus Armbruster wrote:
> >
> >> Yong Huang writes:
> >>
> >> > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster
> >> wrote:
> >> >
> >> >> Hyman Huang writes:
> >> >>
> >> >> > Add comment in detail for commit 433957bb7f (qapi:
> >> >> > Make parameter 'file' optional for
> >> >> > BlockdevCreateOptionsLUKS).
> >> >> >
> >> >> > Signed-off-by: Hyman Huang
> >> >> > ---
> >> >> > qapi/block-core.json | 20 +++-
> >> >> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> >> > index ab5a93a966..42b0840d43 100644
> >> >> > --- a/qapi/block-core.json
> >> >> > +++ b/qapi/block-core.json
> >> >> > @@ -4973,7 +4973,25 @@
> >> >> > ##
> >> >> > # @BlockdevCreateOptionsLUKS:
> >> >> > #
> >> >> > -# Driver specific image creation options for LUKS.
> >> >> > +# Driver specific image creation options for LUKS. Note that
> >> >> > +# @file is required if @preallocation is specified and equals
> >> >> > +# PREALLOC_MODE_ON. The following three scenarios determine how
> >> >> > +# creation logic behaves when @preallocation is either equal to
> >> >> > +# PREALLOC_MODE_OFF or is not given:
> >> >> > +#
> >> >> > +# 1) When @file is given only, format the block device referenced
> >> >> > +# by @file as the LUKS specification and trunk it to the @size.
> >> >>
> >> >> Do you mean "truncate it to @size"?
> >> >>
> >> > Yes, :( sorry for the spelling mistake.
> >>
> >> Writing good documentation in a second language is *hard*. All we can
> >> reasonably expect from contributors to try their best. And then we
> >> improve the text together in review. Just like we do for code :)
> >>
> >> >> > +# In this case, the @size should reflect amount of space made
> >> >> > +# available to the guest, so the trunk size must take account
> >> >> > +# of that which will be used by the crypto header.
> >> >> > +#
> >> >> > +# 2) When @header is given only, just format the block device
> >> >> > +# referenced by @header as the LUKS specification.
> >> >> > +#
> >> >> > +# 3) When both @file and @header are given, block device
> >> >> > +# referenced by @file should be trunked to @size, and block
> >> >> > +# device referenced by @header should be formatted as the LUKS
> >> >> > +# specification.
> >> >> > #
> >> >> > # @file: Node to create the image format on, mandatory except when
> >> >> > #'preallocation' is not requested
> >> >>
> >> >> Let's see whether I understand.
> >> >>
> >> >> blockdev-create with "driver": "luks" can work in three different ways:
> >> >>
> >> >> 1. Create an image with a LUKS header
> >> >>
> >> >> 2. Create just a detached LUKS header
> >> >>
> >> >> 3. Create an image and a detached LUKS header
> >> >>
> >> >> Correct?
> >> >>
> >> >
> >> > Yes
> >> >
> >> >
> >> >> @file and @header are BlockdevRef, which means they refer to existing
> >> >> images with arbitrary driver. Could be "file", "qcow2", or anything.
> >> >>
> >> >> Correct?
> >> >>
> >> > Yes
> >> >
> >> >
> >> >>
> >> >> To get 1., specify @file, but not @header.
> >> >>
> >> >> To get 2., specify @header, but not @file.
> >> >>
> >> >> To get 3., specify both.
> >> >>
> >> >> Specifying neither is an error.
> >> >>
> >> >> Correct?
> >> >>
> >> >
> >> > Yes
> >> >
> >> >
> >> >> In any case, @size is the logical size of the image (how much data it
> >> >> can hold).
> >> >>
> >> >
> >> > Yes
> >> >
> >> >
> >> >>
> >> >> With 1., the actual image size is a bit larger due to the LUKS header.
> >> >> The @file image is resized to that size: if it's shorter, it's grown, if
> >> >> it's longer, it's truncated.
> >> >>
> >> >
> >> > Yes
> >> >
> >> >
> >> >> With 2., @size is merely recorded in the detached LUKS header.
> >> >>
> >> >
> >> > In LUKS1 specification, payload data size is not contained in the header,
> >> > so in this case, @size is not recorded in the detached LUKS header.
> >> > The creation logic just does the LUKS header formatting only.
> >>
> >> Is @size unused then?
> >>
> >
> > IIUC, yes. Creation logic will ignore the @size. See the following code
> > in function block_crypto_co_create_luks:
> >
> > if (luks_opts->header) {
> > /* LUKS volume with detached header */
> > hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
> > if (hdr_bs == NULL) {
> > return -EIO;
> > }
> >
> > cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
> >
> > /* Format the LUKS header node, here just ignore the size
> > * and passed zero to block_crypto_co_create_generic */
> > ret = block_crypto_co_create_generic(hdr_bs, 0, _opts,
> > PREALLOC_MODE_OFF, cflags,
> > errp);
> > if (ret < 0) {
> > goto fail;
> > }
> >
> >
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Yong Huang writes:
> On Wed, Feb 21, 2024 at 4:26 PM Markus Armbruster wrote:
>
>> Yong Huang writes:
>>
>> > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster
>> wrote:
>> >
>> >> Hyman Huang writes:
>> >>
>> >> > Add comment in detail for commit 433957bb7f (qapi:
>> >> > Make parameter 'file' optional for
>> >> > BlockdevCreateOptionsLUKS).
>> >> >
>> >> > Signed-off-by: Hyman Huang
>> >> > ---
>> >> > qapi/block-core.json | 20 +++-
>> >> > 1 file changed, 19 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> >> > index ab5a93a966..42b0840d43 100644
>> >> > --- a/qapi/block-core.json
>> >> > +++ b/qapi/block-core.json
>> >> > @@ -4973,7 +4973,25 @@
>> >> > ##
>> >> > # @BlockdevCreateOptionsLUKS:
>> >> > #
>> >> > -# Driver specific image creation options for LUKS.
>> >> > +# Driver specific image creation options for LUKS. Note that
>> >> > +# @file is required if @preallocation is specified and equals
>> >> > +# PREALLOC_MODE_ON. The following three scenarios determine how
>> >> > +# creation logic behaves when @preallocation is either equal to
>> >> > +# PREALLOC_MODE_OFF or is not given:
>> >> > +#
>> >> > +# 1) When @file is given only, format the block device referenced
>> >> > +# by @file as the LUKS specification and trunk it to the @size.
>> >>
>> >> Do you mean "truncate it to @size"?
>> >>
>> > Yes, :( sorry for the spelling mistake.
>>
>> Writing good documentation in a second language is *hard*. All we can
>> reasonably expect from contributors to try their best. And then we
>> improve the text together in review. Just like we do for code :)
>>
>> >> > +# In this case, the @size should reflect amount of space made
>> >> > +# available to the guest, so the trunk size must take account
>> >> > +# of that which will be used by the crypto header.
>> >> > +#
>> >> > +# 2) When @header is given only, just format the block device
>> >> > +# referenced by @header as the LUKS specification.
>> >> > +#
>> >> > +# 3) When both @file and @header are given, block device
>> >> > +# referenced by @file should be trunked to @size, and block
>> >> > +# device referenced by @header should be formatted as the LUKS
>> >> > +# specification.
>> >> > #
>> >> > # @file: Node to create the image format on, mandatory except when
>> >> > #'preallocation' is not requested
>> >>
>> >> Let's see whether I understand.
>> >>
>> >> blockdev-create with "driver": "luks" can work in three different ways:
>> >>
>> >> 1. Create an image with a LUKS header
>> >>
>> >> 2. Create just a detached LUKS header
>> >>
>> >> 3. Create an image and a detached LUKS header
>> >>
>> >> Correct?
>> >>
>> >
>> > Yes
>> >
>> >
>> >> @file and @header are BlockdevRef, which means they refer to existing
>> >> images with arbitrary driver. Could be "file", "qcow2", or anything.
>> >>
>> >> Correct?
>> >>
>> > Yes
>> >
>> >
>> >>
>> >> To get 1., specify @file, but not @header.
>> >>
>> >> To get 2., specify @header, but not @file.
>> >>
>> >> To get 3., specify both.
>> >>
>> >> Specifying neither is an error.
>> >>
>> >> Correct?
>> >>
>> >
>> > Yes
>> >
>> >
>> >> In any case, @size is the logical size of the image (how much data it
>> >> can hold).
>> >>
>> >
>> > Yes
>> >
>> >
>> >>
>> >> With 1., the actual image size is a bit larger due to the LUKS header.
>> >> The @file image is resized to that size: if it's shorter, it's grown, if
>> >> it's longer, it's truncated.
>> >>
>> >
>> > Yes
>> >
>> >
>> >> With 2., @size is merely recorded in the detached LUKS header.
>> >>
>> >
>> > In LUKS1 specification, payload data size is not contained in the header,
>> > so in this case, @size is not recorded in the detached LUKS header.
>> > The creation logic just does the LUKS header formatting only.
>>
>> Is @size unused then?
>>
>
> IIUC, yes. Creation logic will ignore the @size. See the following code
> in function block_crypto_co_create_luks:
>
> if (luks_opts->header) {
> /* LUKS volume with detached header */
> hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
> if (hdr_bs == NULL) {
> return -EIO;
> }
>
> cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
>
> /* Format the LUKS header node, here just ignore the size
> * and passed zero to block_crypto_co_create_generic */
> ret = block_crypto_co_create_generic(hdr_bs, 0, _opts,
> PREALLOC_MODE_OFF, cflags, errp);
> if (ret < 0) {
> goto fail;
> }
>
> /* Format the LUKS payload node */
> if (luks_opts->file) {
> ret = block_crypto_co_format_luks_payload(luks_opts, errp);
> if (ret < 0) {
> goto fail;
> }
> }
@size is a required argument, but silently ignored when @header is
present and @file is absent (2. Create just a detached LUKS
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
On Wed, Feb 21, 2024 at 4:26 PM Markus Armbruster wrote:
> Yong Huang writes:
>
> > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster
> wrote:
> >
> >> Hyman Huang writes:
> >>
> >> > Add comment in detail for commit 433957bb7f (qapi:
> >> > Make parameter 'file' optional for
> >> > BlockdevCreateOptionsLUKS).
> >> >
> >> > Signed-off-by: Hyman Huang
> >> > ---
> >> > qapi/block-core.json | 20 +++-
> >> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> > index ab5a93a966..42b0840d43 100644
> >> > --- a/qapi/block-core.json
> >> > +++ b/qapi/block-core.json
> >> > @@ -4973,7 +4973,25 @@
> >> > ##
> >> > # @BlockdevCreateOptionsLUKS:
> >> > #
> >> > -# Driver specific image creation options for LUKS.
> >> > +# Driver specific image creation options for LUKS. Note that
> >> > +# @file is required if @preallocation is specified and equals
> >> > +# PREALLOC_MODE_ON. The following three scenarios determine how
> >> > +# creation logic behaves when @preallocation is either equal to
> >> > +# PREALLOC_MODE_OFF or is not given:
> >> > +#
> >> > +# 1) When @file is given only, format the block device referenced
> >> > +# by @file as the LUKS specification and trunk it to the @size.
> >>
> >> Do you mean "truncate it to @size"?
> >>
> > Yes, :( sorry for the spelling mistake.
>
> Writing good documentation in a second language is *hard*. All we can
> reasonably expect from contributors to try their best. And then we
> improve the text together in review. Just like we do for code :)
>
> >> > +# In this case, the @size should reflect amount of space made
> >> > +# available to the guest, so the trunk size must take account
> >> > +# of that which will be used by the crypto header.
> >> > +#
> >> > +# 2) When @header is given only, just format the block device
> >> > +# referenced by @header as the LUKS specification.
> >> > +#
> >> > +# 3) When both @file and @header are given, block device
> >> > +# referenced by @file should be trunked to @size, and block
> >> > +# device referenced by @header should be formatted as the LUKS
> >> > +# specification.
> >> > #
> >> > # @file: Node to create the image format on, mandatory except when
> >> > #'preallocation' is not requested
> >>
> >> Let's see whether I understand.
> >>
> >> blockdev-create with "driver": "luks" can work in three different ways:
> >>
> >> 1. Create an image with a LUKS header
> >>
> >> 2. Create just a detached LUKS header
> >>
> >> 3. Create an image and a detached LUKS header
> >>
> >> Correct?
> >>
> >
> > Yes
> >
> >
> >> @file and @header are BlockdevRef, which means they refer to existing
> >> images with arbitrary driver. Could be "file", "qcow2", or anything.
> >>
> >> Correct?
> >>
> > Yes
> >
> >
> >>
> >> To get 1., specify @file, but not @header.
> >>
> >> To get 2., specify @header, but not @file.
> >>
> >> To get 3., specify both.
> >>
> >> Specifying neither is an error.
> >>
> >> Correct?
> >>
> >
> > Yes
> >
> >
> >> In any case, @size is the logical size of the image (how much data it
> >> can hold).
> >>
> >
> > Yes
> >
> >
> >>
> >> With 1., the actual image size is a bit larger due to the LUKS header.
> >> The @file image is resized to that size: if it's shorter, it's grown, if
> >> it's longer, it's truncated.
> >>
> >
> > Yes
> >
> >
> >> With 2., @size is merely recorded in the detached LUKS header.
> >>
> >
> > In LUKS1 specification, payload data size is not contained in the header,
> > so in this case, @size is not recorded in the detached LUKS header.
> > The creation logic just does the LUKS header formatting only.
>
> Is @size unused then?
>
IIUC, yes. Creation logic will ignore the @size. See the following code
in function block_crypto_co_create_luks:
if (luks_opts->header) {
/* LUKS volume with detached header */
hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
if (hdr_bs == NULL) {
return -EIO;
}
cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
/* Format the LUKS header node, here just ignore the size
* and passed zero to block_crypto_co_create_generic */
ret = block_crypto_co_create_generic(hdr_bs, 0, _opts,
PREALLOC_MODE_OFF, cflags,
errp);
if (ret < 0) {
goto fail;
}
/* Format the LUKS payload node */
if (luks_opts->file) {
ret = block_crypto_co_format_luks_payload(luks_opts, errp);
if (ret < 0) {
goto fail;
}
}
>
> >> With 3., @size is recorded in the detached LUKS header, and the @file
> >> image is resized as with 1.
> >>
> >
> > Same reason as above, @size is not recorded and @file image is
> > resized to @size exactly.
> >
> >
> >>
> >> Correct?
> >>
> >>
> > Thanks for the comment,
> >
> > Yong
>
>
--
Best regards
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Yong Huang writes: > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster wrote: > >> Hyman Huang writes: >> >> > Add comment in detail for commit 433957bb7f (qapi: >> > Make parameter 'file' optional for >> > BlockdevCreateOptionsLUKS). >> > >> > Signed-off-by: Hyman Huang >> > --- >> > qapi/block-core.json | 20 +++- >> > 1 file changed, 19 insertions(+), 1 deletion(-) >> > >> > diff --git a/qapi/block-core.json b/qapi/block-core.json >> > index ab5a93a966..42b0840d43 100644 >> > --- a/qapi/block-core.json >> > +++ b/qapi/block-core.json >> > @@ -4973,7 +4973,25 @@ >> > ## >> > # @BlockdevCreateOptionsLUKS: >> > # >> > -# Driver specific image creation options for LUKS. >> > +# Driver specific image creation options for LUKS. Note that >> > +# @file is required if @preallocation is specified and equals >> > +# PREALLOC_MODE_ON. The following three scenarios determine how >> > +# creation logic behaves when @preallocation is either equal to >> > +# PREALLOC_MODE_OFF or is not given: >> > +# >> > +# 1) When @file is given only, format the block device referenced >> > +# by @file as the LUKS specification and trunk it to the @size. >> >> Do you mean "truncate it to @size"? >> > Yes, :( sorry for the spelling mistake. Writing good documentation in a second language is *hard*. All we can reasonably expect from contributors to try their best. And then we improve the text together in review. Just like we do for code :) >> > +# In this case, the @size should reflect amount of space made >> > +# available to the guest, so the trunk size must take account >> > +# of that which will be used by the crypto header. >> > +# >> > +# 2) When @header is given only, just format the block device >> > +# referenced by @header as the LUKS specification. >> > +# >> > +# 3) When both @file and @header are given, block device >> > +# referenced by @file should be trunked to @size, and block >> > +# device referenced by @header should be formatted as the LUKS >> > +# specification. >> > # >> > # @file: Node to create the image format on, mandatory except when >> > #'preallocation' is not requested >> >> Let's see whether I understand. >> >> blockdev-create with "driver": "luks" can work in three different ways: >> >> 1. Create an image with a LUKS header >> >> 2. Create just a detached LUKS header >> >> 3. Create an image and a detached LUKS header >> >> Correct? >> > > Yes > > >> @file and @header are BlockdevRef, which means they refer to existing >> images with arbitrary driver. Could be "file", "qcow2", or anything. >> >> Correct? >> > Yes > > >> >> To get 1., specify @file, but not @header. >> >> To get 2., specify @header, but not @file. >> >> To get 3., specify both. >> >> Specifying neither is an error. >> >> Correct? >> > > Yes > > >> In any case, @size is the logical size of the image (how much data it >> can hold). >> > > Yes > > >> >> With 1., the actual image size is a bit larger due to the LUKS header. >> The @file image is resized to that size: if it's shorter, it's grown, if >> it's longer, it's truncated. >> > > Yes > > >> With 2., @size is merely recorded in the detached LUKS header. >> > > In LUKS1 specification, payload data size is not contained in the header, > so in this case, @size is not recorded in the detached LUKS header. > The creation logic just does the LUKS header formatting only. Is @size unused then? >> With 3., @size is recorded in the detached LUKS header, and the @file >> image is resized as with 1. >> > > Same reason as above, @size is not recorded and @file image is > resized to @size exactly. > > >> >> Correct? >> >> > Thanks for the comment, > > Yong
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster wrote: > Hyman Huang writes: > > > Add comment in detail for commit 433957bb7f (qapi: > > Make parameter 'file' optional for > > BlockdevCreateOptionsLUKS). > > > > Signed-off-by: Hyman Huang > > --- > > qapi/block-core.json | 20 +++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index ab5a93a966..42b0840d43 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -4973,7 +4973,25 @@ > > ## > > # @BlockdevCreateOptionsLUKS: > > # > > -# Driver specific image creation options for LUKS. > > +# Driver specific image creation options for LUKS. Note that > > +# @file is required if @preallocation is specified and equals > > +# PREALLOC_MODE_ON. The following three scenarios determine how > > +# creation logic behaves when @preallocation is either equal to > > +# PREALLOC_MODE_OFF or is not given: > > +# > > +# 1) When @file is given only, format the block device referenced > > +# by @file as the LUKS specification and trunk it to the @size. > > Do you mean "truncate it to @size"? > Yes, :( sorry for the spelling mistake. > > > +# In this case, the @size should reflect amount of space made > > +# available to the guest, so the trunk size must take account > > +# of that which will be used by the crypto header. > > +# > > +# 2) When @header is given only, just format the block device > > +# referenced by @header as the LUKS specification. > > +# > > +# 3) When both @file and @header are given, block device > > +# referenced by @file should be trunked to @size, and block > > +# device referenced by @header should be formatted as the LUKS > > +# specification. > > # > > # @file: Node to create the image format on, mandatory except when > > #'preallocation' is not requested > > Let's see whether I understand. > > blockdev-create with "driver": "luks" can work in three different ways: > > 1. Create an image with a LUKS header > > 2. Create just a detached LUKS header > > 3. Create an image and a detached LUKS header > > Correct? > Yes > @file and @header are BlockdevRef, which means they refer to existing > images with arbitrary driver. Could be "file", "qcow2", or anything. > > Correct? > Yes > > To get 1., specify @file, but not @header. > > To get 2., specify @header, but not @file. > > To get 3., specify both. > > Specifying neither is an error. > > Correct? > Yes > In any case, @size is the logical size of the image (how much data it > can hold). > Yes > > With 1., the actual image size is a bit larger due to the LUKS header. > The @file image is resized to that size: if it's shorter, it's grown, if > it's longer, it's truncated. > Yes > With 2., @size is merely recorded in the detached LUKS header. > In LUKS1 specification, payload data size is not contained in the header, so in this case, @size is not recorded in the detached LUKS header. The creation logic just does the LUKS header formatting only. > > With 3., @size is recorded in the detached LUKS header, and the @file > image is resized as with 1. > Same reason as above, @size is not recorded and @file image is resized to @size exactly. > > Correct? > > Thanks for the comment, Yong -- Best regards
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Hyman Huang writes: > Add comment in detail for commit 433957bb7f (qapi: > Make parameter 'file' optional for > BlockdevCreateOptionsLUKS). > > Signed-off-by: Hyman Huang > --- > qapi/block-core.json | 20 +++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index ab5a93a966..42b0840d43 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -4973,7 +4973,25 @@ > ## > # @BlockdevCreateOptionsLUKS: > # > -# Driver specific image creation options for LUKS. > +# Driver specific image creation options for LUKS. Note that > +# @file is required if @preallocation is specified and equals > +# PREALLOC_MODE_ON. The following three scenarios determine how > +# creation logic behaves when @preallocation is either equal to > +# PREALLOC_MODE_OFF or is not given: > +# > +# 1) When @file is given only, format the block device referenced > +# by @file as the LUKS specification and trunk it to the @size. Do you mean "truncate it to @size"? > +# In this case, the @size should reflect amount of space made > +# available to the guest, so the trunk size must take account > +# of that which will be used by the crypto header. > +# > +# 2) When @header is given only, just format the block device > +# referenced by @header as the LUKS specification. > +# > +# 3) When both @file and @header are given, block device > +# referenced by @file should be trunked to @size, and block > +# device referenced by @header should be formatted as the LUKS > +# specification. > # > # @file: Node to create the image format on, mandatory except when > #'preallocation' is not requested Let's see whether I understand. blockdev-create with "driver": "luks" can work in three different ways: 1. Create an image with a LUKS header 2. Create just a detached LUKS header 3. Create an image and a detached LUKS header Correct? @file and @header are BlockdevRef, which means they refer to existing images with arbitrary driver. Could be "file", "qcow2", or anything. Correct? To get 1., specify @file, but not @header. To get 2., specify @header, but not @file. To get 3., specify both. Specifying neither is an error. Correct? In any case, @size is the logical size of the image (how much data it can hold). With 1., the actual image size is a bit larger due to the LUKS header. The @file image is resized to that size: if it's shorter, it's grown, if it's longer, it's truncated. With 2., @size is merely recorded in the detached LUKS header. With 3., @size is recorded in the detached LUKS header, and the @file image is resized as with 1. Correct?
[PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Add comment in detail for commit 433957bb7f (qapi: Make parameter 'file' optional for BlockdevCreateOptionsLUKS). Signed-off-by: Hyman Huang --- qapi/block-core.json | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index ab5a93a966..42b0840d43 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -4973,7 +4973,25 @@ ## # @BlockdevCreateOptionsLUKS: # -# Driver specific image creation options for LUKS. +# Driver specific image creation options for LUKS. Note that +# @file is required if @preallocation is specified and equals +# PREALLOC_MODE_ON. The following three scenarios determine how +# creation logic behaves when @preallocation is either equal to +# PREALLOC_MODE_OFF or is not given: +# +# 1) When @file is given only, format the block device referenced +# by @file as the LUKS specification and trunk it to the @size. +# In this case, the @size should reflect amount of space made +# available to the guest, so the trunk size must take account +# of that which will be used by the crypto header. +# +# 2) When @header is given only, just format the block device +# referenced by @header as the LUKS specification. +# +# 3) When both @file and @header are given, block device +# referenced by @file should be trunked to @size, and block +# device referenced by @header should be formatted as the LUKS +# specification. # # @file: Node to create the image format on, mandatory except when #'preallocation' is not requested -- 2.39.3
