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

Re: [libvirt] PATCH: 1/4: Remove configFile & autostartLink from virDomainObj



This is a preparatory patch for the later refactoring. The virDomainObj
struct currently contains a configFIle and autostartLink member which
is the path to the /etc/libvirt/{DRIVER}/{NAME}.xml files for QEMU/LXC
drivers. In a subsequent patch we need to store a 'live' config file
in /var/run/libvirt/lxc/{NAME}.xml too. Rather than introduce yet another
struct variable for this, its better to just remove all of them and 
build the path at the places where its actually needed. This is an
iteration of the previous patch I did, using Jim's suggestion of a helper
method to format the filename, instead of repeating asprintf() throughout

 domain_conf.c |  135 +++++++++++++++++++++++++++-------------------------------
 domain_conf.h |   14 +++---
 lxc_driver.c  |   15 ++++--
 qemu_driver.c |   62 ++++++++++++++++++--------
 util.c        |   17 +++++++
 util.h        |    3 +
 6 files changed, 145 insertions(+), 101 deletions(-)


Daniel

diff -r 4af56c5570da src/domain_conf.c
--- a/src/domain_conf.c	Wed Aug 13 14:16:18 2008 +0100
+++ b/src/domain_conf.c	Wed Aug 13 14:16:52 2008 +0100
@@ -421,8 +421,6 @@
     virDomainDefFree(dom->newDef);
 
     VIR_FREE(dom->vcpupids);
-    VIR_FREE(dom->configFile);
-    VIR_FREE(dom->autostartLink);
 
     VIR_FREE(dom);
 }
@@ -3220,31 +3218,19 @@
 
 int virDomainSaveConfig(virConnectPtr conn,
                         const char *configDir,
-                        const char *autostartDir,
-                        virDomainObjPtr dom)
+                        virDomainDefPtr def)
 {
     char *xml;
+    char *configFile = NULL;
     int fd = -1, ret = -1;
     size_t towrite;
     int err;
 
-    if (!dom->configFile &&
-        asprintf(&dom->configFile, "%s/%s.xml",
-                 configDir, dom->def->name) < 0) {
-        dom->configFile = NULL;
-        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+    if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL)
         goto cleanup;
-    }
-    if (!dom->autostartLink &&
-        asprintf(&dom->autostartLink, "%s/%s.xml",
-                 autostartDir, dom->def->name) < 0) {
-        dom->autostartLink = NULL;
-        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
-        goto cleanup;
-    }
 
     if (!(xml = virDomainDefFormat(conn,
-                                   dom->newDef ? dom->newDef : dom->def,
+                                   def,
                                    VIR_DOMAIN_XML_SECURE)))
         goto cleanup;
 
@@ -3255,34 +3241,27 @@
         goto cleanup;
     }
 
-    if ((err = virFileMakePath(autostartDir))) {
-        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                             _("cannot create autostart directory %s: %s"),
-                             autostartDir, strerror(err));
-        goto cleanup;
-    }
-
-    if ((fd = open(dom->configFile,
+    if ((fd = open(configFile,
                    O_WRONLY | O_CREAT | O_TRUNC,
                    S_IRUSR | S_IWUSR )) < 0) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("cannot create config file %s: %s"),
-                              dom->configFile, strerror(errno));
+                             _("cannot create config file %s: %s"),
+                             configFile, strerror(errno));
         goto cleanup;
     }
 
     towrite = strlen(xml);
     if (safewrite(fd, xml, towrite) < 0) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("cannot write config file %s: %s"),
-                              dom->configFile, strerror(errno));
+                             _("cannot write config file %s: %s"),
+                             configFile, strerror(errno));
         goto cleanup;
     }
 
     if (close(fd) < 0) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("cannot save config file %s: %s"),
-                              dom->configFile, strerror(errno));
+                             _("cannot save config file %s: %s"),
+                             configFile, strerror(errno));
         goto cleanup;
     }
 
@@ -3302,25 +3281,18 @@
                                     virDomainObjPtr *doms,
                                     const char *configDir,
                                     const char *autostartDir,
