On 02/02/2012 07:25 AM, ajia(a)redhat.com wrote:
From: Alex Jia<ajia(a)redhat.com>
---
tools/virsh.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index c8fd448..73c2192 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2787,10 +2787,6 @@ out:
ctxt->node = vol_nodes[vol_i];
VIR_FREE(target);
VIR_FREE(source);
- if (vol) {
- virStorageVolFree(vol);
- vol = NULL;
- }
Actually, this code is needed, as error paths in the loop are handled
gracefuly with a 'continue;' so we need to free the volume on such path;
/* get volume source and target paths */
if (!(target = virXPathString("string(./target/@dev)", ctxt))) {
@@ -2852,6 +2848,10 @@ out:
vol_del_failed = true;
}
vshPrint(ctl, _("Volume '%s' removed.\n"),
volume_tok?volume_tok:source);
+
+ /* cleanup */
+ virStorageVolFree(vol);
+ vol = NULL;
}
Yeah, I actualy forgot to clean up the volume after the loop. I modified
your patch to correct this in the final cleanup:
diff --git a/tools/virsh.c b/tools/virsh.c
index c8fd448..af78102 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2875,6 +2875,8 @@ cleanup:
VIR_FREE(volume_tokens);
VIR_FREE(def);
VIR_FREE(vol_nodes);
+ if (vol)
+ virStorageVolFree(vol);
xmlFreeDoc(doc);
xmlXPathFreeContext(ctxt);
virDomainFree(dom);
and pushed. Thanks for catching this bug.
Peter