[libvirt] [PATCH] virsh: fix snapshot-create with no xmlfile

[Ján Tomko]

On 03/11/13 13:48, Peter Krempa wrote:
> On 03/11/13 13:40, Ján Tomko wrote:
>> Properly check the return value of vshCommandOptStringReq for xmlfile:
>> * error out on incorrect input (--xmlfile '')
>> * use default XML <domainsnapshot/> with no --xmlfile specified
>>
>> (Broken by commit b2e8585)
>>
>> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=919826
>> ---
>>   tools/virsh-snapshot.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
>> index ed41014..d994fd9 100644
>> --- a/tools/virsh-snapshot.c
>> +++ b/tools/virsh-snapshot.c
>> @@ -211,7 +211,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
>>       if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
>>           goto cleanup;
>>
>> -    if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) {
>> +    if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0)
>> +        goto cleanup;
>> +    if (!from) {
>>           buffer = vshStrdup(ctl, "<domainsnapshot/>");
> 
> This fallback code is really weird. We should have rejected missing XML
> name by default and encourage to use "snapshot-create-as".
> 
>>       } else {
>>           if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
>>
> 
> ACK to this patch, though. We probably have to support that code forever
> now :(.
> 
> Peter
> 

Thanks, pushed now.

Jan