From: Eric Blake <eblake@redhat.com>
To: "Benoît Canet" <benoit.canet@irqsave.net>,
"Kevin Wolf" <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com
Subject: Re: [Qemu-devel] [PATCH 1/3] blockdev: Fail blockdev-add with encrypted images
Date: Thu, 06 Mar 2014 09:21:55 -0700 [thread overview]
Message-ID: <5318A0A3.4010400@redhat.com> (raw)
In-Reply-To: <20140306160236.GD22291@irqsave.net>
[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]
On 03/06/2014 09:02 AM, Benoît Canet wrote:
> The Thursday 06 Mar 2014 à 16:44:27 (+0100), Kevin Wolf wrote :
>> Encrypted images need a password before they can be used, and we don't
>> want blockdev-add to create BDSes that aren't fully initialised. So for
>> now simply forbid encrypted images; we can come back to it later if we
>> need the functionality.
>>
>> - blockdev_init(NULL, qdict, &local_err);
>> + dinfo = blockdev_init(NULL, qdict, &local_err);
>> if (local_err) {
>> error_propagate(errp, local_err);
>> goto fail;
>> }
>>
>> + if (bdrv_key_required(dinfo->bdrv)) {
>> + drive_uninit(dinfo);
>> + error_setg(errp, "blockdev-add doesn't support encrypted devices");
>
>> + goto fail;
> This goto fail; is an extra the code will flow to fail anyway.
Ah, but notice how the above block also had the same pattern, and now
that we injected a new if clause, the earlier block is still correct.
This is defensive programming, so that future additions don't have to
mess with control flow of earlier code when adding new code later on.
I'm happy with the patch as-is:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2014-03-06 16:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-06 15:44 [Qemu-devel] [PATCH 0/3] blockdev: blockdev-add error path fixes and tests Kevin Wolf
2014-03-06 15:44 ` [Qemu-devel] [PATCH 1/3] blockdev: Fail blockdev-add with encrypted images Kevin Wolf
2014-03-06 16:02 ` Benoît Canet
2014-03-06 16:21 ` Eric Blake [this message]
2014-03-06 15:44 ` [Qemu-devel] [PATCH 2/3] blockdev: Fix NULL pointer dereference in blockdev-add Kevin Wolf
2014-03-06 16:06 ` Benoît Canet
2014-03-06 16:22 ` Eric Blake
2014-03-06 15:44 ` [Qemu-devel] [PATCH 3/3] qemu-iotests: Test a few blockdev-add error cases Kevin Wolf
2014-03-06 16:13 ` Benoît Canet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5318A0A3.4010400@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=benoit.canet@irqsave.net \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).