[libvirt] [PATCH 2/2] virsh-snapshot: Reject --no-metadata together with --print-xml

Peter Krempa pkrempa at redhat.com
Mon Feb 11 13:56:31 UTC 2013


On 02/11/13 14:47, Osier Yang wrote:
> On 2013年02月11日 21:10, Peter Krempa wrote:
>> Manual for "virsh snapshot-create-as" states that --no-metadata and
>> --print-xml are incompatible. Honor this detail in the code.
>> ---
>>   tools/virsh-snapshot.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
>> index fe1cee9..d9659d4 100644
>> --- a/tools/virsh-snapshot.c
>> +++ b/tools/virsh-snapshot.c
>> @@ -412,8 +412,14 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd
>> *cmd)
>>       unsigned int flags = 0;
>>       const vshCmdOpt *opt = NULL;
>>
>> -    if (vshCommandOptBool(cmd, "no-metadata"))
>> +    if (vshCommandOptBool(cmd, "no-metadata")) {
>> +        if (vshCommandOptBool(cmd, "print-xml")) {
>
> Considering using a variable to store the return value instead of
> calling the function twice.

I was considering that option too, but it's used just twice in the 
function and gets called twice only if --no-metadata is specified. This 
case is ultra-rare.

>
>> +            vshError(ctl, "%s",
>> +                     _("--print-xml is incompatible with
>> --no-metadata"));
>> +            return false;
>> +        }
>>           flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA;
>> +    }
>>       if (vshCommandOptBool(cmd, "halt"))
>>           flags |= VIR_DOMAIN_SNAPSHOT_CREATE_HALT;
>>       if (vshCommandOptBool(cmd, "disk-only"))
>
> But I'm fine if you keep the twice calling, as it's trivial. ACK.

I've pushed the series. Thanks for the review.

Peter




More information about the libvir-list mailing list