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

Re: [libvirt] PATCH: Fix redetection of transient QEMU VMs on daemon restarts



On Tue, Jun 02, 2009 at 03:55:27PM +0200, Daniel Veillard wrote:
> On Wed, May 27, 2009 at 04:58:44PM +0100, Daniel P. Berrange wrote:
> [...]
> > +                                            xmlXPathContextPtr ctxt)
> > +{
> > +    char *tmp = NULL;
> > +    long val;
> > +    xmlNodePtr config;
> > +    xmlNodePtr oldctxt;
> 
>   I would s/oldctxt/oldnode/ as what is saved is really only the old
>   XPath current node not the context itself.

Good idea, changed this.

> 
> [...]
> > +char *virDomainObjFormat(virConnectPtr conn,
> > +                         virDomainObjPtr obj,
> > +                         int flags)
> > +{
> > +    char *config_xml = NULL, *xml = NULL;
> > +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> > +
> > +    virBufferVSprintf(&buf, "<domstatus state='%s' pid='%d'>\n",
> > +                      virDomainStateTypeToString(obj->state),
> > +                      obj->pid);
> > +    virBufferEscapeString(&buf, "  <monitor path='%s'/>\n", obj->monitorpath);
> > +
> > +    if (!(config_xml = virDomainDefFormat(conn,
> > +                                          obj->def,
> > +                                          flags)))
> 
>            Hum we are leaking the buffer content here.
> 
> > +        goto cleanup;
> > +
> > +    virBufferAdd(&buf, config_xml, strlen(config_xml));
> > +    virBufferAddLit(&buf, "</domstatus>\n");
> > +
> > +    xml = virBufferContentAndReset(&buf);
> > +cleanup:
> > +    VIR_FREE(config_xml);
> > +    return xml;
> > +
> > +}


Yes, and also forgetting to check virBufferError() to report OOM. Fixed
the cleanup in this function now.


> > +        virDomainObjUnlock(obj);
> > +        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> > +                             _("unexpected domain %s already exists"), obj->def->name);
> 
> 
>   let's wrap to 80 columns
> 
> [...]
> > +/*
> > + * Open an existing VM's monitor, and re-detect VCPUs
> > + * threads
> 
>   maybe update the comment about the security labels too, especially as
> this is a bit arcane.

Updated these two.

> > @@ -1519,10 +1519,8 @@ cleanup:
> >          vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> >          vm->def->graphics[0]->data.vnc.autoport)
> >          vm->def->graphics[0]->data.vnc.port = -1;
> > -    if (vm->logfile != -1) {
> > -        close(vm->logfile);
> > -        vm->logfile = -1;
> > -    }
> > +    if (logfile != -1)
> > +        close(logfile);
> >      vm->def->id = -1;
> >      return -1;
> >  }
> 
>    Hum, do we still use vm->logfile field then ? Maybe I didn't see the
> place where it was removed from the structure.

Yep, this field in the struct has been killed off - see domain_conf.h

Here's the updated patch in full

Daniel


diff --git a/src/domain_conf.c b/src/domain_conf.c
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -39,6 +39,7 @@
 #include "util.h"
 #include "buf.h"
 #include "c-ctype.h"
+#include "logging.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -511,6 +512,31 @@ void virDomainObjListFree(virDomainObjLi
     vms->count = 0;
 }
 
+
+static virDomainObjPtr virDomainObjNew(virConnectPtr conn)
+{
+    virDomainObjPtr domain;
+
+    if (VIR_ALLOC(domain) < 0) {
+        virReportOOMError(conn);
+        return NULL;
+    }
+
+    if (virMutexInit(&domain->lock) < 0) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("cannot initialize mutex"));
+        VIR_FREE(domain);
+        return NULL;
+    }
+
+    virDomainObjLock(domain);
+    domain->state = VIR_DOMAIN_SHUTOFF;
+    domain->monitorWatch = -1;
+    domain->monitor = -1;
+
+    return domain;
+}
+
 virDomainObjPtr virDomainAssignDef(virConnectPtr conn,
                                    virDomainObjListPtr doms,
                                    const virDomainDefPtr def)
@@ -530,29 +556,15 @@ virDomainObjPtr virDomainAssignDef(virCo
         return domain;
     }
 
-    if (VIR_ALLOC(domain) < 0) {
-        virReportOOMError(conn);
-        return NULL;
-    }
-
-    if (virMutexInit(&domain->lock) < 0) {
-        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("cannot initialize mutex"));
-        VIR_FREE(domain);
-        return NULL;
-    }
-
-    virDomainObjLock(domain);
-    domain->state = VIR_DOMAIN_SHUTOFF;
+    if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) {
+        virReportOOMError(conn);
+        return NULL;
+    }
+
+    if (!(domain = virDomainObjNew(conn)))
+        return NULL;
+
     domain->def = def;
-    domain->monitorWatch = -1;
-    domain->monitor = -1;
-
-    if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) {
-        virReportOOMError(conn);
-        VIR_FREE(domain);
-        return NULL;
-    }
 
     doms->objs[doms->count] = domain;
     doms->count++;
@@ -2623,6 +2635,68 @@ no_memory:
     return NULL;
 }
 
+
+static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn,
+                                            virCapsPtr caps,
+                                            xmlXPathContextPtr ctxt)
+{
+    char *tmp = NULL;
+    long val;
+    xmlNodePtr config;
+    xmlNodePtr oldnode;
+    virDomainObjPtr obj;
+
+    if (!(obj = virDomainObjNew(conn)))
+        return NULL;
+
+    if (!(config = virXPathNode(conn, "./domain", ctxt))) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("no domain config"));
+        goto error;
+    }
+
+    oldnode = ctxt->node;
+    ctxt->node = config;
+    obj->def = virDomainDefParseXML(conn, caps, ctxt, 0);
+    ctxt->node = oldnode;
+    if (!obj->def)
+        goto error;
+
+    if (!(tmp = virXPathString(conn, "string(./@state)", ctxt))) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("missing domain state"));
+        goto error;
+    }
+    if ((obj->state = virDomainStateTypeFromString(tmp)) < 0) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             _("invalid domain state '%s'"), tmp);
+        VIR_FREE(tmp);
+        goto error;
+    }
+    VIR_FREE(tmp);
+
+    if ((virXPathLong(conn, "string(./@pid)", ctxt, &val)) < 0) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("invalid pid"));
+        goto error;
+    }
+    obj->pid = (pid_t)val;
+
+    if(!(obj->monitorpath =
+         virXPathString(conn, "string(./monitor[1]/@path)", ctxt))) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("no monitor path"));
+        goto error;
+    }
+
+    return obj;
+
+error:
+    virDomainObjFree(obj);
+    return NULL;
+}
+
+
 /* Called from SAX on parsing errors in the XML. */
 static void
 catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
