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

[Doug Goldstein]

On Mon, Mar 11, 2013 at 7:48 AM, Peter Krempa <pkrempa at redhat.com> 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

Pushed to v1.0.3-maint

Thanks.
-- 
Doug Goldstein