-                                    const char *file)
+                                    const char *name)
 {
     char *configFile = NULL, *autostartLink = NULL;
     virDomainDefPtr def = NULL;
     virDomainObjPtr dom;
     int autostart;
 
-    if (asprintf(&configFile, "%s/%s",
-                 configDir, file) < 0) {
-        configFile = NULL;
-        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+    if ((configFile = virDomainConfigFile(conn, configDir, name)) == NULL)
         goto error;
-    }
-    if (asprintf(&autostartLink, "%s/%s",
-                 autostartDir, file) < 0) {
-        autostartLink = NULL;
-        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+    if ((autostartLink = virDomainConfigFile(conn, autostartDir, name)) == NULL)
         goto error;
-    }
+
 
     if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0)
         goto error;
@@ -3328,20 +3300,10 @@
     if (!(def = virDomainDefParseFile(conn, caps, configFile)))
         goto error;
 
-    if (!virFileMatchesNameSuffix(file, def->name, ".xml")) {
-        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("Domain config filename '%s'"
-                                " does not match domain name '%s'"),
-                              configFile, def->name);
-        goto error;
-    }
-
     if (!(dom = virDomainAssignDef(conn, doms, def)))
         goto error;
 
     dom->state = VIR_DOMAIN_SHUTOFF;
-    dom->configFile = configFile;
-    dom->autostartLink = autostartLink;
     dom->autostart = autostart;
 
     return dom;
@@ -3372,20 +3334,24 @@
     }
 
     while ((entry = readdir(dir))) {
+        virDomainObjPtr dom;
+
         if (entry->d_name[0] == '.')
             continue;
 
-        if (!virFileHasSuffix(entry->d_name, ".xml"))
+        if (!virFileStripSuffix(entry->d_name, ".xml"))
             continue;
 
         /* NB: ignoring errors, so one malformed config doesn't
            kill the whole process */
-        virDomainLoadConfig(conn,
-                            caps,
-                            doms,
-                            configDir,
-                            autostartDir,
-                            entry->d_name);
+        dom = virDomainLoadConfig(conn,
+                                  caps,
+                                  doms,
+                                  configDir,
+                                  autostartDir,
+                                  entry->d_name);
+        if (dom)
+            dom->persistent = 1;
     }
 
     closedir(dir);
@@ -3394,25 +3360,50 @@
 }
 
 int virDomainDeleteConfig(virConnectPtr conn,
-                           virDomainObjPtr dom)
+                          const char *configDir,
+                          const char *autostartDir,
+                          virDomainObjPtr dom)
 {
-    if (!dom->configFile || !dom->autostartLink) {
+    char *configFile = NULL, *autostartLink = NULL;
+    int ret = -1;
+
+    if ((configFile = virDomainConfigFile(conn, configDir, dom->def->name)) == NULL)
+        goto cleanup;
+    if ((autostartLink = virDomainConfigFile(conn, autostartDir, dom->def->name)) == NULL)
+        goto cleanup;
+
+    /* Not fatal if this doesn't work */
+    unlink(autostartLink);
+
+    if (unlink(configFile) < 0 &&
+        errno != ENOENT) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("no config file for %s"), dom->def->name);
-        return -1;
+                             _("cannot remove config for %s: %s"),
+                             dom->def->name, strerror(errno));
+        goto cleanup;
     }
 
-    /* Not fatal if this doesn't work */
-    unlink(dom->autostartLink);
+    ret = 0;
 
-    if (unlink(dom->configFile) < 0) {
-        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("cannot remove config for %s: %s"),
-                             dom->def->name, strerror(errno));
-        return -1;
+cleanup:
+    VIR_FREE(configFile);
+    VIR_FREE(autostartLink);
+    return ret;
+}
+
+char *virDomainConfigFile(virConnectPtr conn,
+                          const char *dir,
+                          const char *name)
+{
+    char *ret = NULL;
+
+    if (asprintf(&ret, "%s/%s.xml", dir, name) < 0) {
+        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+        return NULL;
     }
 
-    return 0;
+    return ret;
 }
 
+
 #endif /* ! PROXY */
diff -r 4af56c5570da src/domain_conf.h
--- a/src/domain_conf.h	Wed Aug 13 14:16:18 2008 +0100
+++ b/src/domain_conf.h	Wed Aug 13 14:16:52 2008 +0100
@@ -458,9 +458,6 @@
     unsigned int autostart : 1;
     unsigned int persistent : 1;
 
-    char *configFile;
-    char *autostartLink;
-
     virDomainDefPtr def; /* The current definition */
     virDomainDefPtr newDef; /* New definition to activate at shutdown */
 
