Re: [libvirt] PATCH: 5/7: Remove paths from virDomainObjPtr

Tuesday, 5 August 2008

This patch removes the configDir and autostartLink paths from the internal
virDomainObjPtr struct. Instead they can calculated at time of use. This
is to facilitate a following patch, where we want to write a config to an
alternative location at some points. This also makes the LXC & QEMU drivers
correctly set/use the 'persistent' property.

 domain_conf.c |   99 +++++++++++++++++++++++++++++-----------------------------
 domain_conf.h |    8 +---
 lxc_driver.c  |   40 ++++++++++++++++++-----
 qemu_driver.c |   71 ++++++++++++++++++++++++++++++-----------
 4 files changed, 138 insertions(+), 80 deletions(-)

Daniel

diff -r a204a9425afd src/domain_conf.c

--- a/src/domain_conf.c	Tue Aug 05 16:50:49 2008 +0100
+++ b/src/domain_conf.c	Tue Aug 05 16:50:51 2008 +0100
@@ -399,8 +399,6 @@
     virDomainDefFree(dom->newDef);
 
     VIR_FREE(dom->vcpupids);
-    VIR_FREE(dom->configFile);
-    VIR_FREE(dom->autostartLink);
 
     VIR_FREE(dom);
 }
@@ -2952,31 +2950,23 @@
 
 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);
-        goto cleanup;
-    }
-    if (!dom->autostartLink &&
-        asprintf(&dom->autostartLink, "%s/%s.xml",
-                 autostartDir, dom->def->name) < 0) {
-        dom->autostartLink = NULL;
+    if (asprintf(&configFile, "%s/%s.xml",
+                 configDir, def->name) < 0) {
+        configFile = 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;
 
@@ -2987,34 +2977,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;
     }
 
@@ -3072,8 +3055,6 @@
         goto error;
 
     dom->state = VIR_DOMAIN_SHUTOFF;
-    dom->configFile = configFile;
-    dom->autostartLink = autostartLink;
     dom->autostart = autostart;
 
     return dom;
@@ -3104,6 +3085,8 @@
     }
 
     while ((entry = readdir(dir))) {
+        virDomainObjPtr dom;
+
         if (entry->d_name[0] == '.')
             continue;
 
@@ -3112,12 +3095,14 @@
 
         /* 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);
@@ -3126,25 +3111,43 @@
 }
 
 int virDomainDeleteConfig(virConnectPtr conn,
-                           virDomainObjPtr dom)
+                          const char *configDir,
+                          const char *autostartDir,
+                          virDomainObjPtr dom)
 {
-    if (!dom->configFile || !dom->autostartLink) {
-        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("no config file for %s"),
dom->def->name);
-        return -1;
+    char *configFile = NULL, *autostartLink = NULL;
+    int ret = -1;
+
+    if (asprintf(&configFile, "%s/%s",
+                 configDir, dom->def->name) < 0) {
+        configFile = NULL;
+        virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+        goto cleanup;
+    }
+    if (asprintf(&autostartLink, "%s/%s",
+                 autostartDir, dom->def->name) < 0) {
+        autostartLink = NULL;
+        virDomainReportError(conn, VIR_ERR_NO_MEMORY, 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;
 }
 
 #endif /* ! PROXY */
diff -r a204a9425afd src/domain_conf.h
--- a/src/domain_conf.h	Tue Aug 05 16:50:49 2008 +0100
+++ b/src/domain_conf.h	Tue Aug 05 16:50:51 2008 +0100
@@ -409,9 +409,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 */
 
@@ -482,8 +479,7 @@
 
 int virDomainSaveConfig(virConnectPtr conn,
                         const char *configDir,
-                        const char *autostartDir,
-                        virDomainObjPtr dom);
+                        virDomainDefPtr def);
 
 virDomainObjPtr virDomainLoadConfig(virConnectPtr conn,
                                     virCapsPtr caps,
@@ -499,6 +495,8 @@
                             const char *autostartDir);
 
 int virDomainDeleteConfig(virConnectPtr conn,
+                          const char *configDir,
+                          const char *autostartDir,
                           virDomainObjPtr dom);
 
 virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn,
diff -r a204a9425afd src/lxc_driver.c
--- a/src/lxc_driver.c	Tue Aug 05 16:50:49 2008 +0100
+++ b/src/lxc_driver.c	Tue Aug 05 16:50:51 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);
 
@@ -639,8 +646,13 @@
         return;
     }
 
-    if (lxcVmTerminate(NULL, vm, SIGINT) < 0)
-        virEventRemoveHandle(fd);
+    lxcVmTerminate(NULL, vm, SIGINT);
+
+    virEventRemoveHandle(fd);
+
+    if (!vm->persistent)
+        virDomainRemoveInactive(&driver->domains,
+                                vm);
 }
 
 
@@ -846,6 +858,7 @@
 {
     lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData;
     virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id);
+    int ret;
 
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
@@ -853,7 +866,12 @@
         return -1;
     }
 
-    return lxcVmTerminate(dom->conn, vm, 0);
+    ret = lxcVmTerminate(dom->conn, vm, 0);
+    if (!vm->persistent)
+        virDomainRemoveInactive(&driver->domains,
+                                vm);
+
+    return ret;
 }
 
 
@@ -869,6 +887,7 @@
 {
     lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData;
     virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id);
+    int ret;
 
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
@@ -876,7 +895,12 @@
         return -1;
     }
 
-    return lxcVmTerminate(dom->conn, vm, SIGKILL);
+    ret = lxcVmTerminate(dom->conn, vm, SIGKILL);
+    if (!vm->persistent)
+        virDomainRemoveInactive(&driver->domains,
+                                vm);
+
+    return ret;
 }
 
 static int lxcCheckNetNsSupport(void)
diff -r a204a9425afd src/qemu_driver.c
--- a/src/qemu_driver.c	Tue Aug 05 16:50:49 2008 +0100
+++ b/src/qemu_driver.c	Tue Aug 05 16:50:51 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;
@@ -1068,7 +1068,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);
     }
@@ -1078,7 +1078,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;
@@ -2138,7 +2138,7 @@
     }
 
     qemudShutdownVMDaemon(dom->conn, driver, vm);
-    if (!vm->configFile)
+    if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains,
                                 vm);
 
@@ -2469,7 +2469,7 @@
 
     /* Shut it down */
     qemudShutdownVMDaemon(dom->conn, driver, vm);
-    if (!vm->configFile)
+    if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains,
                                 vm);
 
@@ -2761,7 +2761,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;
@@ -2867,11 +2867,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;
@@ -2898,7 +2898,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,
@@ -3024,13 +3030,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;
     }
 
@@ -3039,6 +3053,20 @@
     if (vm->autostart == autostart)
         return 0;
 
+    if (asprintf(&configFile, "%s/%s.xml",
+                 driver->configDir, vm->def->name) < 0) {
+        configFile = NULL;
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
+        goto cleanup;
+    }
+
+    if (asprintf(&autostartLink, "%s/%s.xml",
+                 driver->autostartDir, vm->def->name) < 0) {
+        autostartLink = NULL;
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
+        goto cleanup;
+    }
+
     if (autostart) {
         int err;
 
@@ -3046,27 +3074,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


-- 
|: 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 :|


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [libvirt] PATCH: 5/7: Remove paths from virDomainObjPtr