@@ -2755,6 +2829,78 @@ cleanup:
     xmlXPathFreeContext(ctxt);
     return def;
 }
+
+
+virDomainObjPtr virDomainObjParseFile(virConnectPtr conn,
+                                      virCapsPtr caps,
+                                      const char *filename)
+{
+    xmlParserCtxtPtr pctxt;
+    xmlDocPtr xml = NULL;
+    xmlNodePtr root;
+    virDomainObjPtr obj = NULL;
+
+    /* Set up a parser context so we can catch the details of XML errors. */
+    pctxt = xmlNewParserCtxt ();
+    if (!pctxt || !pctxt->sax)
+        goto cleanup;
+    pctxt->sax->error = catchXMLError;
+    pctxt->_private = conn;
+
+    if (conn) virResetError (&conn->err);
+    xml = xmlCtxtReadFile (pctxt, filename, NULL,
+                           XML_PARSE_NOENT | XML_PARSE_NONET |
+                           XML_PARSE_NOWARNING);
+    if (!xml) {
+        if (virGetLastError() == NULL)
+              virDomainReportError(conn, VIR_ERR_XML_ERROR,
+                                   "%s", _("failed to parse xml document"));
+        goto cleanup;
+    }
+
+    if ((root = xmlDocGetRootElement(xml)) == NULL) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                              "%s", _("missing root element"));
+        goto cleanup;
+    }
+
+    obj = virDomainObjParseNode(conn, caps, xml, root);
+
+cleanup:
+    xmlFreeParserCtxt (pctxt);
+    xmlFreeDoc (xml);
+    return obj;
+}
+
+
+virDomainObjPtr virDomainObjParseNode(virConnectPtr conn,
+                                      virCapsPtr caps,
+                                      xmlDocPtr xml,
+                                      xmlNodePtr root)
+{
+    xmlXPathContextPtr ctxt = NULL;
+    virDomainObjPtr obj = NULL;
+
+    if (!xmlStrEqual(root->name, BAD_CAST "domstatus")) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("incorrect root element"));
+        goto cleanup;
+    }
+
+    ctxt = xmlXPathNewContext(xml);
+    if (ctxt == NULL) {
+        virReportOOMError(conn);
+        goto cleanup;
+    }
+
+    ctxt->node = root;
+    obj = virDomainObjParseXML(conn, caps, ctxt);
+
+cleanup:
+    xmlXPathFreeContext(ctxt);
+    return obj;
+}
+
 #endif /* ! PROXY */
 
 /************************************************************************
@@ -3707,6 +3853,40 @@ char *virDomainDefFormat(virConnectPtr c
     return NULL;
 }
 
+char *virDomainObjFormat(virConnectPtr conn,
+                         virDomainObjPtr obj,
+                         int flags)
+{
+    char *config_xml = NULL, *xml = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+    virBufferVSprintf(&buf, "<domstatus state='%s' pid='%d'>\n",
+                      virDomainStateTypeToString(obj->state),
+                      obj->pid);
+    virBufferEscapeString(&buf, "  <monitor path='%s'/>\n", obj->monitorpath);
+
+    if (!(config_xml = virDomainDefFormat(conn,
+                                          obj->def,
+                                          flags)))
+        goto error;
+
+    virBufferAdd(&buf, config_xml, strlen(config_xml));
+    VIR_FREE(config_xml);
+    virBufferAddLit(&buf, "</domstatus>\n");
+
+    if (virBufferError(&buf))
+        goto no_memory;
+
+    return virBufferContentAndReset(&buf);
+
+no_memory:
+    virReportOOMError(conn);
+error:
+    xml = virBufferContentAndReset(&buf);
+    VIR_FREE(xml);
+    return NULL;
+}
+
 
 #ifndef PROXY
 
@@ -3782,6 +3962,27 @@ cleanup:
     return ret;
 }
 
+int virDomainSaveStatus(virConnectPtr conn,
+                        const char *statusDir,
+                        virDomainObjPtr obj)
+{
+    int ret = -1;
+    char *xml;
+
+    if (!(xml = virDomainObjFormat(conn,
+                                   obj,
+                                   VIR_DOMAIN_XML_SECURE)))
+        goto cleanup;
+
+    if (virDomainSaveXML(conn, statusDir, obj->def, xml))
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    VIR_FREE(xml);
+    return ret;
+}
+
 
 virDomainObjPtr virDomainLoadConfig(virConnectPtr conn,
                                     virCapsPtr caps,
@@ -3835,17 +4036,68 @@ error:
     return NULL;
 }
 
+static virDomainObjPtr virDomainLoadStatus(virConnectPtr conn,
+                                           virCapsPtr caps,
+                                           virDomainObjListPtr doms,
+                                           const char *statusDir,
+                                           const char *name,
+                                           virDomainLoadConfigNotify notify,
+                                           void *opaque)
+{
+    char *statusFile = NULL;
+    virDomainObjPtr obj = NULL;
+    virDomainObjPtr tmp = NULL;
+
+    if ((statusFile = virDomainConfigFile(conn, statusDir, name)) == NULL)
+        goto error;
+
+    if (!(obj = virDomainObjParseFile(conn, caps, statusFile)))
+        goto error;
+
+    tmp = virDomainFindByName(doms, obj->def->name);
+    if (tmp) {
+        virDomainObjUnlock(obj);
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             _("unexpected domain %s already exists"),
+                             obj->def->name);
+        goto error;
+    }
+
+    if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) {
+        virReportOOMError(conn);
+        goto error;
+    }
+
+    doms->objs[doms->count] = obj;
+    doms->count++;
+
+    if (notify)
+        (*notify)(obj, 1, opaque);
+
+    VIR_FREE(statusFile);
+    return obj;
+
+error:
+    virDomainObjFree(obj);
+    VIR_FREE(statusFile);
+    return NULL;
+}
+
 int virDomainLoadAllConfigs(virConnectPtr conn,
                             virCapsPtr caps,
                             virDomainObjListPtr doms,
                             const char *configDir,
                             const char *autostartDir,
+                            int liveStatus,
                             virDomainLoadConfigNotify notify,
                             void *opaque)
 {
     DIR *dir;
     struct dirent *entry;
 
+    VIR_INFO("Scanning for %s configs in %s",
+             liveStatus ? "live" : "persistent", configDir);
+
     if (!(dir = opendir(configDir))) {
         if (errno == ENOENT)
             return 0;
@@ -3866,14 +4118,24 @@ int virDomainLoadAllConfigs(virConnectPt
 
         /* NB: ignoring errors, so one malformed config doesn't
            kill the whole process */
