Remove configFile/autostartLink vars from virDomainObj struct

ChangeLog

@@ -1,3 +1,12 @@
+Wed Aug 20 42:42:09 BST 2008 Daniel P. Berrange <berrange@redhat.com>
+
+	* src/util.c, src/util.h: Add convenience APIs for stripping
+	a file extension
+	* src/domain_conf.c, src/domain_conf.h, src/lxc_driver.c,
+	src/qemu_driver.c: Remove fixed configfile/autostartlink
+	fields in virDomainObjPtr. Generate paths on-demand at time
+	of use
+
 Wed Aug 20 15:42:09 CEST 2008 Daniel Veillard <veillard@redhat.com>
 
 	* src/openvz_conf.c src/openvz_driver.c: patch from Evgeniy Sokolov

src/domain_conf.c

@@ -421,8 +421,6 @@ void virDomainObjFree(virDomainObjPtr dom)
     virDomainDefFree(dom->newDef);
 
     VIR_FREE(dom->vcpupids);
-    VIR_FREE(dom->configFile);
-    VIR_FREE(dom->autostartLink);
 
     VIR_FREE(dom);
 }
@@ -3220,31 +3218,19 @@ char *virDomainDefFormat(virConnectPtr conn,
 
 int virDomainSaveConfig(virConnectPtr conn,
                         const char *configDir,
-                        const char *autostartDir,
+                        virDomainDefPtr def)
-                        virDomainObjPtr dom)
 {
     char *xml;
+    char *configFile = NULL;
     int fd = -1, ret = -1;
     size_t towrite;
     int err;
 
-    if (!dom->configFile &&
+    if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL)
-        asprintf(&dom->configFile, "%s/%s.xml",
-                 configDir, dom->def->name) < 0) {
-        dom->configFile = NULL;
-        virDomainReportError(conn, VIR_ERR_NO_MEMORY, 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 @@ int virDomainSaveConfig(virConnectPtr conn,
         goto cleanup;
     }
 
-    if ((err = virFileMakePath(autostartDir))) {
+    if ((fd = open(configFile,
-        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                             _("cannot create autostart directory %s: %s"),
-                             autostartDir, strerror(err));
-        goto cleanup;
-    }
-
-    if ((fd = open(dom->configFile,
                    O_WRONLY | O_CREAT | O_TRUNC,
                    S_IRUSR | S_IWUSR )) < 0) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("cannot create config file %s: %s"),
+                             _("cannot create config file %s: %s"),
-                              dom->configFile, strerror(errno));
+                             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"),
+                             _("cannot write config file %s: %s"),
-                              dom->configFile, strerror(errno));
+                             configFile, strerror(errno));
         goto cleanup;
     }
 
     if (close(fd) < 0) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("cannot save config file %s: %s"),
+                             _("cannot save config file %s: %s"),
-                              dom->configFile, strerror(errno));
+                             configFile, strerror(errno));
         goto cleanup;
     }
 
@@ -3302,25 +3281,18 @@ virDomainObjPtr virDomainLoadConfig(virConnectPtr conn,
                                     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",
+    if ((configFile = virDomainConfigFile(conn, configDir, name)) == NULL)
-                 configDir, file) < 0) {
-        configFile = NULL;
-        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
         goto error;
-    }
+    if ((autostartLink = virDomainConfigFile(conn, autostartDir, name)) == NULL)
-    if (asprintf(&autostartLink, "%s/%s",
-                 autostartDir, file) < 0) {
-        autostartLink = NULL;
-        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
         goto error;
-    }
+
 
     if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0)
         goto error;
@@ -3328,20 +3300,10 @@ virDomainObjPtr virDomainLoadConfig(virConnectPtr conn,
     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 @@ int virDomainLoadAllConfigs(virConnectPtr conn,
     }
 
     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,
+        dom = virDomainLoadConfig(conn,
-                            caps,
+                                  caps,
-                            doms,
+                                  doms,
-                            configDir,
+                                  configDir,
-                            autostartDir,
+                                  autostartDir,
-                            entry->d_name);
+                                  entry->d_name);
+        if (dom)
+            dom->persistent = 1;
     }
 
     closedir(dir);
