Re: [libvirt] [PATCH v3 7/8] undefine: Extend virsh undefine to support the new API
On 07/18/2011 01:54 AM, Osier Yang wrote:
If the domain has managed state file, and --managed-state is
not specified, then it fails with an error telling the user
that a managed save image still exists.
Commit message is now out of date with the code, where you renamed
things --managed-save.
If the domain has managed state file, and --managed-state is
specified, it invokes virDomainUndefineFlags. If
virDomainUndefineFlags fails, then it tries to remove the managed
save image using virDomainManagedSaveRemove first, with
invoking virDomainUndefine following. (For compatibility between
new virsh with this patch and older libvirt without this patch).
Similarly if the domain has no managed state. See the codes for
detail.
NOTE: Have not removing the codes checking if the domain is running
in function "cmdUndefine", it will go along with qemu driver's fix
(allow to undefine a running domain).
Makes sense; after all, that's a separate fix.
@@ -1447,11 +1458,50 @@ cmdUndefine(vshControl *ctl, const vshCmd
*cmd)
virDomainFree(dom);
return false;
}
+
if (!(dom = vshCommandOptDomainBy(ctl, cmd,&name,
VSH_BYNAME|VSH_BYUUID)))
return false;
- if (virDomainUndefine(dom) == 0) {
+ has_managed_save = virDomainHasManagedSaveImage(dom, 0);
+ if (has_managed_save< 0) {
+ virDomainFree(dom);
+ return false;
+ }
Not quite right. If last_error->code == VIR_ERR_NO_SUPPORT, then the
reason that virDomainHasManagedSaveImage returned -1 is because the
hypervisor _can't_ create managed save, and therefore, we should not
fail the undefine (because the undefine by definition won't have any
managed state to worry about).
This logic needs to be updated to return false only if last_error->code
is something besides VIR_ERR_NO_SUPPORT (meaning we hit a real error on
a hypervisor that supports managed save, but can't answer us, rather
than a hypervisor that doesn't have managed save in the first place).
+
+ if (flags == -1) {
+ if (has_managed_save == 1) {
+ vshError(ctl,
+ _("Refusing to undefine while domain managed save "
+ "image exists"));
+ virDomainFree(dom);
+ return false;
+ }
+
+ rc = virDomainUndefine(dom);
+ } else {
+ rc = virDomainUndefineFlags(dom, flags);
+
+ /* It might fail for virDomainUndefineFlags is not
+ * supported on older libvirt, try to undefine the
+ * domain with combo virDomainManagedSaveRemove and
+ * virDomainUndefine.
+ */
+ if (rc< 0) {
+ virErrorPtr err = virGetLastError();
Overkill; look at how cmdVcpucount uses the virsh.c global last_error,
rather than having to call virGetLastError(). That is, virsh already
hooked up a custom handler that captures all errors at the point they
are created, so you can then query last_error, rather than having to
make more virGetLastError API calls yourself.
+++ b/tools/virsh.pod
@@ -807,10 +807,18 @@ hypervisor.
Output the device used for the TTY console of the domain. If the information
is not available the processes will provide an exit code of 1.
-=item B<undefine> I<domain-id>
+=item B<undefine> I<domain-id> optional I<--managed-save>
Now that my virsh.pod cleanup is in (commit 08d3b0a2), this needs to be:
=item B<undefine> I<domain-id> [I<--managed-save>]
-Undefine the configuration for an inactive domain. Since it's not running
-the domain name or UUID must be used as the I<domain-id>.
+Undefine a domain. If the domain is running, this converts it to a
+transient domain, without stopping it. If the domain is inactive,
+the domain configuration is removed.
+
+The I<--managed-save> flag guarantees that any managed save image(see
+the B<managedsave> command) is also cleaned up. Without the flag, attempts
+to undefine a domain with managed save image will fail.
s/with managed/with a managed/
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org