-        dom = virDomainLoadConfig(conn,
-                                  caps,
-                                  doms,
-                                  configDir,
-                                  autostartDir,
-                                  entry->d_name,
-                                  notify,
-                                  opaque);
+        VIR_INFO("Loading config file '%s.xml'", entry->d_name);
+        if (liveStatus)
+            dom = virDomainLoadStatus(conn,
+                                      caps,
+                                      doms,
+                                      configDir,
+                                      entry->d_name,
+                                      notify,
+                                      opaque);
+        else
+            dom = virDomainLoadConfig(conn,
+                                      caps,
+                                      doms,
+                                      configDir,
+                                      autostartDir,
+                                      entry->d_name,
+                                      notify,
+                                      opaque);
         if (dom) {
             virDomainObjUnlock(dom);
             dom->persistent = 1;
diff --git a/src/domain_conf.h b/src/domain_conf.h
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -515,7 +515,6 @@ struct _virDomainObj {
     int monitor;
     char *monitorpath;
     int monitorWatch;
-    int logfile;
     int pid;
     int state;
 
@@ -589,10 +588,22 @@ virDomainDefPtr virDomainDefParseNode(vi
                                       xmlDocPtr doc,
                                       xmlNodePtr root,
                                       int flags);
+
+virDomainObjPtr virDomainObjParseFile(virConnectPtr conn,
+                                      virCapsPtr caps,
+                                      const char *filename);
+virDomainObjPtr virDomainObjParseNode(virConnectPtr conn,
+                                      virCapsPtr caps,
+                                      xmlDocPtr xml,
+                                      xmlNodePtr root);
+
 #endif
 char *virDomainDefFormat(virConnectPtr conn,
                          virDomainDefPtr def,
                          int flags);
+char *virDomainObjFormat(virConnectPtr conn,
+                         virDomainObjPtr obj,
+                         int flags);
 
 int virDomainCpuSetParse(virConnectPtr conn,
                          const char **str,
@@ -615,6 +626,9 @@ int virDomainSaveXML(virConnectPtr conn,
 int virDomainSaveConfig(virConnectPtr conn,
                         const char *configDir,
                         virDomainDefPtr def);
+int virDomainSaveStatus(virConnectPtr conn,
+                        const char *statusDir,
+                        virDomainObjPtr obj);
 
 typedef void (*virDomainLoadConfigNotify)(virDomainObjPtr dom,
                                           int newDomain,
@@ -634,6 +648,7 @@ int virDomainLoadAllConfigs(virConnectPt
                             virDomainObjListPtr doms,
                             const char *configDir,
                             const char *autostartDir,
+                            int liveStatus,
                             virDomainLoadConfigNotify notify,
                             void *opaque);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -94,6 +94,7 @@ virDomainObjListFree;
 virDomainRemoveInactive;
 virDomainSaveXML;
 virDomainSaveConfig;
+virDomainSaveStatus;
 virDomainSoundDefFree;
 virDomainSoundModelTypeFromString;
 virDomainSoundModelTypeToString;
diff --git a/src/lxc_driver.c b/src/lxc_driver.c
--- a/src/lxc_driver.c
+++ b/src/lxc_driver.c
@@ -1190,7 +1190,7 @@ static int lxcStartup(void)
                                 &lxc_driver->domains,
                                 lxc_driver->configDir,
                                 lxc_driver->autostartDir,
-                                NULL, NULL) < 0)
+                                0, NULL, NULL) < 0)
         goto cleanup;
 
     for (i = 0 ; i < lxc_driver->domains.count ; i++) {
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -2798,193 +2798,3 @@ cleanup:
     return def;
 }
 
-
-/* Called from SAX on parsing errors in the XML. */
-static void
-catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
-{
-    xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx;
-
-    if (ctxt) {
-        virConnectPtr conn = ctxt->_private;
-
-        if (ctxt->lastError.level == XML_ERR_FATAL &&
-            ctxt->lastError.message != NULL) {
-            qemudReportError (conn, NULL, NULL, VIR_ERR_XML_DETAIL,
-                                  _("at line %d: %s"),
-                                  ctxt->lastError.line,
-                                  ctxt->lastError.message);
-        }
-    }
-}
-
-
-/**
- * qemudDomainStatusParseFile
- *
- * read the last known status of a domain
- *
- * Returns 0 on success
- */
-qemudDomainStatusPtr
-qemudDomainStatusParseFile(virConnectPtr conn,
-                           virCapsPtr caps,
-                           const char *filename, int flags)
-{
-    xmlParserCtxtPtr pctxt = NULL;
-    xmlXPathContextPtr ctxt = NULL;
-    xmlDocPtr xml = NULL;
-    xmlNodePtr root, config_root;
-    virDomainDefPtr def = NULL;
-    char *tmp = NULL;
-    long val;
-    qemudDomainStatusPtr status = NULL;
-
-    if (VIR_ALLOC(status) < 0) {
-        virReportOOMError(conn);
-        goto error;
-    }
-
-    /* Set up a parser context so we can catch the details of XML errors. */
-    pctxt = xmlNewParserCtxt ();
-    if (!pctxt || !pctxt->sax)
-        goto error;
-    pctxt->sax->error = catchXMLError;
-    pctxt->_private = conn;
-
-    if (conn) virResetError (&conn->err);
-    xml = xmlCtxtReadFile (pctxt, filename, NULL,
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
-                           XML_PARSE_NOWARNING);
-    if (!xml) {
-        if (conn && conn->err.code == VIR_ERR_NONE)
-              qemudReportError(conn, NULL, NULL, VIR_ERR_XML_ERROR,
-                                   "%s", _("failed to parse xml document"));
-        goto error;
-    }
-
-    if ((root = xmlDocGetRootElement(xml)) == NULL) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                              "%s", _("missing root element"));
-        goto error;
-    }
-
-    ctxt = xmlXPathNewContext(xml);
-    if (ctxt == NULL) {
-        virReportOOMError(conn);
-        goto error;
-    }
-
-    if (!xmlStrEqual(root->name, BAD_CAST "domstatus")) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("incorrect root element"));
-        goto error;
-    }
-
-    ctxt->node = root;
-    if(!(tmp = virXPathString(conn, "string(./@state)", ctxt))) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("invalid domain state"));
-        goto error;
-    } else {
-        status->state = virDomainStateTypeFromString(tmp);
-        VIR_FREE(tmp);
-    }
-
-    if((virXPathLong(conn, "string(./@pid)", ctxt, &val)) < 0) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("invalid pid"));
-        goto error;
-    } else
-        status->pid = (pid_t)val;
-
-    if(!(tmp = virXPathString(conn, "string(./monitor[1]/@path)", ctxt))) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("no monitor path"));
-        goto error;
-    } else
-        status->monitorpath = tmp;
-
-    if(!(config_root = virXPathNode(conn, "./domain", ctxt))) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("no domain config"));
-        goto error;
-    }
-    if(!(def = virDomainDefParseNode(conn, caps, xml, config_root, flags)))
-        goto error;
-    else
-        status->def = def;
-
-cleanup:
-    xmlFreeParserCtxt (pctxt);
-    xmlXPathFreeContext(ctxt);
-    xmlFreeDoc (xml);
-    return status;
-
-error:
-    VIR_FREE(tmp);
-    VIR_FREE(status);
-    goto cleanup;
-}
-
-
-/**
- * qemudDomainStatusFormat
- *
- * Get the state of a running domain as XML
- *
- * Returns xml on success
- */
-static char*
-qemudDomainStatusFormat(virConnectPtr conn,
-                        virDomainObjPtr vm)
-{
-    char *config_xml = NULL, *xml = NULL;
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
-
-    virBufferVSprintf(&buf, "<domstatus state='%s' pid='%d'>\n",
-                      virDomainStateTypeToString(vm->state),
-                      vm->pid);
-    virBufferEscapeString(&buf, "  <monitor path='%s'/>\n", vm->monitorpath);
-
-    if (!(config_xml = virDomainDefFormat(conn,
-                                          vm->def,
-                                          VIR_DOMAIN_XML_SECURE)))
-        goto cleanup;
-
-    virBufferAdd(&buf, config_xml, strlen(config_xml));
-    virBufferAddLit(&buf, "</domstatus>\n");
-
-    xml = virBufferContentAndReset(&buf);
-cleanup:
-    VIR_FREE(config_xml);
-    return xml;
-}
-
-
-/**
- * qemudSaveDomainStatus
- *
- * Save the current status of a running domain
- *
- * Returns 0 on success
- */
-int
-qemudSaveDomainStatus(virConnectPtr conn,
-                      struct qemud_driver *driver,
-                      virDomainObjPtr vm)
-{
-    int ret = -1;
-    char *xml = NULL;
-
-    if (!(xml = qemudDomainStatusFormat(conn, vm)))
-        goto cleanup;
-
-    if ((ret = virDomainSaveXML(conn, driver->stateDir, vm->def, xml)))
-        goto cleanup;
-
-    ret = 0;
-cleanup:
-    VIR_FREE(xml);
-    return ret;
-}
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -92,15 +92,6 @@ struct qemud_driver {
     virSecurityDriverPtr securityDriver;
 };
 
