[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH RFC] virsh: Add option to undefine storage with domains



Adds an option to virsh undefine command to undefine managed
storage volumes along with (inactive) domains. Storage volumes
are enumerated and the user may interactivly choose volumes
to delete.

Unmanaged volumes are listed and the user shall delete them
manualy.
---
I marked this as a RFC because I am concerned about my "naming scheme" of  the added parameters.
I couldn't decide which of the following "volumes/storage/disks/..." to use. I'd appreciate your
comments on this. 

This is my second approach to this problem after I got some really good critique from Eric,
Daniel and Dave. The user has the choice to activate an interactive mode, that allows to select on a 
per-device basis which volumes/disks to remove along with the domain.

To avoid possible problems, I only allowed to remove storage for inactive domains and unmanaged
images (which sidetracked me a lot on my previous attempt) are left to a action of the user.
(the user is notified about any unmanaged image for the domain).

My next concern is about interactive of the user. I tried to implement a boolean query function, 
but I'd like to know if I took the right path, as I couldn't find any example in virsh from which I
could learn.

Thanks for your comments (and time) :)

    Peter Krempa

 tools/virsh.c |  265 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 259 insertions(+), 6 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 61f69f0..3795d2b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -295,6 +295,9 @@ static int vshCommandOptULongLong(const vshCmd *cmd, const char *name,
 static bool vshCommandOptBool(const vshCmd *cmd, const char *name);
 static const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd,
                                           const vshCmdOpt *opt);
+static int vshInteractiveBoolPrompt(vshControl *ctl,
+                                    const char *prompt,
+                                     bool *confirm);

 #define VSH_BYID     (1 << 1)
 #define VSH_BYUUID   (1 << 2)
@@ -1422,6 +1425,8 @@ static const vshCmdInfo info_undefine[] = {
 static const vshCmdOptDef opts_undefine[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name or uuid")},
     {"managed-save", VSH_OT_BOOL, 0, N_("remove domain managed state file")},
+    {"disks", VSH_OT_BOOL, 0, N_("remove associated disk images managed in storage pools (interactive)")},
+    {"disks-all", VSH_OT_BOOL, 0, N_("remove all associated disk images managed in storage pools")},
     {NULL, 0, 0, NULL}
 };

@@ -1434,9 +1439,25 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
     int id;
     int flags = 0;
     int managed_save = vshCommandOptBool(cmd, "managed-save");
+    int remove_disks = vshCommandOptBool(cmd, "disks");
+    int remove_all_disks = vshCommandOptBool(cmd, "disks-all");
     int has_managed_save = 0;
     int rc = -1;

+    char *domxml;
+    xmlDocPtr xml = NULL;
+    xmlXPathContextPtr ctxt = NULL;
+    xmlXPathObjectPtr obj = NULL;
+    xmlNodePtr cur = NULL;
+    int i = 0;
+    char *source = NULL;
+    char *target = NULL;
+    char *type = NULL;
+    xmlBufferPtr xml_buf = NULL;
+    virStorageVolPtr volume = NULL;
+    int state;
+    bool confirm = false;
+
     if (managed_save)
         flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;

@@ -1475,15 +1496,172 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
         }
     }

