[libvirt] [PATCH 0/2] undefine with storage fixes
Two fixes related to the virsh undefine --storage command. Peter Krempa (2): virsh: man: Mention that volumes need to be in storage pool for undefine virsh: domain: Fix undefine with storage of 'volume' disks tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 3 ++- 2 files changed, 36 insertions(+), 3 deletions(-) -- 1.8.5.1
https://bugzilla.redhat.com/show_bug.cgi?id=1044445 When undefining a VM with storage the man page doesn't explicitly mention that the volumes need to be a part of the storage pool otherwise it won't work. --- tools/virsh.pod | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 677931f..59213c6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1794,7 +1794,8 @@ on inactive domains. Volume deletion is only attempted after the domain is undefined; if not all of the requested volumes could be deleted, the error message indicates what still remains behind. If a volume path is not found in the domain definition, it's treated as if the volume was successfully -deleted. +deleted. Only volumes managed by libvirt in storage pools can be removed this +way. (See B<domblklist> for list of target names associated to a domain). Example: --storage vda,/path/to/storage.img -- 1.8.5.1
On 12/18/2013 07:10 AM, Peter Krempa wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1044445
When undefining a VM with storage the man page doesn't explicitly mention that the volumes need to be a part of the storage pool otherwise it won't work. --- tools/virsh.pod | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK. (Although this is yet another place where libvirt auto-creating temporary storage pools for any disk referenced by a domain might be nice...)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 677931f..59213c6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1794,7 +1794,8 @@ on inactive domains. Volume deletion is only attempted after the domain is undefined; if not all of the requested volumes could be deleted, the error message indicates what still remains behind. If a volume path is not found in the domain definition, it's treated as if the volume was successfully -deleted. +deleted. Only volumes managed by libvirt in storage pools can be removed this +way. (See B<domblklist> for list of target names associated to a domain). Example: --storage vda,/path/to/storage.img
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/18/13 16:16, Eric Blake wrote:
On 12/18/2013 07:10 AM, Peter Krempa wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1044445
When undefining a VM with storage the man page doesn't explicitly mention that the volumes need to be a part of the storage pool otherwise it won't work. --- tools/virsh.pod | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK. (Although this is yet another place where libvirt auto-creating temporary storage pools for any disk referenced by a domain might be nice...)
I'm not quite sure if we want to expose the temporary storage pool functionality outside of libvirtd. Let's see how the review of my gluster series will end up where we can discuss that. Peter P.S.: Pushed.
The undefine code that removes the storage along with the VM didn't take into acount the existence of 'volume' type disks. Add the functionality. --- tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8b80e1e..760dca5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2939,6 +2939,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) int nvol_nodes; char *source = NULL; char *target = NULL; + char *pool = NULL; size_t i; size_t j; @@ -3048,6 +3049,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) vshUndefineVolume vol; VIR_FREE(source); VIR_FREE(target); + VIR_FREE(pool); /* get volume source and target paths */ if (!(target = virXPathString("string(./target/@dev)", ctxt))) @@ -3057,9 +3059,12 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) "./source/@file|" "./source/@dir|" "./source/@name|" - "./source/@dev)", ctxt))) + "./source/@dev|" + "./source/@volume)", ctxt))) continue; + pool = virXPathString("string(./source/@pool)", ctxt); + /* lookup if volume was selected by user */ if (vol_list) { bool found = false; @@ -3075,7 +3080,33 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) continue; } - if (!(vol.vol = virStorageVolLookupByPath(ctl->conn, source))) { + if (pool) { + virStoragePoolPtr storagepool = NULL; + + if (!source) { + vshPrint(ctl, + _("Missing storage volume name for disk '%s'"), + target); + continue; + } + + if (!(storagepool = virStoragePoolLookupByName(ctl->conn, + pool))) { + vshPrint(ctl, + _("Storage pool '%s' for volume '%s' not found."), + pool, target); + vshResetLibvirtError(); + continue; + } + + vol.vol = virStorageVolLookupByName(storagepool, source); + virStoragePoolFree(storagepool); + + } else { + vol.vol = virStorageVolLookupByPath(ctl->conn, source); + } + + if (!vol.vol) { vshPrint(ctl, _("Storage volume '%s'(%s) is not managed by libvirt. " "Remove it manually.\n"), target, source); @@ -3190,6 +3221,7 @@ out: cleanup: VIR_FREE(source); VIR_FREE(target); + VIR_FREE(pool); for (i = 0; i < nvols; i++) { VIR_FREE(vols[i].source); VIR_FREE(vols[i].target); -- 1.8.5.1
On 12/18/2013 07:10 AM, Peter Krempa wrote:
The undefine code that removes the storage along with the VM didn't take into acount the existence of 'volume' type disks. Add the functionality.
s/acount/account/
--- tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/18/13 16:18, Eric Blake wrote:
On 12/18/2013 07:10 AM, Peter Krempa wrote:
The undefine code that removes the storage along with the VM didn't take into acount the existence of 'volume' type disks. Add the functionality.
s/acount/account/
--- tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
ACK.
Pushed; Thanks. Peter
participants (2)
-
Eric Blake -
Peter Krempa