On Thu, May 11, 2017 at 04:29:11PM +0200, Alberto Garcia wrote:
> On Tue 25 Apr 2017 05:38:51 PM CEST, Daniel P. Berrange wrote:
> > + switch (s->crypt_method_header) {
> > + case QCOW_CRYPT_NONE:
> > + if (encryptfmt) {
> > + error_setg(errp, "No encryption in image header, but options "
> > + "specified format '%s'", encryptfmt);
> > + goto fail;
> > + }
> > + break;
>
> You forgot to set the return value here (it is undefined at this
> point)...
Opps, yes.
>
> > +
> > + case QCOW_CRYPT_AES:
> > + if (encryptfmt && !g_str_equal(encryptfmt, "aes")) {
> > + error_setg(errp,
> > + "Header reported 'aes' encryption format but "
> > + "options specify '%s'", encryptfmt);
> > + ret = -EINVAL;
> > + goto fail;
> > + }
>
> ...and here you could break instead, the condition immediately after the
> switch sets ret to -EINVAL and returns.
In thi case I think its clearer to set it right here, rather than having
to follow the code paths through & read the later logic
> > +# Since: 2.10
> > +##
> > +{ 'enum': 'BlockdevQcow2EncryptionFormat',
> > + 'data': [ 'qcow' ] }
> > +
>
> Same question here, isn't this supposed to be 'aes' ?
Yes
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 :|