@@ -532,15 +529,14 @@
 
 int virDomainSaveConfig(virConnectPtr conn,
                         const char *configDir,
-                        const char *autostartDir,
-                        virDomainObjPtr dom);
+                        virDomainDefPtr def);
 
 virDomainObjPtr virDomainLoadConfig(virConnectPtr conn,
                                     virCapsPtr caps,
                                     virDomainObjPtr *doms,
                                     const char *configDir,
                                     const char *autostartDir,
-                                    const char *file);
+                                    const char *name);
 
 int virDomainLoadAllConfigs(virConnectPtr conn,
                             virCapsPtr caps,
@@ -549,7 +545,13 @@
                             const char *autostartDir);
 
 int virDomainDeleteConfig(virConnectPtr conn,
+                          const char *configDir,
+                          const char *autostartDir,
                           virDomainObjPtr dom);
+
+char *virDomainConfigFile(virConnectPtr conn,
+                          const char *dir,
+                          const char *name);
 
 virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn,
                         xmlNodePtr node);
diff -r 4af56c5570da src/lxc_driver.c
--- a/src/lxc_driver.c	Wed Aug 13 14:16:18 2008 +0100
+++ b/src/lxc_driver.c	Wed Aug 13 14:16:52 2008 +0100
@@ -250,11 +250,11 @@
         virDomainDefFree(def);
         return NULL;
     }
+    vm->persistent = 1;
 
     if (virDomainSaveConfig(conn,
                             driver->configDir,
-                            driver->autostartDir,
-                            vm) < 0) {
+                            vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainRemoveInactive(&driver->domains, vm);
         return NULL;
     }
@@ -284,10 +284,17 @@
         return -1;
     }
 
-    if (virDomainDeleteConfig(dom->conn, vm) <0)
+    if (!vm->persistent) {
+        lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR,
+                 "%s", _("cannot undefine transient domain"));
         return -1;
+    }
 
-    vm->configFile[0] = '\0';
+    if (virDomainDeleteConfig(dom->conn,
+                              driver->configDir,
+                              driver->autostartDir,
+                              vm) <0)
+        return -1;
 
     virDomainRemoveInactive(&driver->domains, vm);
 
diff -r 4af56c5570da src/qemu_driver.c
--- a/src/qemu_driver.c	Wed Aug 13 14:16:18 2008 +0100
+++ b/src/qemu_driver.c	Wed Aug 13 14:16:52 2008 +0100
@@ -351,7 +351,7 @@
         virDomainObjPtr next = vm->next;
         if (virDomainIsActive(vm))
             qemudShutdownVMDaemon(NULL, qemu_driver, vm);
-        if (!vm->configFile)
+        if (!vm->persistent)
             virDomainRemoveInactive(&qemu_driver->domains,
                                     vm);
         vm = next;
@@ -1071,7 +1071,7 @@
 static int qemudDispatchVMLog(struct qemud_driver *driver, virDomainObjPtr vm, int fd) {
     if (qemudVMData(driver, vm, fd) < 0) {
         qemudShutdownVMDaemon(NULL, driver, vm);
-        if (!vm->configFile)
+        if (!vm->persistent)
             virDomainRemoveInactive(&driver->domains,
                                     vm);
     }
@@ -1081,7 +1081,7 @@
 static int qemudDispatchVMFailure(struct qemud_driver *driver, virDomainObjPtr vm,
                                   int fd ATTRIBUTE_UNUSED) {
     qemudShutdownVMDaemon(NULL, driver, vm);
-    if (!vm->configFile)
+    if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains,
                                 vm);
     return 0;
@@ -2141,7 +2141,7 @@
     }
 
     qemudShutdownVMDaemon(dom->conn, driver, vm);
-    if (!vm->configFile)
+    if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains,
                                 vm);
 
@@ -2472,7 +2472,7 @@
 
     /* Shut it down */
     qemudShutdownVMDaemon(dom->conn, driver, vm);
-    if (!vm->configFile)
+    if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains,
                                 vm);
 
@@ -2764,7 +2764,7 @@
     if (ret < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("failed to start VM"));
-        if (!vm->configFile)
+        if (!vm->persistent)
             virDomainRemoveInactive(&driver->domains,
                                     vm);
         return -1;
