On 11/22/2012 02:41 PM, Peter Krempa wrote:

When the value of memspec was empty taking of a snapshot failed without reporting an error. --- tools/virsh-snapshot.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 398730c..8ec6456 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -358,18 +358,19 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (desc) virBufferEscapeString(&buf, " <description>%s</description>\n", desc);

- if (vshCommandOptString(cmd, "memspec", &memspec) < 0 || - vshParseSnapshotMemspec(ctl, &buf, memspec) < 0) { - virBufferFreeAndReset(&buf); + if (vshCommandOptString(cmd, "memspec", &memspec) < 0) { + vshError(ctl, _("memspec argument must not be empty")); goto cleanup; } + + if (memspec && vshParseSnapshotMemspec(ctl, &buf, memspec) < 0) + goto cleanup; + if (vshCommandOptBool(cmd, "diskspec")) { virBufferAddLit(&buf, " <disks>\n"); while ((opt = vshCommandOptArgv(cmd, opt))) { - if (vshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) { - virBufferFreeAndReset(&buf); + if (vshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) goto cleanup; - } } virBufferAddLit(&buf, " </disks>\n"); } @@ -390,6 +391,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) ret = vshSnapshotCreate(ctl, dom, buffer, flags, NULL);

cleanup: + virBufferFreeAndReset(&buf); VIR_FREE(buffer); if (dom) virDomainFree(dom);

I've missed the leak in the first version, thanks for finding that out. I double-checked this one and it seems alright, so ACK. Martin