| Message ID | af7253711254128efbc37b25fb5c47b851367ce7.1706586786.git.yong.huang@smartx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support generic Luks encryption | expand |
yong.huang@smartx.com writes: > From: Hyman Huang <yong.huang@smartx.com> > > To support detached LUKS header creation, make the existing 'file' > field in BlockdevCreateOptionsLUKS optional. > > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> [...] > diff --git a/qapi/block-core.json b/qapi/block-core.json > index ae604c6019..69a88d613d 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -4957,7 +4957,8 @@ > # > # Driver specific image creation options for LUKS. > # > -# @file: Node to create the image format on > +# @file: Node to create the image format on, mandatory except when > +# 'preallocation' is not requested You mean when @preallocation is "off"? Cases: 1. @file is mandatory 2. @file is optional and present 3. @file is optional and absent Ignorant question: behavior in each case? > # > # @size: Size of the virtual disk in bytes > # > @@ -4968,7 +4969,7 @@ > ## > { 'struct': 'BlockdevCreateOptionsLUKS', > 'base': 'QCryptoBlockCreateOptionsLUKS', > - 'data': { 'file': 'BlockdevRef', > + 'data': { '*file': 'BlockdevRef', > 'size': 'size', > '*preallocation': 'PreallocMode' } }
On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com> wrote: > yong.huang@smartx.com writes: > > > From: Hyman Huang <yong.huang@smartx.com> > > > > To support detached LUKS header creation, make the existing 'file' > > field in BlockdevCreateOptionsLUKS optional. > > > > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > [...] > > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index ae604c6019..69a88d613d 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -4957,7 +4957,8 @@ > > # > > # Driver specific image creation options for LUKS. > > # > > -# @file: Node to create the image format on > > +# @file: Node to create the image format on, mandatory except when > > +# 'preallocation' is not requested > > You mean when @preallocation is "off"? > > Cases: > > 1. @file is mandatory > When @preallocation is specified to PREALLOC_MODE_ON, file is mandatory because preallocation aims to act on payload data that @file holds. > 2. @file is optional and present > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, @file if optional. If @file present，there are two cases: 1. @header is absent, the creation process degenerate to the origin action. 2. @header is present, the creation process would trunk the payload data image that @file holds and do the LUKS formatting on the image that @header refers; > > 3. @file is optional and absent > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, @file if optional. If @file is absent, do the LUKS formatting only. Note that Either the parameter 'header' or 'file' must be specified. Here's my interpretation; do let me know if any of the points are off or need to be refactored. > > Ignorant question: behavior in each case? > > > # > > # @size: Size of the virtual disk in bytes > > # > > @@ -4968,7 +4969,7 @@ > > ## > > { 'struct': 'BlockdevCreateOptionsLUKS', > > 'base': 'QCryptoBlockCreateOptionsLUKS', > > - 'data': { 'file': 'BlockdevRef', > > + 'data': { '*file': 'BlockdevRef', > > 'size': 'size', > > '*preallocation': 'PreallocMode' } } > > Thanks, Yong
Yong Huang <yong.huang@smartx.com> writes: > On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com> wrote: > >> yong.huang@smartx.com writes: >> >> > From: Hyman Huang <yong.huang@smartx.com> >> > >> > To support detached LUKS header creation, make the existing 'file' >> > field in BlockdevCreateOptionsLUKS optional. >> > >> > Signed-off-by: Hyman Huang <yong.huang@smartx.com> >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >> >> [...] >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json >> > index ae604c6019..69a88d613d 100644 >> > --- a/qapi/block-core.json >> > +++ b/qapi/block-core.json >> > @@ -4957,7 +4957,8 @@ >> > # >> > # Driver specific image creation options for LUKS. >> > # >> > -# @file: Node to create the image format on >> > +# @file: Node to create the image format on, mandatory except when >> > +# 'preallocation' is not requested >> >> You mean when @preallocation is "off"? >> >> Cases: >> >> 1. @file is mandatory >> > > When @preallocation is specified to PREALLOC_MODE_ON, file > is mandatory because preallocation aims to act on payload data that > @file holds. > > >> 2. @file is optional and present >> > > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, > @file if optional. > If @file present，there are two cases: > 1. @header is absent, the creation process degenerate to the origin action. > 2. @header is present, the creation process would trunk the payload data > image that @file holds and do the LUKS formatting on the image that > @header refers; > > >> >> 3. @file is optional and absent >> > > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, > @file if optional. > If @file is absent, do the LUKS formatting only. > Note that Either the parameter 'header' or 'file' must be specified. > > Here's my interpretation; do let me know if any of the points are off or > need to be refactored. > > >> >> Ignorant question: behavior in each case? Thanks! Would it make sense to work the above into the documentation? [...]
On Tue, Feb 20, 2024 at 4:56 PM Markus Armbruster <armbru@redhat.com> wrote: > Yong Huang <yong.huang@smartx.com> writes: > > > On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com> > wrote: > > > >> yong.huang@smartx.com writes: > >> > >> > From: Hyman Huang <yong.huang@smartx.com> > >> > > >> > To support detached LUKS header creation, make the existing 'file' > >> > field in BlockdevCreateOptionsLUKS optional. > >> > > >> > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > >> > >> [...] > >> > >> > diff --git a/qapi/block-core.json b/qapi/block-core.json > >> > index ae604c6019..69a88d613d 100644 > >> > --- a/qapi/block-core.json > >> > +++ b/qapi/block-core.json > >> > @@ -4957,7 +4957,8 @@ > >> > # > >> > # Driver specific image creation options for LUKS. > >> > # > >> > -# @file: Node to create the image format on > >> > +# @file: Node to create the image format on, mandatory except when > >> > +# 'preallocation' is not requested > >> > >> You mean when @preallocation is "off"? > >> > >> Cases: > >> > >> 1. @file is mandatory > >> > > > > When @preallocation is specified to PREALLOC_MODE_ON, file > > is mandatory because preallocation aims to act on payload data that > > @file holds. > > > > > >> 2. @file is optional and present > >> > > > > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, > > @file if optional. > > If @file present，there are two cases: > > 1. @header is absent, the creation process degenerate to the origin > action. > > 2. @header is present, the creation process would trunk the payload data > > image that @file holds and do the LUKS formatting on the image that > > @header refers; > > > > > >> > >> 3. @file is optional and absent > >> > > > > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, > > @file if optional. > > If @file is absent, do the LUKS formatting only. > > Note that Either the parameter 'header' or 'file' must be specified. > > > > Here's my interpretation; do let me know if any of the points are off or > > need to be refactored. > > > > > >> > >> Ignorant question: behavior in each case? > > Thanks! Would it make sense to work the above into the documentation? > You mean adding the above interpretation to the following patch? https://patchew.org/QEMU/c2049499aa05758b4cf18dcec942694ed454a980.1708358310.git.yong.huang@smartx.com/ > [...] > > Yong
Yong Huang <yong.huang@smartx.com> writes: > On Tue, Feb 20, 2024 at 4:56 PM Markus Armbruster <armbru@redhat.com> wrote: > >> Yong Huang <yong.huang@smartx.com> writes: >> >> > On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com> >> wrote: >> > >> >> yong.huang@smartx.com writes: >> >> >> >> > From: Hyman Huang <yong.huang@smartx.com> >> >> > >> >> > To support detached LUKS header creation, make the existing 'file' >> >> > field in BlockdevCreateOptionsLUKS optional. >> >> > >> >> > Signed-off-by: Hyman Huang <yong.huang@smartx.com> >> >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >> >> >> >> [...] >> >> >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json >> >> > index ae604c6019..69a88d613d 100644 >> >> > --- a/qapi/block-core.json >> >> > +++ b/qapi/block-core.json >> >> > @@ -4957,7 +4957,8 @@ >> >> > # >> >> > # Driver specific image creation options for LUKS. >> >> > # >> >> > -# @file: Node to create the image format on >> >> > +# @file: Node to create the image format on, mandatory except when >> >> > +# 'preallocation' is not requested >> >> >> >> You mean when @preallocation is "off"? >> >> >> >> Cases: >> >> >> >> 1. @file is mandatory >> >> >> > >> > When @preallocation is specified to PREALLOC_MODE_ON, file >> > is mandatory because preallocation aims to act on payload data that >> > @file holds. >> > >> > >> >> 2. @file is optional and present >> >> >> > >> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, >> > @file if optional. >> > If @file present，there are two cases: >> > 1. @header is absent, the creation process degenerate to the origin action. >> > 2. @header is present, the creation process would trunk the payload data >> > image that @file holds and do the LUKS formatting on the image that >> > @header refers; >> > >> > >> >> >> >> 3. @file is optional and absent >> >> >> > >> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, >> > @file if optional. >> > If @file is absent, do the LUKS formatting only. >> > Note that Either the parameter 'header' or 'file' must be specified. >> > >> > Here's my interpretation; do let me know if any of the points are off or >> > need to be refactored. >> > >> > >> >> >> >> Ignorant question: behavior in each case? >> >> Thanks! Would it make sense to work the above into the documentation? >> > > You mean adding the above interpretation to the following patch? > > https://patchew.org/QEMU/c2049499aa05758b4cf18dcec942694ed454a980.1708358310.git.yong.huang@smartx.com/ To the doc comments. The doc comments are the source code for the "QEMU QMP Reference Manual". That manual should fully explain what the QMP commands do. Information on how to best use the commands, or an introduction to concepts behind the commands can also be useful, but is often a bad fit for a *reference* manual. We can put it elsewhere then. Makes sense?
On Tue, Feb 20, 2024 at 5:47 PM Markus Armbruster <armbru@redhat.com> wrote: > Yong Huang <yong.huang@smartx.com> writes: > > > On Tue, Feb 20, 2024 at 4:56 PM Markus Armbruster <armbru@redhat.com> > wrote: > > > >> Yong Huang <yong.huang@smartx.com> writes: > >> > >> > On Tue, Feb 20, 2024 at 2:31 PM Markus Armbruster <armbru@redhat.com> > >> wrote: > >> > > >> >> yong.huang@smartx.com writes: > >> >> > >> >> > From: Hyman Huang <yong.huang@smartx.com> > >> >> > > >> >> > To support detached LUKS header creation, make the existing 'file' > >> >> > field in BlockdevCreateOptionsLUKS optional. > >> >> > > >> >> > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > >> >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > >> >> > >> >> [...] > >> >> > >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json > >> >> > index ae604c6019..69a88d613d 100644 > >> >> > --- a/qapi/block-core.json > >> >> > +++ b/qapi/block-core.json > >> >> > @@ -4957,7 +4957,8 @@ > >> >> > # > >> >> > # Driver specific image creation options for LUKS. > >> >> > # > >> >> > -# @file: Node to create the image format on > >> >> > +# @file: Node to create the image format on, mandatory except when > >> >> > +# 'preallocation' is not requested > >> >> > >> >> You mean when @preallocation is "off"? > >> >> > >> >> Cases: > >> >> > >> >> 1. @file is mandatory > >> >> > >> > > >> > When @preallocation is specified to PREALLOC_MODE_ON, file > >> > is mandatory because preallocation aims to act on payload data that > >> > @file holds. > >> > > >> > > >> >> 2. @file is optional and present > >> >> > >> > > >> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, > >> > @file if optional. > >> > If @file present，there are two cases: > >> > 1. @header is absent, the creation process degenerate to the origin > action. > >> > 2. @header is present, the creation process would trunk the payload > data > >> > image that @file holds and do the LUKS formatting on the image that > >> > @header refers; > >> > > >> > > >> >> > >> >> 3. @file is optional and absent > >> >> > >> > > >> > When @preallocation is not specified or equals to PREALLOC_MODE_OFF, > >> > @file if optional. > >> > If @file is absent, do the LUKS formatting only. > >> > Note that Either the parameter 'header' or 'file' must be specified. > >> > > >> > Here's my interpretation; do let me know if any of the points are off > or > >> > need to be refactored. > >> > > >> > > >> >> > >> >> Ignorant question: behavior in each case? > >> > >> Thanks! Would it make sense to work the above into the documentation? > >> > > > > You mean adding the above interpretation to the following patch? > > > > > https://patchew.org/QEMU/c2049499aa05758b4cf18dcec942694ed454a980.1708358310.git.yong.huang@smartx.com/ > > To the doc comments. > > The doc comments are the source code for the "QEMU QMP Reference > Manual". That manual should fully explain what the QMP commands do. > Information on how to best use the commands, or an introduction to > concepts behind the commands can also be useful, but is often a bad fit > for a *reference* manual. We can put it elsewhere then. > > Makes sense? > > Of course yes, it can be somewhat complex to use the LUKS volume with a detachable header, but users may find some relief from the interpretation. I'll try it.
diff --git a/block/crypto.c b/block/crypto.c index 68656158e9..e87dc84111 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -663,9 +663,9 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp) assert(create_options->driver == BLOCKDEV_DRIVER_LUKS); luks_opts = &create_options->u.luks; - bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp); - if (bs == NULL) { - return -EIO; + if (luks_opts->file == NULL) { + error_setg(errp, "Formatting LUKS disk requires parameter 'file'"); + return -EINVAL; } create_opts = (QCryptoBlockCreateOptions) { @@ -677,10 +677,17 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp) preallocation = luks_opts->preallocation; } - ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts, - preallocation, errp); - if (ret < 0) { - goto fail; + if (luks_opts->file) { + bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp); + if (bs == NULL) { + return -EIO; + } + + ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts, + preallocation, errp); + if (ret < 0) { + goto fail; + } } ret = 0; diff --git a/qapi/block-core.json b/qapi/block-core.json index ae604c6019..69a88d613d 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -4957,7 +4957,8 @@ # # Driver specific image creation options for LUKS. # -# @file: Node to create the image format on +# @file: Node to create the image format on, mandatory except when +# 'preallocation' is not requested # # @size: Size of the virtual disk in bytes # @@ -4968,7 +4969,7 @@ ## { 'struct': 'BlockdevCreateOptionsLUKS', 'base': 'QCryptoBlockCreateOptionsLUKS', - 'data': { 'file': 'BlockdevRef', + 'data': { '*file': 'BlockdevRef', 'size': 'size', '*preallocation': 'PreallocMode' } }