-/* Status needed to reconenct to running VMs */
-typedef struct _qemudDomainStatus qemudDomainStatus;
-typedef qemudDomainStatus *qemudDomainStatusPtr;
-struct _qemudDomainStatus {
-    char *monitorpath;
-    pid_t pid;
-    int state;
-    virDomainDefPtr def;
-};
 
 /* Port numbers used for KVM migration. */
 #define QEMUD_MIGRATION_FIRST_PORT 49152
@@ -143,13 +134,4 @@ virDomainDefPtr qemuParseCommandLineStri
                                            virCapsPtr caps,
                                            const char *args);
 
-const char *qemudVirtTypeToString       (int type);
-qemudDomainStatusPtr qemudDomainStatusParseFile(virConnectPtr conn,
-                                                virCapsPtr caps,
-                                                const char *filename,
-                                                int flags);
-int qemudSaveDomainStatus(virConnectPtr conn,
-                          struct qemud_driver *driver,
-                          virDomainObjPtr vm);
-
 #endif /* __QEMUD_CONF_H */
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -120,6 +120,8 @@ static int qemudMonitorCommandExtra(cons
 static int qemudDomainSetMemoryBalloon(virConnectPtr conn,
                                        virDomainObjPtr vm,
                                        unsigned long newmem);
+static int qemudDetectVcpuPIDs(virConnectPtr conn,
+                               virDomainObjPtr vm);
 
 static struct qemud_driver *qemu_driver = NULL;
 
@@ -282,79 +284,65 @@ static int qemudOpenMonitor(virConnectPt
                             const char *monitor,
                             int reconnect);
 
+
+/*
+ * Open an existing VM's monitor, re-detect VCPU threads
+ * and re-reserve the security labels in use
+ */
+static int
+qemuReconnectDomain(struct qemud_driver *driver,
+                    virDomainObjPtr obj)
+{
+    int rc;
+
+    if ((rc = qemudOpenMonitor(NULL, driver, obj, obj->monitorpath, 1)) != 0) {
+        VIR_ERROR(_("Failed to reconnect monitor for %s: %d\n"),
+                  obj->def->name, rc);
+        goto error;
+    }
+
+    if (qemudDetectVcpuPIDs(NULL, obj) < 0) {
+        goto error;
+    }
+
+    if (obj->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
+        driver->securityDriver &&
+        driver->securityDriver->domainReserveSecurityLabel &&
+        driver->securityDriver->domainReserveSecurityLabel(NULL, obj) < 0)
+        return -1;
+
+    if (obj->def->id >= driver->nextvmid)
+        driver->nextvmid = obj->def->id + 1;
+
+    return 0;
+
+error:
+    return -1;
+}
+
 /**
  * qemudReconnectVMs
  *
- * Reconnect running vms to the daemon process
- */
-static int
-qemudReconnectVMs(struct qemud_driver *driver)
+ * Try to re-open the resources for live VMs that we care
+ * about.
+ */
+static void
+qemuReconnectDomains(struct qemud_driver *driver)
 {
     int i;
 
     for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjPtr vm = driver->domains.objs[i];
-        qemudDomainStatusPtr status = NULL;
-        char *config = NULL;
-        int rc;
-
-        virDomainObjLock(vm);
-        if ((rc = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0)
-            DEBUG("Found pid %d for '%s'", vm->pid, vm->def->name);
-        else
-            goto next;
-
-        if ((config = virDomainConfigFile(NULL,
-                                          driver->stateDir,
-                                          vm->def->name)) == NULL) {
-            VIR_ERROR(_("Failed to read domain status for %s\n"),
-                      vm->def->name);
-            goto next_error;
-        }
-
-        status = qemudDomainStatusParseFile(NULL, driver->caps, config, 0);
-        if (status) {
-            vm->newDef = vm->def;
-            vm->def = status->def;
-        } else {
-            VIR_ERROR(_("Failed to parse domain status for %s\n"),
-                      vm->def->name);
-            goto next_error;
-        }
-
-        if ((rc = qemudOpenMonitor(NULL, driver, vm, status->monitorpath, 1)) != 0) {
-            VIR_ERROR(_("Failed to reconnect monitor for %s: %d\n"),
-                      vm->def->name, rc);
-            goto next_error;
-        }
-
-        if ((vm->logfile = qemudLogFD(NULL, driver->logDir, vm->def->name)) < 0)
-            goto next_error;
-
-        if (vm->def->id >= driver->nextvmid)
-            driver->nextvmid = vm->def->id + 1;
-
-        vm->state = status->state;
-        goto next;
-
-next_error:
-        /* we failed to reconnect the vm so remove it's traces */
-        vm->def->id = -1;
-        qemudRemoveDomainStatus(NULL, driver, vm);
-        /* Restore orignal def, if we'd loaded a live one */
-        if (vm->newDef) {
-            virDomainDefFree(vm->def);
-            vm->def = vm->newDef;
-            vm->newDef = NULL;
-        }
-next:
-        virDomainObjUnlock(vm);
-        if (status)
-            VIR_FREE(status->monitorpath);
-        VIR_FREE(status);
-        VIR_FREE(config);
-    }
-    return 0;
+        virDomainObjPtr obj = driver->domains.objs[i];
+
+        virDomainObjLock(obj);
+        if (qemuReconnectDomain(driver, obj) < 0) {
+            /* If we can't get the monitor back, then kill the VM
+             * so user has ability to start it again later without
+             * danger of ending up running twice */
+            qemudShutdownVMDaemon(NULL, driver, obj);
+        }
+        virDomainObjUnlock(obj);
+    }
 }
 
 static int
