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 <[email protected]>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to