Re: [libvirt] [PATCH 07/12] libxl: Introduce libxlDriverConfig object

The libxlDriverPrivate struct contains an variety of data with varying access needs. Similar to the QEMU and LXC drivers, move all the static config data into a dedicated libxlDriverConfig object. The only locking requirement is to hold the driver lock while obtaining an instance of libxlDriverConfig. Once a reference is held on the config object, it can be used completely lockless since it is immutable. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/libxl/libxl_conf.c | 124 ++++++++++++++++++- src/libxl/libxl_conf.h | 52 +++++--- src/libxl/libxl_driver.c | 313 +++++++++++++++++++++++------------------------ 3 files changed, 309 insertions(+), 180 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 231a53d..19fd8a6 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -64,6 +64,41 @@ static const char *xen_cap_re = "(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(x86_32|x static regex_t xen_cap_rec; +static virClassPtr libxlDriverConfigClass; +static void libxlDriverConfigDispose(void *obj); + +static int libxlConfigOnceInit(void) +{ + if (!(libxlDriverConfigClass = virClassNew(virClassForObject(), + "libxlDriverConfig", + sizeof(libxlDriverConfig), + libxlDriverConfigDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(libxlConfig) + +static void +libxlDriverConfigDispose(void *obj) +{ + libxlDriverConfigPtr cfg = obj; + + virObjectUnref(cfg->caps); + libxl_ctx_free(cfg->ctx); + xtl_logger_destroy(cfg->logger); + if (cfg->logger_file) + VIR_FORCE_FCLOSE(cfg->logger_file); + + VIR_FREE(cfg->configDir); + VIR_FREE(cfg->autostartDir); + VIR_FREE(cfg->logDir); + VIR_FREE(cfg->stateDir); + VIR_FREE(cfg->libDir); + VIR_FREE(cfg->saveDir); +} + static int libxlCapsInitHost(libxl_ctx *ctx, virCapsPtr caps) { @@ -978,8 +1013,8 @@ error: return -1; } -bool -libxlGetAutoballoonConf(libxlDriverPrivatePtr driver) +static bool +libxlGetAutoballoonConf(libxlDriverConfigPtr cfg) { regex_t regex; int ret; @@ -990,11 +1025,94 @@ libxlGetAutoballoonConf(libxlDriverPrivatePtr driver) if (ret) return true; - ret = regexec(&regex, driver->verInfo->commandline, 0, NULL, 0); + ret = regexec(&regex, cfg->verInfo->commandline, 0, NULL, 0); regfree(&regex); return ret == REG_NOMATCH; } +libxlDriverConfigPtr +libxlDriverConfigNew(void) +{ + libxlDriverConfigPtr cfg; + char *log_file = NULL; + char ebuf[1024]; + unsigned int free_mem; + + if (libxlConfigInitialize() < 0) + return NULL; + + if (!(cfg = virObjectNew(libxlDriverConfigClass))) + return NULL; + + if (VIR_STRDUP(cfg->configDir, LIBXL_CONFIG_DIR) < 0) + goto error; + if (VIR_STRDUP(cfg->autostartDir, LIBXL_AUTOSTART_DIR) < 0) + goto error; + if (VIR_STRDUP(cfg->logDir, LIBXL_LOG_DIR) < 0) + goto error; + if (VIR_STRDUP(cfg->stateDir, LIBXL_STATE_DIR) < 0) + goto error; + if (VIR_STRDUP(cfg->libDir, LIBXL_LIB_DIR) < 0) + goto error; + if (VIR_STRDUP(cfg->saveDir, LIBXL_SAVE_DIR) < 0) + goto error; + + if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) < 0) + goto error; + + if ((cfg->logger_file = fopen(log_file, "a")) == NULL) { + VIR_ERROR(_("Failed to create log file '%s': %s"), + log_file, virStrerror(errno, ebuf, sizeof(ebuf))); + goto error; + } + VIR_FREE(log_file); + + cfg->logger = + (xentoollog_logger *)xtl_createlogger_stdiostream(cfg->logger_file, + XTL_DEBUG, 0); + if (!cfg->logger) { + VIR_ERROR(_("cannot create logger for libxenlight, disabling driver")); + goto error; + } + + if (libxl_ctx_alloc(&cfg->ctx, LIBXL_VERSION, 0, cfg->logger)) { + VIR_ERROR(_("cannot initialize libxenlight context, probably not " + "running in a Xen Dom0, disabling driver")); + goto error; + } + + if ((cfg->verInfo = libxl_get_version_info(cfg->ctx)) == NULL) { + VIR_ERROR(_("cannot version information from libxenlight, " + "disabling driver")); + goto error; + } + cfg->version = (cfg->verInfo->xen_version_major * 1000000) + + (cfg->verInfo->xen_version_minor * 1000); + + /* This will fill xenstore info about free and dom0 memory if missing, + * should be called before starting first domain */ + if (libxl_get_free_memory(cfg->ctx, &free_mem)) { + VIR_ERROR(_("Unable to configure libxl's memory management parameters")); + goto error; + } + + /* setup autoballoon */ + cfg->autoballoon = libxlGetAutoballoonConf(cfg); + + return cfg; + +error: + VIR_FREE(log_file); + virObjectUnref(cfg); + return NULL; +} + +libxlDriverConfigPtr +libxlDriverConfigGet(libxlDriverPrivatePtr driver) +{ + return virObjectRef(driver->config); +} + virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx) { diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index be3a473..e3875ba 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -51,10 +51,13 @@ typedef struct _libxlDriverPrivate libxlDriverPrivate; typedef libxlDriverPrivate *libxlDriverPrivatePtr; -struct _libxlDriverPrivate { - virMutex lock; - virCapsPtr caps; - virDomainXMLOptionPtr xmlopt; + +typedef struct _libxlDriverConfig libxlDriverConfig; +typedef libxlDriverConfig *libxlDriverConfigPtr; + +struct _libxlDriverConfig { + virObject parent; + const libxl_version_info *verInfo; unsigned int version; @@ -64,27 +67,43 @@ struct _libxlDriverPrivate { /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */ libxl_ctx *ctx; - virPortAllocatorPtr reservedVNCPorts; - /* Controls automatic ballooning of domain0. If true, attempt to get * memory for new domains from domain0. */ bool autoballoon; + /* Once created, caps are immutable */ + virCapsPtr caps; + + char *configDir; + char *autostartDir; + char *logDir; + char *stateDir; + char *libDir; + char *saveDir; +}; + + +struct _libxlDriverPrivate { + virMutex lock; + + /* Require lock to get reference on 'config', + * then lockless thereafter */ + libxlDriverConfigPtr config; + size_t nactive; + virStateInhibitCallback inhibitCallback; void *inhibitOpaque; virDomainObjListPtr domains; + virDomainXMLOptionPtr xmlopt; + virDomainEventStatePtr domainEventState; - virSysinfoDefPtr hostsysinfo; - char *configDir; - char *autostartDir; - char *logDir; - char *stateDir; - char *libDir; - char *saveDir; + virPortAllocatorPtr reservedVNCPorts; + + virSysinfoDefPtr hostsysinfo; }; typedef struct _libxlEventHookInfo libxlEventHookInfo; @@ -103,8 +122,11 @@ struct _libxlSavefileHeader { uint32_t unused[10]; }; -bool -libxlGetAutoballoonConf(libxlDriverPrivatePtr driver); +libxlDriverConfigPtr +libxlDriverConfigNew(void); + +libxlDriverConfigPtr +libxlDriverConfigGet(libxlDriverPrivatePtr driver); virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a26fbf6..e604899 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -126,35 +126,44 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) { libxl_physinfo phy_info; virArch hostarch = virArchFromHost(); + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); + int ret = -1; - if (libxl_get_physinfo(driver->ctx, &phy_info)) { + if (libxl_get_physinfo(cfg->ctx, &phy_info)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("libxl_get_physinfo_info failed")); - return -1; + goto cleanup; } if (virStrcpyStatic(info->model, virArchToString(hostarch)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("machine type %s too big for destination"), virArchToString(hostarch)); - return -1; + goto cleanup; } - info->memory = phy_info.total_pages * (driver->verInfo->pagesize / 1024); + info->memory = phy_info.total_pages * (cfg->verInfo->pagesize / 1024); info->cpus = phy_info.nr_cpus; info->nodes = phy_info.nr_nodes; info->cores = phy_info.cores_per_socket; info->threads = phy_info.threads_per_core; info->sockets = 1; info->mhz = phy_info.cpu_khz / 1000; - return 0; + + ret = 0; + +cleanup: + virObjectUnref(cfg); + return ret; } static char * libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { char *ret; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); - ignore_value(virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name)); + ignore_value(virAsprintf(&ret, "%s/%s.save", cfg->saveDir, vm->def->name)); + virObjectUnref(cfg); return ret; } @@ -168,6 +177,8 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from, virDomainDefPtr def = NULL; libxlSavefileHeader hdr; char *xml = NULL; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); + int ret = -1; if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) { virReportSystemError(-fd, @@ -207,23 +218,25 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from, goto error; } - if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, + if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, VIR_DOMAIN_XML_INACTIVE))) goto error; - VIR_FREE(xml); - *ret_def = def; *ret_hdr = hdr; - return fd; + ret = fd; + goto cleanup; error: - VIR_FREE(xml); virDomainDefFree(def); VIR_FORCE_CLOSE(fd); - return -1; + +cleanup: + VIR_FREE(xml); + virObjectUnref(cfg); + return ret;