@@ -508,14 +496,25 @@ qemudStartup(void) {
         goto error;
     }
 
+    /* Get all the running persistent or transient configs first */
+    if (virDomainLoadAllConfigs(NULL,
+                                qemu_driver->caps,
+                                &qemu_driver->domains,
+                                qemu_driver->stateDir,
+                                NULL,
+                                1, NULL, NULL) < 0)
+        goto error;
+
+    qemuReconnectDomains(qemu_driver);
+
+    /* Then inactive persistent configs */
     if (virDomainLoadAllConfigs(NULL,
                                 qemu_driver->caps,
                                 &qemu_driver->domains,
                                 qemu_driver->configDir,
                                 qemu_driver->autostartDir,
-                                NULL, NULL) < 0)
-        goto error;
-    qemudReconnectVMs(qemu_driver);
+                                0, NULL, NULL) < 0)
+        goto error;
     qemuDriverUnlock(qemu_driver);
 
     qemudAutostartConfigs(qemu_driver);
@@ -564,7 +563,7 @@ qemudReload(void) {
                             &qemu_driver->domains,
                             qemu_driver->configDir,
                             qemu_driver->autostartDir,
-                            qemudNotifyLoadDomain, qemu_driver);
+                            0, qemudNotifyLoadDomain, qemu_driver);
     qemuDriverUnlock(qemu_driver);
 
     qemudAutostartConfigs(qemu_driver);
