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

6 Aug 2008


      "Daniel P. Berrange" <berrange@redhat.com> wrote:
...

ACK, modulo two questions:
...
diff -r a204a9425afd src/domain_conf.c
 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",
Shouldn't that be "%s/%s.xml", as used in at least two
other places where configFile is defined?
This deserves a tiny helper function, so that the
dom->configFile mapping happens in just one place.
...
+                 configDir, dom->def->name) < 0) {
+        configFile = NULL;
...
...
#endif /* ! PROXY */
...
diff -r a204a9425afd src/qemu_driver.c
...
 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) {
Your 6/7 patch has this:

    if (asprintf(&autostartLink, "%s/%s",

Is the different format deliberate?
Maybe a helper function for the dom->autostartLink name, too?
...
+        autostartLink = NULL;
+        qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
+        goto cleanup;
+    }
+
...
+cleanup:
+    VIR_FREE(configFile);
+    VIR_FREE(autostartLink);
+
+    return ret;
 }
/* This uses the 'info blockstats' monitor command which was

    

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

Jim Meyering