@@ -3394,25 +3360,50 @@ int virDomainLoadAllConfigs(virConnectPtr conn,
 }
 
 int virDomainDeleteConfig(virConnectPtr conn,
-                           virDomainObjPtr dom)
+                          const char *configDir,
+                          const char *autostartDir,
+                          virDomainObjPtr dom)
 {
-    if (!dom->configFile || !dom->autostartLink) {
+    char *configFile = NULL, *autostartLink = NULL;
-        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+    int ret = -1;
-                              _("no config file for %s"), dom->def->name);
+
-        return -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(dom->autostartLink);
+    unlink(autostartLink);
 
-    if (unlink(dom->configFile) < 0) {
+    if (unlink(configFile) < 0 &&
+        errno != ENOENT) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("cannot remove config for %s: %s"),
+                             _("cannot remove config for %s: %s"),
                              dom->def->name, strerror(errno));
-        return -1;
+        goto cleanup;
     }
 
-    return 0;
+    ret = 0;
+
+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 ret;
+}
+
+
 #endif /* ! PROXY */

src/domain_conf.h

@@ -458,9 +458,6 @@ struct _virDomainObj {
     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 @@ char *virDomainCpuSetFormat(virConnectPtr conn,
 
 int virDomainSaveConfig(virConnectPtr conn,
                         const char *configDir,
-                        const char *autostartDir,
+                        virDomainDefPtr def);
-                        virDomainObjPtr dom);
 
 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,8 +545,14 @@ int virDomainLoadAllConfigs(virConnectPtr conn,
                             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);
 

src/lxc_driver.c

@@ -250,11 +250,11 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml)
         virDomainDefFree(def);
         return NULL;
     }
+    vm->persistent = 1;
 
     if (virDomainSaveConfig(conn,
                             driver->configDir,
-                            driver->autostartDir,
+                            vm->newDef ? vm->newDef : vm->def) < 0) {
-                            vm) < 0) {
         virDomainRemoveInactive(&driver->domains, vm);
         return NULL;
     }
@@ -284,10 +284,17 @@ static int lxcDomainUndefine(virDomainPtr dom)
         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);
 

src/qemu_driver.c

@@ -351,7 +351,7 @@ qemudShutdown(void) {
         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;
@@ -1072,7 +1072,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
 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);
     }
@@ -1082,7 +1082,7 @@ static int qemudDispatchVMLog(struct qemud_driver *driver, virDomainObjPtr vm, i
 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;
@@ -2162,7 +2162,7 @@ static int qemudDomainDestroy(virDomainPtr dom) {
     }
 
     qemudShutdownVMDaemon(dom->conn, driver, vm);
-    if (!vm->configFile)
+    if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains,
                                 vm);
 
@@ -2493,7 +2493,7 @@ static int qemudDomainSave(virDomainPtr dom,
 
     /* Shut it down */
     qemudShutdownVMDaemon(dom->conn, driver, vm);
-    if (!vm->configFile)
+    if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains,
                                 vm);
 
@@ -2785,7 +2785,7 @@ static int qemudDomainRestore(virConnectPtr conn,
     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;
@@ -2891,11 +2891,11 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
         virDomainDefFree(def);
         return NULL;
     }
+    vm->persistent = 1;
 
     if (virDomainSaveConfig(conn,
                             driver->configDir,
-                            driver->autostartDir,
+                            vm->newDef ? vm->newDef : vm->def) < 0) {
-                            vm) < 0) {
         virDomainRemoveInactive(&driver->domains,
                                 vm);
         return NULL;
@@ -2922,7 +2922,13 @@ static int qemudDomainUndefine(virDomainPtr dom) {
         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,
@@ -3162,9 +3168,11 @@ static int qemudDomainGetAutostart(virDomainPtr dom,
 }
 
 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,
@@ -3172,11 +3180,22 @@ static int qemudDomainSetAutostart(virDomainPtr dom,
         return -1;
     }
 
+    if (!vm->persistent) {
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+                         "%s", _("cannot set autostart for transient domain"));
+        return -1;
+    }
+
     autostart = (autostart != 0);
 
     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;
 
@@ -3184,27 +3203,32 @@ static int qemudDomainSetAutostart(virDomainPtr dom,
             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"),
+                             _("Failed to create symlink '%s to '%s': %s"),
-                             vm->autostartLink, vm->configFile, strerror(errno));
+                             autostartLink, configFile, strerror(errno));
-            return -1;
+            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));
+                             autostartLink, strerror(errno));
-            return -1;
+            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

src/util.c

@@ -89,6 +89,23 @@ ReportError(virConnectPtr conn,
                     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__
 
 static int virSetCloseExec(int fd) {

src/util.h

@@ -55,6 +55,9 @@ int virFileMatchesNameSuffix(const char *file,
 int virFileHasSuffix(const char *str,
                      const char *suffix);
 
+int virFileStripSuffix(char *str,
+                       const char *suffix);
+
 int virFileLinkPointsTo(const char *checkLink,
                         const char *checkDest);