@@ -1329,6 +1328,7 @@ static int qemudStartVMDaemon(virConnect
     int pos = -1;
     char ebuf[1024];
     char *pidfile = NULL;
+    int logfile;
 
     struct gemudHookData hookData;
     hookData.conn = conn;
@@ -1370,7 +1370,7 @@ static int qemudStartVMDaemon(virConnect
         goto cleanup;
     }
 
-    if((vm->logfile = qemudLogFD(conn, driver->logDir, vm->def->name)) < 0)
+    if ((logfile = qemudLogFD(conn, driver->logDir, vm->def->name)) < 0)
         goto cleanup;
 
     emulator = vm->def->emulator;
@@ -1419,29 +1419,29 @@ static int qemudStartVMDaemon(virConnect
 
     tmp = progenv;
     while (*tmp) {
-        if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
+        if (safewrite(logfile, *tmp, strlen(*tmp)) < 0)
             VIR_WARN(_("Unable to write envv to logfile: %s\n"),
                      virStrerror(errno, ebuf, sizeof ebuf));
-        if (safewrite(vm->logfile, " ", 1) < 0)
+        if (safewrite(logfile, " ", 1) < 0)
             VIR_WARN(_("Unable to write envv to logfile: %s\n"),
                      virStrerror(errno, ebuf, sizeof ebuf));
         tmp++;
     }
     tmp = argv;
     while (*tmp) {
-        if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
+        if (safewrite(logfile, *tmp, strlen(*tmp)) < 0)
             VIR_WARN(_("Unable to write argv to logfile: %s\n"),
                      virStrerror(errno, ebuf, sizeof ebuf));
-        if (safewrite(vm->logfile, " ", 1) < 0)
+        if (safewrite(logfile, " ", 1) < 0)
             VIR_WARN(_("Unable to write argv to logfile: %s\n"),
                      virStrerror(errno, ebuf, sizeof ebuf));
         tmp++;
     }
-    if (safewrite(vm->logfile, "\n", 1) < 0)
+    if (safewrite(logfile, "\n", 1) < 0)
         VIR_WARN(_("Unable to write argv to logfile: %s\n"),
                  virStrerror(errno, ebuf, sizeof ebuf));
 
-    if ((pos = lseek(vm->logfile, 0, SEEK_END)) < 0)
+    if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
         VIR_WARN(_("Unable to seek to end of logfile: %s\n"),
                  virStrerror(errno, ebuf, sizeof ebuf));
 
@@ -1449,7 +1449,7 @@ static int qemudStartVMDaemon(virConnect
         FD_SET(tapfds[i], &keepfd);
 
     ret = virExecDaemonize(conn, argv, progenv, &keepfd, &child,
-                           stdin_fd, &vm->logfile, &vm->logfile,
+                           stdin_fd, &logfile, &logfile,
                            VIR_EXEC_NONBLOCK,
                            qemudSecurityHook, &hookData,
                            pidfile);
@@ -1499,7 +1499,7 @@ static int qemudStartVMDaemon(virConnect
         (qemudInitCpus(conn, vm, migrateFrom) < 0) ||
         (qemudInitPasswords(conn, driver, vm) < 0) ||
         (qemudDomainSetMemoryBalloon(conn, vm, vm->def->memory) < 0) ||
-        (qemudSaveDomainStatus(conn, qemu_driver, vm) < 0)) {
+        (virDomainSaveStatus(conn, driver->stateDir, vm) < 0)) {
         qemudShutdownVMDaemon(conn, driver, vm);
         ret = -1;
         /* No need for 'goto cleanup' now since qemudShutdownVMDaemon does enough */
@@ -1517,10 +1517,8 @@ cleanup:
         vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
         vm->def->graphics[0]->data.vnc.autoport)
         vm->def->graphics[0]->data.vnc.port = -1;
-    if (vm->logfile != -1) {
-        close(vm->logfile);
-        vm->logfile = -1;
-    }
+    if (logfile != -1)
+        close(logfile);
     vm->def->id = -1;
     return -1;
 }
@@ -1547,14 +1545,8 @@ static void qemudShutdownVMDaemon(virCon
         vm->monitorWatch = -1;
     }
 
-    if (close(vm->logfile) < 0) {
-        char ebuf[1024];
-        VIR_WARN(_("Unable to close logfile: %s\n"),
-                 virStrerror(errno, ebuf, sizeof ebuf));
-    }
     if (vm->monitor != -1)
         close(vm->monitor);
-    vm->logfile = -1;
     vm->monitor = -1;
 
     /* shut it off for sure */
@@ -2183,7 +2175,7 @@ static int qemudDomainSuspend(virDomainP
                                          VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
         VIR_FREE(info);
     }
-    if (qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
+    if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0)
         goto cleanup;
     ret = 0;
 
@@ -2233,7 +2225,7 @@ static int qemudDomainResume(virDomainPt
                                          VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
         VIR_FREE(info);
     }
-    if (qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
+    if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0)
         goto cleanup;
     ret = 0;
 
@@ -4116,7 +4108,7 @@ static int qemudDomainAttachDevice(virDo
         goto cleanup;
     }
 
-    if (!ret && qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
+    if (!ret && virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0)
         ret = -1;
 
 cleanup:
@@ -4238,7 +4230,7 @@ static int qemudDomainDetachDevice(virDo
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                          "%s", _("only SCSI or virtio disk device can be detached dynamically"));
 
-    if (!ret && qemudSaveDomainStatus(dom->conn, driver, vm) < 0)
+    if (!ret && virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0)
         ret = -1;
 
 cleanup:
diff --git a/src/security.h b/src/security.h
--- a/src/security.h
+++ b/src/security.h
@@ -38,6 +38,8 @@ typedef int (*virSecurityDomainSetImageL
                                                virDomainDiskDefPtr disk);
 typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn,
                                           virDomainObjPtr sec);
+typedef int (*virSecurityDomainReserveLabel) (virConnectPtr conn,
+                                           virDomainObjPtr sec);
 typedef int (*virSecurityDomainGetLabel) (virConnectPtr conn,
                                           virDomainObjPtr vm,
                                           virSecurityLabelPtr sec);
@@ -57,6 +59,7 @@ struct _virSecurityDriver {
     virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel;
     virSecurityDomainSetImageLabel domainSetSecurityImageLabel;
     virSecurityDomainGenLabel domainGenSecurityLabel;
+    virSecurityDomainReserveLabel domainReserveSecurityLabel;
     virSecurityDomainGetLabel domainGetSecurityLabel;
     virSecurityDomainSetLabel domainSetSecurityLabel;
     virSecurityDomainRestoreLabel domainRestoreSecurityLabel;
diff --git a/src/security_selinux.c b/src/security_selinux.c
--- a/src/security_selinux.c
+++ b/src/security_selinux.c
@@ -216,6 +216,44 @@ done:
 }
 
 static int
+SELinuxReserveSecurityLabel(virConnectPtr conn,
+                            virDomainObjPtr vm)
+{
+    security_context_t pctx;
+    context_t ctx = NULL;
+    const char *mcs;
+
+    if (getpidcon(vm->pid, &pctx) == -1) {
+        char ebuf[1024];
+        virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: error calling "
+                               "getpidcon(): %s"), __func__,
+                               virStrerror(errno, ebuf, sizeof ebuf));
+        return -1;
+    }
+
+    ctx = context_new(pctx);
+    VIR_FREE(pctx);
+    if (!ctx)
+        goto err;
+
+    mcs = context_range_get(ctx);
+    if (!mcs)
+        goto err;
+
+    mcsAdd(mcs);
+
+    context_free(ctx);
+
+    return 0;
+
+err:
+    context_free(ctx);
+    return -1;
+}
+
+
+
+static int
 SELinuxSecurityDriverProbe(void)
 {
     return is_selinux_enabled() ? SECURITY_DRIVER_ENABLE : SECURITY_DRIVER_DISABLE;
@@ -422,6 +460,7 @@ virSecurityDriver virSELinuxSecurityDriv
     .domainSetSecurityImageLabel = SELinuxSetSecurityImageLabel,
     .domainRestoreSecurityImageLabel = SELinuxRestoreSecurityImageLabel,
     .domainGenSecurityLabel     = SELinuxGenSecurityLabel,
+    .domainReserveSecurityLabel     = SELinuxReserveSecurityLabel,
     .domainGetSecurityLabel     = SELinuxGetSecurityLabel,
     .domainRestoreSecurityLabel = SELinuxRestoreSecurityLabel,
     .domainSetSecurityLabel     = SELinuxSetSecurityLabel,
diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -394,7 +394,7 @@ umlStartup(void) {
                                 &uml_driver->domains,
                                 uml_driver->configDir,
                                 uml_driver->autostartDir,
-                                NULL, NULL) < 0)
+                                0, NULL, NULL) < 0)
         goto error;
 
     umlAutostartConfigs(uml_driver);
@@ -433,7 +433,7 @@ umlReload(void) {
                             &uml_driver->domains,
                             uml_driver->configDir,
                             uml_driver->autostartDir,
-                            NULL, NULL);
+                            0, NULL, NULL);
 
     umlAutostartConfigs(uml_driver);
     umlDriverUnlock(uml_driver);


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