[libvirt] PATCH: Fix redetection of transient QEMU VMs on daemon restarts
When the libvirtd daemon starts up, it reads all config files from /etc/libvirt/qemu. For each config file loaded, it then probes for a pidfile / live status XML config in /var/run/libvirt/qemu. In retrospect there is an obvious problem with this approach: it misses all transient VMs which don't ever have config files in /etc/libvirt/qemu. The core goal of this patch, is thus to change the way we deal with startup to apply the following logic. - Scan & load all status files in /var/run/libvirt/qemu for running VMs - Reconnect to the monitor for all VMs, and re-detect VCPU threads. - Scan & load all inactive configs in /etc/libvirt/qemu This ensures we always re-detect all inactive and running VMs, whether transient or persistent. It also fixes two other bugs, first we forgot to detect VCPU threads, so vCPU affinity functions were broken, and it also re-reserves the MCS category with selinux so it is not re-used later. Finally, if the attempt to reconnect to the QEMU monitor for a running VM fails, then we explicitly kill off that VM, rather than just ignoring it. This avoids leaving orphaned QEMU processes. The patch is larger than anticipated, because I moved all the status file code out of QEMU driver into the generic domain_conf.c file. This enables both scanning loops to be done via the single method virDomainLoadAllConfigs, just by pasing different directories. I also want to re-use this code in the LXC and UML drivers to improve bugs in their logic domain_conf.c | 314 ++++++++++++++++++++++++++++++++++++++++++++++----- domain_conf.h | 17 ++ libvirt_private.syms | 1 lxc_driver.c | 2 qemu_conf.c | 190 ------------------------------ qemu_conf.h | 18 -- qemu_driver.c | 178 +++++++++++++--------------- security.h | 3 security_selinux.c | 39 ++++++ uml_driver.c | 4 10 files changed, 431 insertions(+), 335 deletions(-) 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++; @@ -2621,6 +2633,68 @@ no_memory: return NULL; } + +static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn, + virCapsPtr caps, + xmlXPathContextPtr ctxt) +{ + char *tmp = NULL; + long val; + xmlNodePtr config; + xmlNodePtr oldctxt; + 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; + } + + oldctxt = ctxt->node; + ctxt->node = config; + obj->def = virDomainDefParseXML(conn, caps, ctxt, 0); + ctxt->node = oldctxt; + 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, ...) @@ -2753,6 +2827,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 */ /************************************************************************ @@ -3704,6 +3850,33 @@ 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 cleanup; + + virBufferAdd(&buf, config_xml, strlen(config_xml)); + virBufferAddLit(&buf, "</domstatus>\n"); + + xml = virBufferContentAndReset(&buf); +cleanup: + VIR_FREE(config_xml); + return xml; + +} + #ifndef PROXY @@ -3779,6 +3952,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, @@ -3832,17 +4026,67 @@ 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; @@ -3863,14 +4107,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 @@ -1176,7 +1176,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 @@ -2765,193 +2765,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 @@ -141,13 +132,4 @@ virDomainDefPtr qemuParseCommandLine(vir virDomainDefPtr qemuParseCommandLineString(virConnectPtr conn, 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 @@ -125,6 +125,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; @@ -287,79 +289,65 @@ static int qemudOpenMonitor(virConnectPt const char *monitor, int reconnect); + +/* + * Open an existing VM's monitor, and re-detect VCPUs + * threads + */ +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 @@ -513,14 +501,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); @@ -569,7 +568,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); @@ -1331,6 +1330,7 @@ static int qemudStartVMDaemon(virConnect int pos = -1; char ebuf[1024]; char *pidfile = NULL; + int logfile; struct gemudHookData hookData; hookData.conn = conn; @@ -1372,7 +1372,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; @@ -1421,29 +1421,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)); @@ -1451,7 +1451,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); @@ -1501,7 +1501,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 */ @@ -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; } @@ -1549,14 +1547,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 */ @@ -2260,7 +2252,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; @@ -2310,7 +2302,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; @@ -4192,7 +4184,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: @@ -4314,7 +4306,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 @@ -399,7 +399,7 @@ umlStartup(void) { ¨_driver->domains, uml_driver->configDir, uml_driver->autostartDir, - NULL, NULL) < 0) + 0, NULL, NULL) < 0) goto error; umlAutostartConfigs(uml_driver); @@ -438,7 +438,7 @@ umlReload(void) { ¨_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 :|
On Wed, May 27, 2009 at 04:58:44PM +0100, Daniel P. Berrange wrote:
When the libvirtd daemon starts up, it reads all config files from /etc/libvirt/qemu. For each config file loaded, it then probes for a pidfile / live status XML config in /var/run/libvirt/qemu.
In retrospect there is an obvious problem with this approach: it misses all transient VMs which don't ever have config files in /etc/libvirt/qemu. The core goal of this patch, is thus to change the way we deal with startup to apply the following logic.
- Scan & load all status files in /var/run/libvirt/qemu for running VMs - Reconnect to the monitor for all VMs, and re-detect VCPU threads. - Scan & load all inactive configs in /etc/libvirt/qemu
This ensures we always re-detect all inactive and running VMs, whether transient or persistent.
It also fixes two other bugs, first we forgot to detect VCPU threads, so vCPU affinity functions were broken, and it also re-reserves the MCS category with selinux so it is not re-used later.
Finally, if the attempt to reconnect to the QEMU monitor for a running VM fails, then we explicitly kill off that VM, rather than just ignoring it. This avoids leaving orphaned QEMU processes.
The patch is larger than anticipated, because I moved all the status
Well that's a lot of changes too ! But this sounds good. Maybe we shoudl try some testing like repeating /etc/init.d/libvirtd start and stop a few hundred times while keeping some activity and see what's left at the end ...
file code out of QEMU driver into the generic domain_conf.c file. This enables both scanning loops to be done via the single method virDomainLoadAllConfigs, just by pasing different directories. I also want to re-use this code in the LXC and UML drivers to improve bugs in their logic
okay, so there is some general refactoring too. [...]
+ 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. [...]
+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; + +} [...] +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);
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.
+ */ +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; +} + [...] @@ -1331,6 +1330,7 @@ static int qemudStartVMDaemon(virConnect int pos = -1; char ebuf[1024]; char *pidfile = NULL; + int logfile;
struct gemudHookData hookData; hookData.conn = conn; @@ -1372,7 +1372,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; @@ -1421,29 +1421,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));
@@ -1451,7 +1451,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); @@ -1501,7 +1501,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 */ @@ -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. Looks good except for the couple of things raised earlier, thanks ! ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
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) { ¨_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) { ¨_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 :|
On Wed, Jun 03, 2009 at 05:42:20PM +0100, Daniel P. Berrange wrote:
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
ACK, in case you didn't commit it yet ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel P. Berrange -
Daniel Veillard