-    if (flags == -1) {
-        if (has_managed_save == 1) {
+
+    if (flags == -1 && has_managed_save == 1) {
+        vshError(ctl,
+                 _("Refusing to undefine while domain managed save "
+                   "image exists"));
+        virDomainFree(dom);
+        return false;
+    }
+
+    if (remove_disks || remove_all_disks) {
+        if ((state = vshDomainState(ctl, dom, NULL)) < 0) {
+            vshError(ctl, _("Failed to get domain state"));
+            goto disk_error;
+        }
+
+        /* removal of storage is possible only for inactive domains */
+        if (!((state == VIR_DOMAIN_SHUTOFF) ||
+              (state == VIR_DOMAIN_CRASHED))) {
             vshError(ctl,
-                     _("Refusing to undefine while domain managed save "
-                       "image exists"));
-            virDomainFree(dom);
-            return false;
+                     _("Domain needs to be inactive to delete it with associated storage"));
+            goto disk_error;
+        }
+
+        if (remove_disks && !ctl->imode) {
+            vshError(ctl, "%s\n", _("Option --disks is available only in interactive mode"));
+            goto disk_error;
+        }
+
+        domxml = virDomainGetXMLDesc(dom, 0);
+        if (!domxml) {
+            vshError(ctl, "%s", _("Failed to get disk information"));
+            goto disk_error;
+        }
+
+        xml = xmlReadDoc((const xmlChar *) domxml, "domain.xml", NULL,
+                         XML_PARSE_NOENT |
+                         XML_PARSE_NONET |
+                         XML_PARSE_NOWARNING);
+        VIR_FREE(domxml);
+
+        if (!xml) {
+            vshError(ctl, "%s", _("Failed to get disk information"));
+            goto disk_error;
+        }
+
+        ctxt = xmlXPathNewContext(xml);
+        if (!ctxt) {
+            vshError(ctl, "%s", _("Failed to get disk information"));
+            goto disk_error;
+        }
+
+        obj = xmlXPathEval(BAD_CAST "/domain/devices/disk", ctxt);
+        if ((obj == NULL) || (obj->type != XPATH_NODESET) ||
+            (obj->nodesetval == NULL)) {
+            vshError(ctl, "%s", _("Failed to get disk information"));
+            goto disk_error;
+        }
+
+        for (i = 0; i < obj->nodesetval->nodeNr; i++) {
+            cur = obj->nodesetval->nodeTab[i]->children;
+
+            type = virXMLPropString(cur, "device");
+
+            while (cur != NULL) {
+                if (cur->type == XML_ELEMENT_NODE) {
+                    if (xmlStrEqual(cur->name, BAD_CAST "target"))
+                        target = virXMLPropString(cur, "dev");
+                    else if (xmlStrEqual(cur->name, BAD_CAST "source"))
+                        source = virXMLPropString(cur, "file");
+                }
+                cur = cur->next;
+            }
+
+            if (!source) {
+                VIR_FREE(target);
+                VIR_FREE(type);
+            }
+
+            volume = virStorageVolLookupByPath(ctl->conn,  (const char *) source);
+            if (!volume) {
+                vshPrint(ctl, "%s %s %s %s\n",
+                         _("Volume: Source:"), (const char *)source,
+                         _("Target:"), (const char *) target);
+                vshPrint(ctl, _("This volume isn't managed by any storage pool, "
+                                "please delete it manualy\n\n"));
+                /* remove error indication */
+                virFreeError(last_error);
+                last_error = NULL;
+            } else {
+                vshPrint(ctl, "%s %s %s %s\n",
+                         _("Volume: Source:"), (const char *)source,
+                         _("Target:"), (const char *) target);
+
+                if (remove_all_disks) {
+                    confirm = true;
+                } else {
+                    if (vshInteractiveBoolPrompt(ctl,
+                                                 _("Do you want to undefine this volume?"),
+                                                 &confirm) < 0) {
+                        vshError(ctl, _("\nError while geting response from user"));
+                        virStorageVolFree(volume);
+                        goto disk_error;
+                    }
+                }
+
+                /* removal of volume */
+                if (confirm) {
+                    if (virStorageVolDelete(volume, 0) == 0) {
+                        virStorageVolFree(volume);
+
+                        vshPrint(ctl, _("Volume deleted\n\n"));
+
+                        /* remove definition of volume from xml */
+                        xml_buf = xmlBufferCreate();
+                        if (!xml_buf) {
+                            vshPrint(ctl, _("Failed to create XML buffer. "
+                                            "If domain undefinition fails, domain will be left in inconsistent state.\n\n"));
+                            goto disk_next;
+                        }
+
+                        if (xmlNodeDump(xml_buf, xml, obj->nodesetval->nodeTab[i], 0, 0) < 0) {
+                            vshPrint(ctl, _("Failed to extract XML volume description. "
+                                            "If domain undefinition fails, domain will be left in inconsistent state.\n\n"));
+
+                            xmlBufferFree(xml_buf);
+                            xml_buf = NULL;
+                            goto disk_next;
+                        }
+
+                        if (virDomainDetachDeviceFlags(dom,
+                                                       (char *) xmlBufferContent(xml_buf),
+                                                       VIR_DOMAIN_AFFECT_CONFIG) < 0) {
+                            vshPrint(ctl,
+                                     _("Failed to remove volume \"%s\" from configuration. "
+                                       "If domain undefinition fails, domain will be left in inconsistent state.\n\n"),
+                                     source);
+
+                            xmlBufferFree(xml_buf);
+                            xml_buf = NULL;
+                            goto disk_next;
+                        }
+
+                        xmlBufferFree(xml_buf);
+                        xml_buf = NULL;
+
+                    } else {
+                        virStorageVolFree(volume);
+
+                        vshError(ctl, _("Failed to delete volume."));
+                        goto disk_error;
+                    }
+                }
+            }
+
+disk_next:
+            VIR_FREE(source);
+            VIR_FREE(target);
+            VIR_FREE(type);
         }

+        xmlXPathFreeObject(obj);
+        xmlXPathFreeContext(ctxt);
+        xmlFreeDoc(xml);
+    } /* end of disk undefine stuff */
+
+    if (flags == -1) {
         rc = virDomainUndefine(dom);
     } else {
         rc = virDomainUndefineFlags(dom, flags);
@@ -1520,6 +1698,21 @@ end:

     virDomainFree(dom);
     return ret;
+
+disk_error:
+    VIR_FREE(source);
+    VIR_FREE(target);
+    VIR_FREE(type);
+
+    xmlXPathFreeObject(obj);
+    xmlXPathFreeContext(ctxt);
+    xmlFreeDoc(xml);
+    xmlBufferFree(xml_buf);
+
+    virDomainFree(dom);
+
+    vshError(ctl, _("\nFailed to undefine domain %s"), name);
+    return false;
 }


@@ -14736,6 +14929,66 @@ vshReadline (vshControl *ctl, const char *prompt)

 #endif /* !USE_READLINE */

+
+/*
+ * Propmpt for boolean question (yes/no)
+ *
+ * Returns "true" on positive answer, "false" on negative
+ * -1 on error.
+ */
+static int
+vshInteractiveBoolPrompt(vshControl *ctl,
+                         const char *prompt,
+                         bool *confirm) {
+    const char *positive = _("yes");
+    const char *negative = _("no");
+    char *r;
+    char buff[100];
+    int ret = -1;
+    int len;
+    int c;
+
+    if (confirm == NULL)
+        return ret;
+
+    if (!ctl->imode)
+        return ret;
+
+    while (ret == -1) {
+        vshPrint(ctl, "%s (%s/%s)? ", prompt, positive, negative);
+
+        r = fgets(buff, sizeof(buff), stdin);
+        if (r == NULL)
+            break;
+        len = strlen(buff);
+        if (len > 0 && buff[len-1] == '\n')
+            buff[len-1] = '\0';
+        else
+            /* eat rest of line */
+            while ((c = fgetc(stdin) != EOF))
+                if (c == '\n')
+                    break;
+
+        /* compare */
+        if (STRCASEEQLEN(buff, positive, strlen(positive))) {
+            ret = 0;
+            *confirm = true;
+            break;
+        }
+
+        if (STRCASEEQLEN(buff, negative, strlen(negative))) {
+            ret = 0;
+            *confirm = false;
+            break;
+        }
+
+        /* errorneus response - warn and ask again*/
+        vshPrint(ctl, "\"%s\" %s\n", buff, _("Response not understood"));
+
+    }
+    return ret;
+}
+
 /*
  * Deinitialize virsh
  */
-- 
1.7.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]