@@ -2870,11 +2870,11 @@
         virDomainDefFree(def);
         return NULL;
     }
+    vm->persistent = 1;
 
     if (virDomainSaveConfig(conn,
                             driver->configDir,
-                            driver->autostartDir,
-                            vm) < 0) {
+                            vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainRemoveInactive(&driver->domains,
                                 vm);
         return NULL;
@@ -2901,7 +2901,13 @@
         return -1;
     }
 
-    if (virDomainDeleteConfig(dom->conn, vm) < 0)
+    if (!vm->persistent) {
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+                         "%s", _("cannot undefine transient domain"));
+        return -1;
+    }
+
+    if (virDomainDeleteConfig(dom->conn, driver->configDir, driver->autostartDir, vm) < 0)
         return -1;
 
     virDomainRemoveInactive(&driver->domains,
@@ -3141,13 +3147,21 @@
 }
 
 static int qemudDomainSetAutostart(virDomainPtr dom,
-                            int autostart) {
+                                   int autostart) {
     struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
     virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+    char *configFile = NULL, *autostartLink = NULL;
+    int ret = -1;
 
     if (!vm) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                          "%s", _("no domain with matching uuid"));
+        return -1;
+    }
+
+    if (!vm->persistent) {
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+                         "%s", _("cannot set autostart for transient domain"));
         return -1;
     }
 
@@ -3156,6 +3170,11 @@
     if (vm->autostart == autostart)
         return 0;
 
+    if ((configFile = virDomainConfigFile(dom->conn, driver->configDir, vm->def->name)) == NULL)
+        goto cleanup;
+    if ((autostartLink = virDomainConfigFile(dom->conn, driver->autostartDir, vm->def->name)) == NULL)
+        goto cleanup;
+
     if (autostart) {
         int err;
 
@@ -3163,27 +3182,32 @@
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
                              _("cannot create autostart directory %s: %s"),
                              driver->autostartDir, strerror(err));
-            return -1;
+            goto cleanup;
         }
 
-        if (symlink(vm->configFile, vm->autostartLink) < 0) {
+        if (symlink(configFile, autostartLink) < 0) {
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
-                             _("Failed to create symlink '%s' to '%s': %s"),
-                             vm->autostartLink, vm->configFile, strerror(errno));
-            return -1;
+                             _("Failed to create symlink '%s to '%s': %s"),
+                             autostartLink, configFile, strerror(errno));
+            goto cleanup;
         }
     } else {
-        if (unlink(vm->autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) {
+        if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) {
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
                              _("Failed to delete symlink '%s': %s"),
-                             vm->autostartLink, strerror(errno));
-            return -1;
+                             autostartLink, strerror(errno));
+            goto cleanup;
         }
     }
 
     vm->autostart = autostart;
+    ret = 0;
 
-    return 0;
+cleanup:
+    VIR_FREE(configFile);
+    VIR_FREE(autostartLink);
+
+    return ret;
 }
 
 /* This uses the 'info blockstats' monitor command which was
diff -r 4af56c5570da src/util.c
--- a/src/util.c	Wed Aug 13 14:16:18 2008 +0100
+++ b/src/util.c	Wed Aug 13 14:16:52 2008 +0100
@@ -82,6 +82,23 @@
     }
     __virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, code, VIR_ERR_ERROR,
                     NULL, NULL, NULL, -1, -1, "%s", errorMessage);
+}
+
+int virFileStripSuffix(char *str,
+                       const char *suffix)
+{
+    int len = strlen(str);
+    int suffixlen = strlen(suffix);
+
+    if (len < suffixlen)
+        return 0;
+
+    if (!STREQ(str + len - suffixlen, suffix))
+        return 0;
+
+    str[len-suffixlen] = '\0';
+
+    return 1;
 }
 
 #ifndef __MINGW32__
diff -r 4af56c5570da src/util.h
--- a/src/util.h	Wed Aug 13 14:16:18 2008 +0100
+++ b/src/util.h	Wed Aug 13 14:16:52 2008 +0100
@@ -55,6 +55,9 @@
 int virFileHasSuffix(const char *str,
                      const char *suffix);
 
+int virFileStripSuffix(char *str,
+                       const char *suffix);
+
 int virFileLinkPointsTo(const char *checkLink,
                         const char *checkDest);
 

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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