| Message ID | 20190219125044.5416-2-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: improve error handling when luks creation fails | expand |
On 2/19/19 6:50 AM, Daniel P. Berrangé wrote: > If the qcow2 image does not have any encryption method specified in its > header, the user should not be providing any encryption options when > opening it. We already detect this if the user had set "encrypt.format" > but this field is optional so must consider any "encrypt.*" option to be > an error. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > block/qcow2.c | 6 ++++++ > 1 file changed, 6 insertions(+) > Reviewed-by: Eric Blake <eblake@redhat.com>
On 19.02.19 13:50, Daniel P. Berrangé wrote: > If the qcow2 image does not have any encryption method specified in its > header, the user should not be providing any encryption options when > opening it. We already detect this if the user had set "encrypt.format" > but this field is optional so must consider any "encrypt.*" option to be > an error. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > block/qcow2.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 65a54c9ac6..ecc577175f 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1045,6 +1045,12 @@ static int qcow2_update_options_prepare(BlockDriverState *bs, > ret = -EINVAL; > goto fail; > } > + if (encryptopts && qdict_size(encryptopts)) { > + error_setg(errp, "No encryption in image header, but encryption " > + "options provided"); > + ret = -EINVAL; > + goto fail; > + } Doesn't this make the block right before this one a bit superfluous? Max > break; > > case QCOW_CRYPT_AES: >
On Fri, Feb 22, 2019 at 08:17:40PM +0100, Max Reitz wrote: > On 19.02.19 13:50, Daniel P. Berrangé wrote: > > If the qcow2 image does not have any encryption method specified in its > > header, the user should not be providing any encryption options when > > opening it. We already detect this if the user had set "encrypt.format" > > but this field is optional so must consider any "encrypt.*" option to be > > an error. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > block/qcow2.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index 65a54c9ac6..ecc577175f 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > @@ -1045,6 +1045,12 @@ static int qcow2_update_options_prepare(BlockDriverState *bs, > > ret = -EINVAL; > > goto fail; > > } > > + if (encryptopts && qdict_size(encryptopts)) { > > + error_setg(errp, "No encryption in image header, but encryption " > > + "options provided"); > > + ret = -EINVAL; > > + goto fail; > > + } > > Doesn't this make the block right before this one a bit superfluous? Yes, in the sense that we'll still get an error if we removed the prior block. The prior block has a more useful error message which will help diagnosis though, so I thought it worth keeping both. Regards, Daniel
diff --git a/block/qcow2.c b/block/qcow2.c index 65a54c9ac6..ecc577175f 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1045,6 +1045,12 @@ static int qcow2_update_options_prepare(BlockDriverState *bs, ret = -EINVAL; goto fail; } + if (encryptopts && qdict_size(encryptopts)) { + error_setg(errp, "No encryption in image header, but encryption " + "options provided"); + ret = -EINVAL; + goto fail; + } break; case QCOW_CRYPT_AES:
If the qcow2 image does not have any encryption method specified in its header, the user should not be providing any encryption options when opening it. We already detect this if the user had set "encrypt.format" but this field is optional so must consider any "encrypt.*" option to be an error. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- block/qcow2.c | 6 ++++++ 1 file changed, 6 insertions(+)