[libvirt] [PATCH 00/10 v2] Automatically shutdown VMs on session quit
A re-post of https://www.redhat.com/archives/libvir-list/2012-October/msg01768.html Changed in this version - Fixed only review bug mentioned last time - Rebased to current GIT
From: "Daniel P. Berrange" <berrange@redhat.com> Change some legacy function names to use 'qemu' as their prefix instead of 'qemud' which was a hang over from when the QEMU driver ran inside a separate daemon Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_conf.c | 4 +- src/qemu/qemu_conf.h | 4 +- src/qemu/qemu_driver.c | 541 ++++++++++++++++++++++--------------------- src/qemu/qemu_monitor_text.c | 6 +- src/qemu/qemu_process.c | 2 +- 5 files changed, 280 insertions(+), 277 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dc4d680..ce3fa67 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -72,8 +72,8 @@ void qemuDriverUnlock(struct qemud_driver *driver) } -int qemudLoadDriverConfig(struct qemud_driver *driver, - const char *filename) { +int qemuLoadDriverConfig(struct qemud_driver *driver, + const char *filename) { virConfPtr conf; virConfValuePtr p; char *user; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 2c7f70c..4c729e4 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -176,8 +176,8 @@ struct _qemuDomainCmdlineDef { void qemuDriverLock(struct qemud_driver *driver); void qemuDriverUnlock(struct qemud_driver *driver); -int qemudLoadDriverConfig(struct qemud_driver *driver, - const char *filename); +int qemuLoadDriverConfig(struct qemud_driver *driver, + const char *filename); struct qemuDomainDiskInfo { bool removable; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d4cafcc..c526f5f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -136,14 +136,14 @@ static void processWatchdogEvent(void *data, void *opaque); -static int qemudShutdown(void); +static int qemuShutdown(void); static int qemuDomainObjStart(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, unsigned int flags); -static int qemudDomainGetMaxVcpus(virDomainPtr dom); +static int qemuDomainGetMaxVcpus(virDomainPtr dom); static void qemuDomainManagedSaveLoad(void *payload, const void *n ATTRIBUTE_UNUSED, @@ -605,12 +605,12 @@ qemuDomainFindMaxID(void *payload, /** - * qemudStartup: + * qemuStartup: * * Initialization function for the QEmu daemon */ static int -qemudStartup(int privileged) { +qemuStartup(int privileged) { char *base = NULL; char *driverConf = NULL; int rc; @@ -763,7 +763,7 @@ qemudStartup(int privileged) { virStrerror(-rc, ebuf, sizeof(ebuf))); } - if (qemudLoadDriverConfig(qemu_driver, driverConf) < 0) { + if (qemuLoadDriverConfig(qemu_driver, driverConf) < 0) { goto error; } VIR_FREE(driverConf); @@ -931,11 +931,11 @@ error: VIR_FREE(driverConf); VIR_FREE(membase); VIR_FREE(mempath); - qemudShutdown(); + qemuShutdown(); return -1; } -static void qemudNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) +static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) { struct qemud_driver *driver = opaque; @@ -950,13 +950,13 @@ static void qemudNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) } /** - * qemudReload: + * qemuReload: * * Function to restart the QEmu daemon, it will recheck the configuration * files and update its state and the networking */ static int -qemudReload(void) { +qemuReload(void) { if (!qemu_driver) return 0; @@ -966,14 +966,14 @@ qemudReload(void) { qemu_driver->configDir, qemu_driver->autostartDir, 0, QEMU_EXPECTED_VIRT_TYPES, - qemudNotifyLoadDomain, qemu_driver); + qemuNotifyLoadDomain, qemu_driver); qemuDriverUnlock(qemu_driver); return 0; } /** - * qemudActive: + * qemuActive: * * Checks if the QEmu daemon is active, i.e. has an active domain or * an active network @@ -981,7 +981,7 @@ qemudReload(void) { * Returns 1 if active, 0 otherwise */ static int -qemudActive(void) { +qemuActive(void) { int active = 0; if (!qemu_driver) @@ -995,12 +995,12 @@ qemudActive(void) { } /** - * qemudShutdown: + * qemuShutdown: * * Shutdown the QEmu daemon, it will stop all active domains and networks */ static int -qemudShutdown(void) { +qemuShutdown(void) { int i; if (!qemu_driver) @@ -1069,9 +1069,9 @@ qemudShutdown(void) { } -static virDrvOpenStatus qemudOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, - unsigned int flags) +static virDrvOpenStatus qemuOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -1130,7 +1130,7 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; } -static int qemudClose(virConnectPtr conn) { +static int qemuClose(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; /* Get rid of callbacks registered for this conn */ @@ -1145,7 +1145,7 @@ static int qemudClose(virConnectPtr conn) { /* Which features are supported by this driver? */ static int -qemudSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) +qemuSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) { switch (feature) { case VIR_DRV_FEATURE_MIGRATION_V2: @@ -1161,7 +1161,7 @@ qemudSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) } } -static const char *qemudGetType(virConnectPtr conn ATTRIBUTE_UNUSED) { +static const char *qemuGetType(virConnectPtr conn ATTRIBUTE_UNUSED) { return "QEMU"; } @@ -1227,7 +1227,7 @@ qemuGetSysinfo(virConnectPtr conn, unsigned int flags) return virBufferContentAndReset(&buf); } -static int qemudGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type) { +static int qemuGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type) { if (!type) return 16; @@ -1246,7 +1246,7 @@ static int qemudGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED, const char *typ } -static char *qemudGetCapabilities(virConnectPtr conn) { +static char *qemuGetCapabilities(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; virCapsPtr caps = NULL; char *xml = NULL; @@ -1272,8 +1272,8 @@ cleanup: static int -qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, - pid_t pid, int tid) +qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, + pid_t pid, int tid) { char *proc; FILE *pidinfo; @@ -1347,8 +1347,8 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, } -static virDomainPtr qemudDomainLookupByID(virConnectPtr conn, - int id) { +static virDomainPtr qemuDomainLookupByID(virConnectPtr conn, + int id) { struct qemud_driver *driver = conn->privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1372,8 +1372,8 @@ cleanup: return dom; } -static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { +static virDomainPtr qemuDomainLookupByUUID(virConnectPtr conn, + const unsigned char *uuid) { struct qemud_driver *driver = conn->privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1399,8 +1399,8 @@ cleanup: return dom; } -static virDomainPtr qemudDomainLookupByName(virConnectPtr conn, - const char *name) { +static virDomainPtr qemuDomainLookupByName(virConnectPtr conn, + const char *name) { struct qemud_driver *driver = conn->privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1497,7 +1497,7 @@ cleanup: return ret; } -static int qemudGetVersion(virConnectPtr conn, unsigned long *version) { +static int qemuGetVersion(virConnectPtr conn, unsigned long *version) { struct qemud_driver *driver = conn->privateData; int ret = -1; @@ -1515,7 +1515,7 @@ cleanup: return ret; } -static int qemudListDomains(virConnectPtr conn, int *ids, int nids) { +static int qemuListDomains(virConnectPtr conn, int *ids, int nids) { struct qemud_driver *driver = conn->privateData; int n; @@ -1526,7 +1526,7 @@ static int qemudListDomains(virConnectPtr conn, int *ids, int nids) { return n; } -static int qemudNumDomains(virConnectPtr conn) { +static int qemuNumDomains(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; int n; @@ -1560,8 +1560,8 @@ qemuCanonicalizeMachine(virDomainDefPtr def, qemuCapsPtr caps) } -static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, - unsigned int flags) { +static virDomainPtr qemuDomainCreate(virConnectPtr conn, const char *xml, + unsigned int flags) { struct qemud_driver *driver = conn->privateData; virDomainDefPtr def; virDomainObjPtr vm = NULL; @@ -1657,7 +1657,7 @@ cleanup: } -static int qemudDomainSuspend(virDomainPtr dom) { +static int qemuDomainSuspend(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; @@ -1740,7 +1740,7 @@ cleanup: } -static int qemudDomainResume(virDomainPtr dom) { +static int qemuDomainResume(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; @@ -2126,7 +2126,7 @@ qemuDomainDestroy(virDomainPtr dom) return qemuDomainDestroyFlags(dom, 0); } -static char *qemudDomainGetOSType(virDomainPtr dom) { +static char *qemuDomainGetOSType(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; char *type = NULL; @@ -2179,8 +2179,8 @@ cleanup: return ret; } -static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, - unsigned int flags) { +static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; qemuDomainObjPrivatePtr priv; virDomainObjPtr vm; @@ -2276,14 +2276,14 @@ cleanup: return ret; } -static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) +static int qemuDomainSetMemory(virDomainPtr dom, unsigned long newmem) { - return qemudDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE); + return qemuDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE); } -static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) +static int qemuDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) { - return qemudDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM); + return qemuDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM); } static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) @@ -2408,8 +2408,8 @@ cleanup: return ret; } -static int qemudDomainGetInfo(virDomainPtr dom, - virDomainInfoPtr info) +static int qemuDomainGetInfo(virDomainPtr dom, + virDomainInfoPtr info) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -2433,7 +2433,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, if (!virDomainObjIsActive(vm)) { info->cpuTime = 0; } else { - if (qemudGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) { + if (qemuGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read cputime for domain")); goto cleanup; @@ -2587,39 +2587,42 @@ cleanup: } -#define QEMUD_SAVE_MAGIC "LibvirtQemudSave" -#define QEMUD_SAVE_PARTIAL "LibvirtQemudPart" -#define QEMUD_SAVE_VERSION 2 +/* It would be nice to replace 'Qemud' with 'Qemu' but + * this magic string is ABI, so it can't be changed + */ +#define QEMU_SAVE_MAGIC "LibvirtQemudSave" +#define QEMU_SAVE_PARTIAL "LibvirtQemudPart" +#define QEMU_SAVE_VERSION 2 -verify(sizeof(QEMUD_SAVE_MAGIC) == sizeof(QEMUD_SAVE_PARTIAL)); +verify(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL)); -enum qemud_save_formats { - QEMUD_SAVE_FORMAT_RAW = 0, - QEMUD_SAVE_FORMAT_GZIP = 1, - QEMUD_SAVE_FORMAT_BZIP2 = 2, +enum qemu_save_formats { + QEMU_SAVE_FORMAT_RAW = 0, + QEMU_SAVE_FORMAT_GZIP = 1, + QEMU_SAVE_FORMAT_BZIP2 = 2, /* * Deprecated by xz and never used as part of a release - * QEMUD_SAVE_FORMAT_LZMA + * QEMU_SAVE_FORMAT_LZMA */ - QEMUD_SAVE_FORMAT_XZ = 3, - QEMUD_SAVE_FORMAT_LZOP = 4, + QEMU_SAVE_FORMAT_XZ = 3, + QEMU_SAVE_FORMAT_LZOP = 4, /* Note: add new members only at the end. These values are used in the on-disk format. Do not change or re-use numbers. */ - QEMUD_SAVE_FORMAT_LAST + QEMU_SAVE_FORMAT_LAST }; -VIR_ENUM_DECL(qemudSaveCompression) -VIR_ENUM_IMPL(qemudSaveCompression, QEMUD_SAVE_FORMAT_LAST, +VIR_ENUM_DECL(qemuSaveCompression) +VIR_ENUM_IMPL(qemuSaveCompression, QEMU_SAVE_FORMAT_LAST, "raw", "gzip", "bzip2", "xz", "lzop") -struct qemud_save_header { - char magic[sizeof(QEMUD_SAVE_MAGIC)-1]; +struct qemu_save_header { + char magic[sizeof(QEMU_SAVE_MAGIC)-1]; uint32_t version; uint32_t xml_len; uint32_t was_running; @@ -2628,7 +2631,7 @@ struct qemud_save_header { }; static inline void -bswap_header(struct qemud_save_header *hdr) { +bswap_header(struct qemu_save_header *hdr) { hdr->version = bswap_32(hdr->version); hdr->xml_len = bswap_32(hdr->xml_len); hdr->was_running = bswap_32(hdr->was_running); @@ -2639,7 +2642,7 @@ bswap_header(struct qemud_save_header *hdr) { /* return -errno on failure, or 0 on success */ static int qemuDomainSaveHeader(int fd, const char *path, const char *xml, - struct qemud_save_header *header) + struct qemu_save_header *header) { int ret = 0; @@ -2661,13 +2664,13 @@ endjob: return ret; } -/* Given a enum qemud_save_formats compression level, return the name +/* Given a enum qemu_save_formats compression level, return the name * of the program to run, or NULL if no program is needed. */ static const char * qemuCompressProgramName(int compress) { - return (compress == QEMUD_SAVE_FORMAT_RAW ? NULL : - qemudSaveCompressionTypeToString(compress)); + return (compress == QEMU_SAVE_FORMAT_RAW ? NULL : + qemuSaveCompressionTypeToString(compress)); } /* Internal function to properly create or open existing files, with @@ -2798,7 +2801,7 @@ qemuDomainSaveMemory(struct qemud_driver *driver, unsigned int flags, enum qemuDomainAsyncJob asyncJob) { - struct qemud_save_header header; + struct qemu_save_header header; bool bypassSecurityDriver = false; bool needUnlink = false; int ret = -1; @@ -2812,8 +2815,8 @@ qemuDomainSaveMemory(struct qemud_driver *driver, char *xml = NULL; memset(&header, 0, sizeof(header)); - memcpy(header.magic, QEMUD_SAVE_PARTIAL, sizeof(header.magic)); - header.version = QEMUD_SAVE_VERSION; + memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic)); + header.version = QEMU_SAVE_VERSION; header.was_running = was_running ? 1 : 0; header.compressed = compressed; @@ -2887,7 +2890,7 @@ qemuDomainSaveMemory(struct qemud_driver *driver, if ((fd = qemuOpenFile(driver, path, O_WRONLY, NULL, NULL)) < 0) goto cleanup; - memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); + memcpy(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)); if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) { virReportSystemError(errno, _("unable to write %s"), path); @@ -3040,14 +3043,14 @@ cleanup: } /* Returns true if a compression program is available in PATH */ -static bool qemudCompressProgramAvailable(enum qemud_save_formats compress) +static bool qemuCompressProgramAvailable(enum qemu_save_formats compress) { const char *prog; char *c; - if (compress == QEMUD_SAVE_FORMAT_RAW) + if (compress == QEMU_SAVE_FORMAT_RAW) return true; - prog = qemudSaveCompressionTypeToString(compress); + prog = qemuSaveCompressionTypeToString(compress); c = virFindFileInPath(prog); if (!c) return false; @@ -3071,16 +3074,16 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, qemuDriverLock(driver); if (driver->saveImageFormat == NULL) - compressed = QEMUD_SAVE_FORMAT_RAW; + compressed = QEMU_SAVE_FORMAT_RAW; else { - compressed = qemudSaveCompressionTypeFromString(driver->saveImageFormat); + compressed = qemuSaveCompressionTypeFromString(driver->saveImageFormat); if (compressed < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Invalid save image format specified " "in configuration file")); goto cleanup; } - if (!qemudCompressProgramAvailable(compressed)) { + if (!qemuCompressProgramAvailable(compressed)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Compression program for image format " "in configuration file isn't available")); @@ -3173,7 +3176,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) VIR_INFO("Saving state to %s", name); - compressed = QEMUD_SAVE_FORMAT_RAW; + compressed = QEMU_SAVE_FORMAT_RAW; if ((ret = qemuDomainSaveInternal(driver, dom, vm, name, compressed, NULL, flags)) == 0) vm->hasManagedSave = true; @@ -3304,7 +3307,7 @@ static int doCoreDump(struct qemud_driver *driver, virDomainObjPtr vm, const char *path, - enum qemud_save_formats compress, + enum qemu_save_formats compress, unsigned int dump_flags) { int fd = -1; @@ -3366,38 +3369,38 @@ cleanup: return ret; } -static enum qemud_save_formats +static enum qemu_save_formats getCompressionType(struct qemud_driver *driver) { - int compress = QEMUD_SAVE_FORMAT_RAW; + int compress = QEMU_SAVE_FORMAT_RAW; /* * We reuse "save" flag for "dump" here. Then, we can support the same * format in "save" and "dump". */ if (driver->dumpImageFormat) { - compress = qemudSaveCompressionTypeFromString(driver->dumpImageFormat); + compress = qemuSaveCompressionTypeFromString(driver->dumpImageFormat); /* Use "raw" as the format if the specified format is not valid, * or the compress program is not available. */ if (compress < 0) { VIR_WARN("%s", _("Invalid dump image format specified in " "configuration file, using raw")); - return QEMUD_SAVE_FORMAT_RAW; + return QEMU_SAVE_FORMAT_RAW; } - if (!qemudCompressProgramAvailable(compress)) { + if (!qemuCompressProgramAvailable(compress)) { VIR_WARN("%s", _("Compression program for dump image format " "in configuration file isn't available, " "using raw")); - return QEMUD_SAVE_FORMAT_RAW; + return QEMU_SAVE_FORMAT_RAW; } } return compress; } -static int qemudDomainCoreDump(virDomainPtr dom, - const char *path, - unsigned int flags) +static int qemuDomainCoreDump(virDomainPtr dom, + const char *path, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3672,9 +3675,9 @@ unlock: VIR_FREE(wdEvent); } -static int qemudDomainHotplugVcpus(struct qemud_driver *driver, - virDomainObjPtr vm, - unsigned int nvcpus) +static int qemuDomainHotplugVcpus(struct qemud_driver *driver, + virDomainObjPtr vm, + unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; int i, rc = 1; @@ -3923,7 +3926,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if ((max = qemudGetMaxVCPUs(NULL, type)) < 0) { + if ((max = qemuGetMaxVCPUs(NULL, type)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not determine max vcpus for the domain")); goto endjob; @@ -3941,7 +3944,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (qemudDomainHotplugVcpus(driver, vm, nvcpus) < 0) + if (qemuDomainHotplugVcpus(driver, vm, nvcpus) < 0) goto endjob; } @@ -3978,11 +3981,11 @@ qemuDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) static int -qemudDomainPinVcpuFlags(virDomainPtr dom, - unsigned int vcpu, - unsigned char *cpumap, - int maplen, - unsigned int flags) { +qemuDomainPinVcpuFlags(virDomainPtr dom, + unsigned int vcpu, + unsigned char *cpumap, + int maplen, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -4153,20 +4156,20 @@ cleanup: } static int -qemudDomainPinVcpu(virDomainPtr dom, +qemuDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, int maplen) { - return qemudDomainPinVcpuFlags(dom, vcpu, cpumap, maplen, - VIR_DOMAIN_AFFECT_LIVE); + return qemuDomainPinVcpuFlags(dom, vcpu, cpumap, maplen, + VIR_DOMAIN_AFFECT_LIVE); } static int -qemudDomainGetVcpuPinInfo(virDomainPtr dom, - int ncpumaps, - unsigned char *cpumaps, - int maplen, - unsigned int flags) { +qemuDomainGetVcpuPinInfo(virDomainPtr dom, + int ncpumaps, + unsigned char *cpumaps, + int maplen, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; @@ -4251,10 +4254,10 @@ cleanup: } static int -qemudDomainPinEmulator(virDomainPtr dom, - unsigned char *cpumap, - int maplen, - unsigned int flags) +qemuDomainPinEmulator(virDomainPtr dom, + unsigned char *cpumap, + int maplen, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -4413,10 +4416,10 @@ cleanup: } static int -qemudDomainGetEmulatorPinInfo(virDomainPtr dom, - unsigned char *cpumaps, - int maplen, - unsigned int flags) +qemuDomainGetEmulatorPinInfo(virDomainPtr dom, + unsigned char *cpumaps, + int maplen, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; @@ -4489,11 +4492,11 @@ cleanup: } static int -qemudDomainGetVcpus(virDomainPtr dom, - virVcpuInfoPtr info, - int maxinfo, - unsigned char *cpumaps, - int maplen) { +qemuDomainGetVcpus(virDomainPtr dom, + virVcpuInfoPtr info, + int maxinfo, + unsigned char *cpumaps, + int maplen) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int i, v, maxcpu, hostcpus; @@ -4540,11 +4543,11 @@ qemudDomainGetVcpus(virDomainPtr dom, info[i].state = VIR_VCPU_RUNNING; if (priv->vcpupids != NULL && - qemudGetProcessInfo(&(info[i].cpuTime), - &(info[i].cpu), - NULL, - vm->pid, - priv->vcpupids[i]) < 0) { + qemuGetProcessInfo(&(info[i].cpuTime), + &(info[i].cpu), + NULL, + vm->pid, + priv->vcpupids[i]) < 0) { virReportSystemError(errno, "%s", _("cannot get vCPU placement & pCPU time")); goto cleanup; @@ -4589,7 +4592,7 @@ cleanup: static int -qemudDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) +qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -4628,13 +4631,13 @@ cleanup: } static int -qemudDomainGetMaxVcpus(virDomainPtr dom) +qemuDomainGetMaxVcpus(virDomainPtr dom) { - return qemudDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_VCPU_MAXIMUM)); + return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_VCPU_MAXIMUM)); } -static int qemudDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) +static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; virDomainObjPtr vm; @@ -4719,7 +4722,7 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom, } /* - * Check the comment in qemudDomainGetSecurityLabel function. + * Check the comment in qemuDomainGetSecurityLabel function. */ if (!virDomainObjIsActive(vm)) { /* No seclabels */ @@ -4764,8 +4767,8 @@ cleanup: qemuDriverUnlock(driver); return ret; } -static int qemudNodeGetSecurityModel(virConnectPtr conn, - virSecurityModelPtr secmodel) +static int qemuNodeGetSecurityModel(virConnectPtr conn, + virSecurityModelPtr secmodel) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; char *p; @@ -4812,14 +4815,14 @@ static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) qemuDomainSaveImageOpen(struct qemud_driver *driver, const char *path, virDomainDefPtr *ret_def, - struct qemud_save_header *ret_header, + struct qemu_save_header *ret_header, bool bypass_cache, virFileWrapperFdPtr *wrapperFd, const char *xmlin, int state, bool edit, bool unlink_corrupt) { int fd = -1; - struct qemud_save_header header; + struct qemu_save_header header; char *xml = NULL; virDomainDefPtr def = NULL; int oflags = edit ? O_RDWR : O_RDONLY; @@ -4856,10 +4859,10 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver, goto error; } - if (memcmp(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)) != 0) { + if (memcmp(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)) != 0) { const char *msg = _("image magic is incorrect"); - if (memcmp(header.magic, QEMUD_SAVE_PARTIAL, + if (memcmp(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic)) == 0) { msg = _("save image is incomplete"); if (unlink_corrupt) { @@ -4876,15 +4879,15 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver, goto error; } - if (header.version > QEMUD_SAVE_VERSION) { + if (header.version > QEMU_SAVE_VERSION) { /* convert endianess and try again */ bswap_header(&header); } - if (header.version > QEMUD_SAVE_VERSION) { + if (header.version > QEMU_SAVE_VERSION) { virReportError(VIR_ERR_OPERATION_FAILED, _("image version is not supported (%d > %d)"), - header.version, QEMUD_SAVE_VERSION); + header.version, QEMU_SAVE_VERSION); goto error; } @@ -4957,7 +4960,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, int *fd, - const struct qemud_save_header *header, + const struct qemu_save_header *header, const char *path, bool start_paused) { @@ -4967,7 +4970,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, virCommandPtr cmd = NULL; if (header->version == 2) { - const char *prog = qemudSaveCompressionTypeToString(header->compressed); + const char *prog = qemuSaveCompressionTypeToString(header->compressed); if (prog == NULL) { virReportError(VIR_ERR_OPERATION_FAILED, _("Invalid compressed save format %d"), @@ -4975,7 +4978,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, goto out; } - if (header->compressed != QEMUD_SAVE_FORMAT_RAW) { + if (header->compressed != QEMU_SAVE_FORMAT_RAW) { cmd = virCommandNewArgList(prog, "-dc", NULL); intermediatefd = *fd; *fd = -1; @@ -5077,7 +5080,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, virDomainObjPtr vm = NULL; int fd = -1; int ret = -1; - struct qemud_save_header header; + struct qemu_save_header header; virFileWrapperFdPtr wrapperFd = NULL; int state = -1; @@ -5149,7 +5152,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, char *ret = NULL; virDomainDefPtr def = NULL; int fd = -1; - struct qemud_save_header header; + struct qemu_save_header header; /* We only take subset of virDomainDefFormat flags. */ virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); @@ -5179,7 +5182,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, int ret = -1; virDomainDefPtr def = NULL; int fd = -1; - struct qemud_save_header header; + struct qemu_save_header header; char *xml = NULL; size_t len; int state = -1; @@ -5256,7 +5259,7 @@ qemuDomainObjRestore(virConnectPtr conn, virDomainDefPtr def = NULL; int fd = -1; int ret = -1; - struct qemud_save_header header; + struct qemu_save_header header; virFileWrapperFdPtr wrapperFd = NULL; fd = qemuDomainSaveImageOpen(driver, path, &def, &header, @@ -5536,8 +5539,8 @@ cleanup: } -static int qemudListDefinedDomains(virConnectPtr conn, - char **const names, int nnames) { +static int qemuListDefinedDomains(virConnectPtr conn, + char **const names, int nnames) { struct qemud_driver *driver = conn->privateData; int n; @@ -5547,7 +5550,7 @@ static int qemudListDefinedDomains(virConnectPtr conn, return n; } -static int qemudNumDefinedDomains(virConnectPtr conn) { +static int qemuNumDefinedDomains(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; int n; @@ -5690,7 +5693,7 @@ qemuDomainStart(virDomainPtr dom) return qemuDomainStartWithFlags(dom, 0); } -static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { +static virDomainPtr qemuDomainDefine(virConnectPtr conn, const char *xml) { struct qemud_driver *driver = conn->privateData; virDomainDefPtr def; virDomainDefPtr def_backup = NULL; @@ -5889,7 +5892,7 @@ cleanup: } static int -qemudDomainUndefine(virDomainPtr dom) +qemuDomainUndefine(virDomainPtr dom) { return qemuDomainUndefineFlags(dom, 0); } @@ -6678,8 +6681,8 @@ static int qemuDomainDetachDevice(virDomainPtr dom, const char *xml) VIR_DOMAIN_AFFECT_LIVE); } -static int qemudDomainGetAutostart(virDomainPtr dom, - int *autostart) { +static int qemuDomainGetAutostart(virDomainPtr dom, + int *autostart) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; @@ -6705,8 +6708,8 @@ cleanup: return ret; } -static int qemudDomainSetAutostart(virDomainPtr dom, - int autostart) { +static int qemuDomainSetAutostart(virDomainPtr dom, + int autostart) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; char *configFile = NULL, *autostartLink = NULL; @@ -8770,9 +8773,9 @@ cleanup: #ifdef __linux__ static int -qemudDomainInterfaceStats(virDomainPtr dom, - const char *path, - struct _virDomainInterfaceStats *stats) +qemuDomainInterfaceStats(virDomainPtr dom, + const char *path, + struct _virDomainInterfaceStats *stats) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -8819,9 +8822,9 @@ cleanup: } #else static int -qemudDomainInterfaceStats(virDomainPtr dom, - const char *path ATTRIBUTE_UNUSED, - struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED) +qemuDomainInterfaceStats(virDomainPtr dom, + const char *path ATTRIBUTE_UNUSED, + struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("interface stats not implemented on this platform")); @@ -9126,10 +9129,10 @@ cleanup: } static int -qemudDomainMemoryStats(virDomainPtr dom, - struct _virDomainMemoryStat *stats, - unsigned int nr_stats, - unsigned int flags) +qemuDomainMemoryStats(virDomainPtr dom, + struct _virDomainMemoryStat *stats, + unsigned int nr_stats, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -9163,7 +9166,7 @@ qemudDomainMemoryStats(virDomainPtr dom, if (ret >= 0 && ret < nr_stats) { long rss; - if (qemudGetProcessInfo(NULL, NULL, &rss, vm->pid, 0) < 0) { + if (qemuGetProcessInfo(NULL, NULL, &rss, vm->pid, 0) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot get RSS for domain")); } else { @@ -9185,11 +9188,11 @@ cleanup: } static int -qemudDomainBlockPeek(virDomainPtr dom, - const char *path, - unsigned long long offset, size_t size, - void *buffer, - unsigned int flags) +qemuDomainBlockPeek(virDomainPtr dom, + const char *path, + unsigned long long offset, size_t size, + void *buffer, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -9253,10 +9256,10 @@ cleanup: } static int -qemudDomainMemoryPeek(virDomainPtr dom, - unsigned long long offset, size_t size, - void *buffer, - unsigned int flags) +qemuDomainMemoryPeek(virDomainPtr dom, + unsigned long long offset, size_t size, + void *buffer, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -9586,12 +9589,12 @@ qemuDomainEventDeregisterAny(virConnectPtr conn, * sets up the corresponding virStream to handle the incoming data. */ static int -qemudDomainMigratePrepareTunnel(virConnectPtr dconn, - virStreamPtr st, - unsigned long flags, - const char *dname, - unsigned long resource ATTRIBUTE_UNUSED, - const char *dom_xml) +qemuDomainMigratePrepareTunnel(virConnectPtr dconn, + virStreamPtr st, + unsigned long flags, + const char *dname, + unsigned long resource ATTRIBUTE_UNUSED, + const char *dom_xml) { struct qemud_driver *driver = dconn->privateData; int ret = -1; @@ -9637,15 +9640,15 @@ cleanup: * This starts an empty VM listening on a TCP port. */ static int ATTRIBUTE_NONNULL(5) -qemudDomainMigratePrepare2(virConnectPtr dconn, - char **cookie ATTRIBUTE_UNUSED, - int *cookielen ATTRIBUTE_UNUSED, - const char *uri_in, - char **uri_out, - unsigned long flags, - const char *dname, - unsigned long resource ATTRIBUTE_UNUSED, - const char *dom_xml) +qemuDomainMigratePrepare2(virConnectPtr dconn, + char **cookie ATTRIBUTE_UNUSED, + int *cookielen ATTRIBUTE_UNUSED, + const char *uri_in, + char **uri_out, + unsigned long flags, + const char *dname, + unsigned long resource ATTRIBUTE_UNUSED, + const char *dom_xml) { struct qemud_driver *driver = dconn->privateData; int ret = -1; @@ -9695,13 +9698,13 @@ cleanup: /* Perform is the second step, and it runs on the source host. */ static int -qemudDomainMigratePerform(virDomainPtr dom, - const char *cookie, - int cookielen, - const char *uri, - unsigned long flags, - const char *dname, - unsigned long resource) +qemuDomainMigratePerform(virDomainPtr dom, + const char *cookie, + int cookielen, + const char *uri, + unsigned long flags, + const char *dname, + unsigned long resource) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -9751,13 +9754,13 @@ cleanup: /* Finish is the third and final step, and it runs on the destination host. */ static virDomainPtr -qemudDomainMigrateFinish2(virConnectPtr dconn, - const char *dname, - const char *cookie ATTRIBUTE_UNUSED, - int cookielen ATTRIBUTE_UNUSED, - const char *uri ATTRIBUTE_UNUSED, - unsigned long flags, - int retcode) +qemuDomainMigrateFinish2(virConnectPtr dconn, + const char *dname, + const char *cookie ATTRIBUTE_UNUSED, + int cookielen ATTRIBUTE_UNUSED, + const char *uri ATTRIBUTE_UNUSED, + unsigned long flags, + int retcode) { struct qemud_driver *driver = dconn->privateData; virDomainObjPtr vm; @@ -10109,11 +10112,11 @@ cleanup: static int -qemudNodeDeviceGetPciInfo(virNodeDevicePtr dev, - unsigned *domain, - unsigned *bus, - unsigned *slot, - unsigned *function) +qemuNodeDeviceGetPciInfo(virNodeDevicePtr dev, + unsigned *domain, + unsigned *bus, + unsigned *slot, + unsigned *function) { virNodeDeviceDefPtr def = NULL; virNodeDevCapsDefPtr cap; @@ -10155,7 +10158,7 @@ out: } static int -qemudNodeDeviceDettach(virNodeDevicePtr dev) +qemuNodeDeviceDettach(virNodeDevicePtr dev) { struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; @@ -10163,7 +10166,7 @@ qemudNodeDeviceDettach(virNodeDevicePtr dev) int ret = -1; bool in_inactive_list = false; - if (qemudNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) + if (qemuNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) return -1; pci = pciGetDevice(domain, bus, slot, function); @@ -10186,7 +10189,7 @@ out: } static int -qemudNodeDeviceReAttach(virNodeDevicePtr dev) +qemuNodeDeviceReAttach(virNodeDevicePtr dev) { struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; @@ -10194,7 +10197,7 @@ qemudNodeDeviceReAttach(virNodeDevicePtr dev) unsigned domain, bus, slot, function; int ret = -1; - if (qemudNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) + if (qemuNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) return -1; pci = pciGetDevice(domain, bus, slot, function); @@ -10230,14 +10233,14 @@ out: } static int -qemudNodeDeviceReset(virNodeDevicePtr dev) +qemuNodeDeviceReset(virNodeDevicePtr dev) { struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; unsigned domain, bus, slot, function; int ret = -1; - if (qemudNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) + if (qemuNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) return -1; pci = pciGetDevice(domain, bus, slot, function); @@ -11359,7 +11362,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, goto endjob; if ((ret = qemuDomainSaveMemory(driver, vm, snap->def->file, - xml, QEMUD_SAVE_FORMAT_RAW, + xml, QEMU_SAVE_FORMAT_RAW, resume, 0, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto endjob; @@ -14726,41 +14729,41 @@ cleanup: static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, - .open = qemudOpen, /* 0.2.0 */ - .close = qemudClose, /* 0.2.0 */ - .supports_feature = qemudSupportsFeature, /* 0.5.0 */ - .type = qemudGetType, /* 0.2.0 */ - .version = qemudGetVersion, /* 0.2.0 */ + .open = qemuOpen, /* 0.2.0 */ + .close = qemuClose, /* 0.2.0 */ + .supports_feature = qemuSupportsFeature, /* 0.5.0 */ + .type = qemuGetType, /* 0.2.0 */ + .version = qemuGetVersion, /* 0.2.0 */ .getHostname = virGetHostname, /* 0.3.3 */ .getSysinfo = qemuGetSysinfo, /* 0.8.8 */ - .getMaxVcpus = qemudGetMaxVCPUs, /* 0.2.1 */ + .getMaxVcpus = qemuGetMaxVCPUs, /* 0.2.1 */ .nodeGetInfo = nodeGetInfo, /* 0.2.0 */ - .getCapabilities = qemudGetCapabilities, /* 0.2.1 */ - .listDomains = qemudListDomains, /* 0.2.0 */ - .numOfDomains = qemudNumDomains, /* 0.2.0 */ + .getCapabilities = qemuGetCapabilities, /* 0.2.1 */ + .listDomains = qemuListDomains, /* 0.2.0 */ + .numOfDomains = qemuNumDomains, /* 0.2.0 */ .listAllDomains = qemuListAllDomains, /* 0.9.13 */ - .domainCreateXML = qemudDomainCreate, /* 0.2.0 */ - .domainLookupByID = qemudDomainLookupByID, /* 0.2.0 */ - .domainLookupByUUID = qemudDomainLookupByUUID, /* 0.2.0 */ - .domainLookupByName = qemudDomainLookupByName, /* 0.2.0 */ - .domainSuspend = qemudDomainSuspend, /* 0.2.0 */ - .domainResume = qemudDomainResume, /* 0.2.0 */ + .domainCreateXML = qemuDomainCreate, /* 0.2.0 */ + .domainLookupByID = qemuDomainLookupByID, /* 0.2.0 */ + .domainLookupByUUID = qemuDomainLookupByUUID, /* 0.2.0 */ + .domainLookupByName = qemuDomainLookupByName, /* 0.2.0 */ + .domainSuspend = qemuDomainSuspend, /* 0.2.0 */ + .domainResume = qemuDomainResume, /* 0.2.0 */ .domainShutdown = qemuDomainShutdown, /* 0.2.0 */ .domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.10 */ .domainReboot = qemuDomainReboot, /* 0.9.3 */ .domainReset = qemuDomainReset, /* 0.9.7 */ .domainDestroy = qemuDomainDestroy, /* 0.2.0 */ .domainDestroyFlags = qemuDomainDestroyFlags, /* 0.9.4 */ - .domainGetOSType = qemudDomainGetOSType, /* 0.2.2 */ + .domainGetOSType = qemuDomainGetOSType, /* 0.2.2 */ .domainGetMaxMemory = qemuDomainGetMaxMemory, /* 0.4.2 */ - .domainSetMaxMemory = qemudDomainSetMaxMemory, /* 0.4.2 */ - .domainSetMemory = qemudDomainSetMemory, /* 0.4.2 */ - .domainSetMemoryFlags = qemudDomainSetMemoryFlags, /* 0.9.0 */ + .domainSetMaxMemory = qemuDomainSetMaxMemory, /* 0.4.2 */ + .domainSetMemory = qemuDomainSetMemory, /* 0.4.2 */ + .domainSetMemoryFlags = qemuDomainSetMemoryFlags, /* 0.9.0 */ .domainSetMemoryParameters = qemuDomainSetMemoryParameters, /* 0.8.5 */ .domainGetMemoryParameters = qemuDomainGetMemoryParameters, /* 0.8.5 */ .domainSetBlkioParameters = qemuDomainSetBlkioParameters, /* 0.9.0 */ .domainGetBlkioParameters = qemuDomainGetBlkioParameters, /* 0.9.0 */ - .domainGetInfo = qemudDomainGetInfo, /* 0.2.0 */ + .domainGetInfo = qemuDomainGetInfo, /* 0.2.0 */ .domainGetState = qemuDomainGetState, /* 0.9.2 */ .domainGetControlInfo = qemuDomainGetControlInfo, /* 0.9.3 */ .domainSave = qemuDomainSave, /* 0.2.0 */ @@ -14769,51 +14772,51 @@ static virDriver qemuDriver = { .domainRestoreFlags = qemuDomainRestoreFlags, /* 0.9.4 */ .domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = qemuDomainSaveImageDefineXML, /* 0.9.4 */ - .domainCoreDump = qemudDomainCoreDump, /* 0.7.0 */ + .domainCoreDump = qemuDomainCoreDump, /* 0.7.0 */ .domainScreenshot = qemuDomainScreenshot, /* 0.9.2 */ .domainSetVcpus = qemuDomainSetVcpus, /* 0.4.4 */ .domainSetVcpusFlags = qemuDomainSetVcpusFlags, /* 0.8.5 */ - .domainGetVcpusFlags = qemudDomainGetVcpusFlags, /* 0.8.5 */ - .domainPinVcpu = qemudDomainPinVcpu, /* 0.4.4 */ - .domainPinVcpuFlags = qemudDomainPinVcpuFlags, /* 0.9.3 */ - .domainGetVcpuPinInfo = qemudDomainGetVcpuPinInfo, /* 0.9.3 */ - .domainPinEmulator = qemudDomainPinEmulator, /* 0.10.0 */ - .domainGetEmulatorPinInfo = qemudDomainGetEmulatorPinInfo, /* 0.10.0 */ - .domainGetVcpus = qemudDomainGetVcpus, /* 0.4.4 */ - .domainGetMaxVcpus = qemudDomainGetMaxVcpus, /* 0.4.4 */ - .domainGetSecurityLabel = qemudDomainGetSecurityLabel, /* 0.6.1 */ + .domainGetVcpusFlags = qemuDomainGetVcpusFlags, /* 0.8.5 */ + .domainPinVcpu = qemuDomainPinVcpu, /* 0.4.4 */ + .domainPinVcpuFlags = qemuDomainPinVcpuFlags, /* 0.9.3 */ + .domainGetVcpuPinInfo = qemuDomainGetVcpuPinInfo, /* 0.9.3 */ + .domainPinEmulator = qemuDomainPinEmulator, /* 0.10.0 */ + .domainGetEmulatorPinInfo = qemuDomainGetEmulatorPinInfo, /* 0.10.0 */ + .domainGetVcpus = qemuDomainGetVcpus, /* 0.4.4 */ + .domainGetMaxVcpus = qemuDomainGetMaxVcpus, /* 0.4.4 */ + .domainGetSecurityLabel = qemuDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ - .nodeGetSecurityModel = qemudNodeGetSecurityModel, /* 0.6.1 */ + .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */ .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */ .domainXMLFromNative = qemuDomainXMLFromNative, /* 0.6.4 */ .domainXMLToNative = qemuDomainXMLToNative, /* 0.6.4 */ - .listDefinedDomains = qemudListDefinedDomains, /* 0.2.0 */ - .numOfDefinedDomains = qemudNumDefinedDomains, /* 0.2.0 */ + .listDefinedDomains = qemuListDefinedDomains, /* 0.2.0 */ + .numOfDefinedDomains = qemuNumDefinedDomains, /* 0.2.0 */ .domainCreate = qemuDomainStart, /* 0.2.0 */ .domainCreateWithFlags = qemuDomainStartWithFlags, /* 0.8.2 */ - .domainDefineXML = qemudDomainDefine, /* 0.2.0 */ - .domainUndefine = qemudDomainUndefine, /* 0.2.0 */ + .domainDefineXML = qemuDomainDefine, /* 0.2.0 */ + .domainUndefine = qemuDomainUndefine, /* 0.2.0 */ .domainUndefineFlags = qemuDomainUndefineFlags, /* 0.9.4 */ .domainAttachDevice = qemuDomainAttachDevice, /* 0.4.1 */ .domainAttachDeviceFlags = qemuDomainAttachDeviceFlags, /* 0.7.7 */ .domainDetachDevice = qemuDomainDetachDevice, /* 0.5.0 */ .domainDetachDeviceFlags = qemuDomainDetachDeviceFlags, /* 0.7.7 */ .domainUpdateDeviceFlags = qemuDomainUpdateDeviceFlags, /* 0.8.0 */ - .domainGetAutostart = qemudDomainGetAutostart, /* 0.2.1 */ - .domainSetAutostart = qemudDomainSetAutostart, /* 0.2.1 */ + .domainGetAutostart = qemuDomainGetAutostart, /* 0.2.1 */ + .domainSetAutostart = qemuDomainSetAutostart, /* 0.2.1 */ .domainGetSchedulerType = qemuGetSchedulerType, /* 0.7.0 */ .domainGetSchedulerParameters = qemuGetSchedulerParameters, /* 0.7.0 */ .domainGetSchedulerParametersFlags = qemuGetSchedulerParametersFlags, /* 0.9.2 */ .domainSetSchedulerParameters = qemuSetSchedulerParameters, /* 0.7.0 */ .domainSetSchedulerParametersFlags = qemuSetSchedulerParametersFlags, /* 0.9.2 */ - .domainMigratePerform = qemudDomainMigratePerform, /* 0.5.0 */ + .domainMigratePerform = qemuDomainMigratePerform, /* 0.5.0 */ .domainBlockResize = qemuDomainBlockResize, /* 0.9.8 */ .domainBlockStats = qemuDomainBlockStats, /* 0.4.1 */ .domainBlockStatsFlags = qemuDomainBlockStatsFlags, /* 0.9.5 */ - .domainInterfaceStats = qemudDomainInterfaceStats, /* 0.4.1 */ - .domainMemoryStats = qemudDomainMemoryStats, /* 0.7.5 */ - .domainBlockPeek = qemudDomainBlockPeek, /* 0.4.4 */ - .domainMemoryPeek = qemudDomainMemoryPeek, /* 0.4.4 */ + .domainInterfaceStats = qemuDomainInterfaceStats, /* 0.4.1 */ + .domainMemoryStats = qemuDomainMemoryStats, /* 0.7.5 */ + .domainBlockPeek = qemuDomainBlockPeek, /* 0.4.4 */ + .domainMemoryPeek = qemuDomainMemoryPeek, /* 0.4.4 */ .domainGetBlockInfo = qemuDomainGetBlockInfo, /* 0.8.1 */ .nodeGetCPUStats = nodeGetCPUStats, /* 0.9.3 */ .nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.3 */ @@ -14821,12 +14824,12 @@ static virDriver qemuDriver = { .nodeGetFreeMemory = nodeGetFreeMemory, /* 0.4.4 */ .domainEventRegister = qemuDomainEventRegister, /* 0.5.0 */ .domainEventDeregister = qemuDomainEventDeregister, /* 0.5.0 */ - .domainMigratePrepare2 = qemudDomainMigratePrepare2, /* 0.5.0 */ - .domainMigrateFinish2 = qemudDomainMigrateFinish2, /* 0.5.0 */ - .nodeDeviceDettach = qemudNodeDeviceDettach, /* 0.6.1 */ - .nodeDeviceReAttach = qemudNodeDeviceReAttach, /* 0.6.1 */ - .nodeDeviceReset = qemudNodeDeviceReset, /* 0.6.1 */ - .domainMigratePrepareTunnel = qemudDomainMigratePrepareTunnel, /* 0.7.2 */ + .domainMigratePrepare2 = qemuDomainMigratePrepare2, /* 0.5.0 */ + .domainMigrateFinish2 = qemuDomainMigrateFinish2, /* 0.5.0 */ + .nodeDeviceDettach = qemuNodeDeviceDettach, /* 0.6.1 */ + .nodeDeviceReAttach = qemuNodeDeviceReAttach, /* 0.6.1 */ + .nodeDeviceReset = qemuNodeDeviceReset, /* 0.6.1 */ + .domainMigratePrepareTunnel = qemuDomainMigratePrepareTunnel, /* 0.7.2 */ .isEncrypted = qemuIsEncrypted, /* 0.7.3 */ .isSecure = qemuIsSecure, /* 0.7.3 */ .domainIsActive = qemuDomainIsActive, /* 0.7.3 */ @@ -14901,10 +14904,10 @@ static virDriver qemuDriver = { static virStateDriver qemuStateDriver = { .name = "QEMU", - .initialize = qemudStartup, - .cleanup = qemudShutdown, - .reload = qemudReload, - .active = qemudActive, + .initialize = qemuStartup, + .cleanup = qemuShutdown, + .reload = qemuReload, + .active = qemuActive, }; int qemuRegister(void) { diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 9af5cd3..43e5449 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2367,8 +2367,8 @@ cleanup: static int -qemudParseDriveAddReply(const char *reply, - virDomainDeviceDriveAddressPtr addr) +qemuParseDriveAddReply(const char *reply, + virDomainDeviceDriveAddressPtr addr) { char *s, *e; @@ -2446,7 +2446,7 @@ try_command: goto cleanup; } - if (qemudParseDriveAddReply(reply, driveAddr) < 0) { + if (qemuParseDriveAddReply(reply, driveAddr) < 0) { if (!tryOldSyntax && strstr(reply, "invalid char in expression")) { VIR_FREE(reply); VIR_FREE(cmd); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8743c60..cdaa2df 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -73,7 +73,7 @@ #define SHUTDOWN_POSTFIX ": shutting down\n" /** - * qemudRemoveDomainStatus + * qemuProcessRemoveDomainStatus * * remove all state files of a domain from statedir * -- 1.7.11.7
Change some legacy function names to use 'qemu' as their prefix instead of 'qemud' which was a hang over from when the QEMU driver ran inside a separate daemon
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_conf.c | 4 +- src/qemu/qemu_conf.h | 4 +- src/qemu/qemu_driver.c | 541 ++++++++++++++++++++++--------------------- src/qemu/qemu_monitor_text.c | 6 +- src/qemu/qemu_process.c | 2 +- 5 files changed, 280 insertions(+), 277 deletions(-)
Fairly mechanical. I'll see if I can spot why there are more lines added than deleted...
-static void qemudNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) +static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
[Yuck - I'll be glad to get off of this brain-dead web interface, as it sure mangles patch reviews] Is it worth a followup patch for lines like this to use our more common convention of: static void qemuNotifyLoadDomain(...) But that doesn't impact this patch.
@@ -1130,7 +1130,7 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; }
-static int qemudClose(virConnectPtr conn) { +static int qemuClose(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData;
As long as we are refactoring code... Another one bugging me is that we use 'struct qemud_driver *' everywhere instead of a nicer typedef'd 'qemuDriverPtr'; but that would also be best done as a separate patch.
-static int qemudGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type) { +static int qemuGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type) {
'{' should be on its own line (saving that for a separate cleanup patch would be okay though).
if (!type) return 16;
@@ -1246,7 +1246,7 @@ static int qemudGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED, const char *typ }
-static char *qemudGetCapabilities(virConnectPtr conn) { +static char *qemuGetCapabilities(virConnectPtr conn) {
And again.
@@ -2587,39 +2587,42 @@ cleanup: }
-#define QEMUD_SAVE_MAGIC "LibvirtQemudSave" -#define QEMUD_SAVE_PARTIAL "LibvirtQemudPart" -#define QEMUD_SAVE_VERSION 2 +/* It would be nice to replace 'Qemud' with 'Qemu' but + * this magic string is ABI, so it can't be changed + */ +#define QEMU_SAVE_MAGIC "LibvirtQemudSave" +#define QEMU_SAVE_PARTIAL "LibvirtQemudPart" +#define QEMU_SAVE_VERSION 2
Aha - there's the three added lines. And I concur; 'virsh restore' would break if we altered this string.
-enum qemud_save_formats { - QEMUD_SAVE_FORMAT_RAW = 0, - QEMUD_SAVE_FORMAT_GZIP = 1, - QEMUD_SAVE_FORMAT_BZIP2 = 2, +enum qemu_save_formats {
While touching this, should we name it qemuSaveFormats? (That is, either here or in a followup, shouldn't enum types be camel-cased?)
+ QEMU_SAVE_FORMAT_RAW = 0, + QEMU_SAVE_FORMAT_GZIP = 1, + QEMU_SAVE_FORMAT_BZIP2 = 2, /* * Deprecated by xz and never used as part of a release - * QEMUD_SAVE_FORMAT_LZMA + * QEMU_SAVE_FORMAT_LZMA */
At this point, we could delete this comment altogether.
@@ -2628,7 +2631,7 @@ struct qemud_save_header { };
static inline void -bswap_header(struct qemud_save_header *hdr) { +bswap_header(struct qemu_save_header *hdr) {
Should we be renaming this helper function into qemuBswapHeader?
hdr->version = bswap_32(hdr->version); hdr->xml_len = bswap_32(hdr->xml_len); hdr->was_running = bswap_32(hdr->was_running); @@ -2639,7 +2642,7 @@ bswap_header(struct qemud_save_header *hdr) { /* return -errno on failure, or 0 on success */ static int qemuDomainSaveHeader(int fd, const char *path, const char *xml, - struct qemud_save_header *header) + struct qemu_save_header *header)
Another annoying struct name, where a typedef'd qemuSaveHeaderPtr would be nicer. But again, save it for a followup patch. ACK. This will cause conflicts to any outstanding patches or to backports of patches made after this point, but I think we can live with that; besides, we seem to be past the worst of the backport churn (observent list readers can probably pinpoint cycles of major backport efforts vs. major feature development).
On Tue, Nov 27, 2012 at 01:38:44PM -0500, Eric Blake wrote:
Change some legacy function names to use 'qemu' as their prefix instead of 'qemud' which was a hang over from when the QEMU driver ran inside a separate daemon
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_conf.c | 4 +- src/qemu/qemu_conf.h | 4 +- src/qemu/qemu_driver.c | 541 ++++++++++++++++++++++--------------------- src/qemu/qemu_monitor_text.c | 6 +- src/qemu/qemu_process.c | 2 +- 5 files changed, 280 insertions(+), 277 deletions(-)
Fairly mechanical. I'll see if I can spot why there are more lines added than deleted...
-static void qemudNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) +static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
[Yuck - I'll be glad to get off of this brain-dead web interface, as it sure mangles patch reviews]
Is it worth a followup patch for lines like this to use our more common convention of:
static void qemuNotifyLoadDomain(...)
But that doesn't impact this patch.
I'll look at doing that & the other similar things you mention in another patch, so that this remains just a rename rather than re-style.
@@ -1130,7 +1130,7 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; }
-static int qemudClose(virConnectPtr conn) { +static int qemuClose(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData;
As long as we are refactoring code... Another one bugging me is that we use 'struct qemud_driver *' everywhere instead of a nicer typedef'd 'qemuDriverPtr'; but that would also be best done as a separate patch.
Oh yes, I'm just itching to introduce a 'virQEMUDriverPtr' type and may well do it real soon.
ACK. This will cause conflicts to any outstanding patches or to backports of patches made after this point, but I think we can live with that; besides, we seem to be past the worst of the backport churn (observent list readers can probably pinpoint cycles of major backport efforts vs. major feature development).
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: Alexander Larsson <alexl@redhat.com> This splits out some common code from virDBusGetSystemBus and uses it to implement a new virDBusGetSessionBus helper. --- src/libvirt_private.syms | 1 + src/util/virdbus.c | 84 ++++++++++++++++++++++++++++++++++++------------ src/util/virdbus.h | 1 + 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 63e187a..8d649bf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1336,6 +1336,7 @@ virConsoleOpen; # virdbus.h virDBusGetSystemBus; +virDBusGetSessionBus; # virfile.h diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 4acce12..2a96778 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -32,40 +32,49 @@ #ifdef HAVE_DBUS static DBusConnection *systembus = NULL; -static virOnceControl once = VIR_ONCE_CONTROL_INITIALIZER; -static DBusError dbuserr; +static DBusConnection *sessionbus = NULL; +static virOnceControl systemonce = VIR_ONCE_CONTROL_INITIALIZER; +static virOnceControl sessiononce = VIR_ONCE_CONTROL_INITIALIZER; +static DBusError systemdbuserr; +static DBusError sessiondbuserr; static dbus_bool_t virDBusAddWatch(DBusWatch *watch, void *data); static void virDBusRemoveWatch(DBusWatch *watch, void *data); static void virDBusToggleWatch(DBusWatch *watch, void *data); -static void virDBusSystemBusInit(void) +static DBusConnection *virDBusBusInit(DBusBusType type, DBusError *dbuserr) { + DBusConnection *bus; + /* Allocate and initialize a new HAL context */ dbus_connection_set_change_sigpipe(FALSE); dbus_threads_init_default(); - dbus_error_init(&dbuserr); - if (!(systembus = dbus_bus_get(DBUS_BUS_SYSTEM, &dbuserr))) - return; + dbus_error_init(dbuserr); + if (!(bus = dbus_bus_get(type, dbuserr))) + return NULL; - dbus_connection_set_exit_on_disconnect(systembus, FALSE); + dbus_connection_set_exit_on_disconnect(bus, FALSE); /* Register dbus watch callbacks */ - if (!dbus_connection_set_watch_functions(systembus, + if (!dbus_connection_set_watch_functions(bus, virDBusAddWatch, virDBusRemoveWatch, virDBusToggleWatch, - NULL, NULL)) { - systembus = NULL; - return; + bus, NULL)) { + return NULL; } + return bus; } +static void virDBusSystemBusInit(void) +{ + systembus = virDBusBusInit(DBUS_BUS_SYSTEM, &systemdbuserr); +} DBusConnection *virDBusGetSystemBus(void) { - if (virOnce(&once, virDBusSystemBusInit) < 0) { + if (virOnce(&systemonce, virDBusSystemBusInit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to run one time DBus initializer")); return NULL; @@ -74,7 +83,7 @@ DBusConnection *virDBusGetSystemBus(void) if (!systembus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get DBus system bus connection: %s"), - dbuserr.message ? dbuserr.message : "watch setup failed"); + systemdbuserr.message ? systemdbuserr.message : "watch setup failed"); return NULL; } @@ -82,13 +91,45 @@ DBusConnection *virDBusGetSystemBus(void) } +static void virDBusSessionBusInit(void) +{ + sessionbus = virDBusBusInit(DBUS_BUS_SESSION, &sessiondbuserr); +} + +DBusConnection *virDBusGetSessionBus(void) +{ + if (virOnce(&sessiononce, virDBusSessionBusInit) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to run one time DBus initializer")); + return NULL; + } + + if (!sessionbus) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to get DBus session bus connection: %s"), + sessiondbuserr.message ? sessiondbuserr.message : "watch setup failed"); + return NULL; + } + + return sessionbus; +} + +struct virDBusWatch +{ + int watch; + DBusConnection *bus; +}; + static void virDBusWatchCallback(int fdatch ATTRIBUTE_UNUSED, int fd ATTRIBUTE_UNUSED, int events, void *opaque) { DBusWatch *watch = opaque; + struct virDBusWatch *info; int dbus_flags = 0; + info = dbus_watch_get_data(watch); + if (events & VIR_EVENT_HANDLE_READABLE) dbus_flags |= DBUS_WATCH_READABLE; if (events & VIR_EVENT_HANDLE_WRITABLE) @@ -100,7 +141,7 @@ static void virDBusWatchCallback(int fdatch ATTRIBUTE_UNUSED, (void)dbus_watch_handle(watch, dbus_flags); - while (dbus_connection_dispatch(systembus) == DBUS_DISPATCH_DATA_REMAINS) + while (dbus_connection_dispatch(info->bus) == DBUS_DISPATCH_DATA_REMAINS) /* keep dispatching while data remains */; } @@ -120,18 +161,13 @@ static int virDBusTranslateWatchFlags(int dbus_flags) } -struct virDBusWatch -{ - int watch; -}; - static void virDBusWatchFree(void *data) { struct virDBusWatch *info = data; VIR_FREE(info); } static dbus_bool_t virDBusAddWatch(DBusWatch *watch, - void *data ATTRIBUTE_UNUSED) + void *data) { int flags = 0; int fd; @@ -148,6 +184,7 @@ static dbus_bool_t virDBusAddWatch(DBusWatch *watch, # else fd = dbus_watch_get_fd(watch); # endif + info->bus = (DBusConnection *)data; info->watch = virEventAddHandle(fd, flags, virDBusWatchCallback, watch, NULL); @@ -194,4 +231,11 @@ DBusConnection *virDBusGetSystemBus(void) return NULL; } +DBusConnection *virDBusGetSessionBus(void) +{ + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("DBus support not compiled into this binary")); + return NULL; +} + #endif /* ! HAVE_DBUS */ diff --git a/src/util/virdbus.h b/src/util/virdbus.h index 27dca00..e443fbe 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -30,5 +30,6 @@ # include "internal.h" DBusConnection *virDBusGetSystemBus(void); +DBusConnection *virDBusGetSessionBus(void); #endif /* __VIR_DBUS_H__ */ -- 1.7.11.7
----- Original Message -----
From: Alexander Larsson <alexl@redhat.com>
This splits out some common code from virDBusGetSystemBus and uses it to implement a new virDBusGetSessionBus helper. --- src/libvirt_private.syms | 1 + src/util/virdbus.c | 84 ++++++++++++++++++++++++++++++++++++------------ src/util/virdbus.h | 1 + 3 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 63e187a..8d649bf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1336,6 +1336,7 @@ virConsoleOpen;
# virdbus.h virDBusGetSystemBus; +virDBusGetSessionBus;
Sorting (although I see 4/10 touches this again, and it doesn't affect bisection, so I won't oppose this going in as-is)
+++ b/src/util/virdbus.c @@ -32,40 +32,49 @@ #ifdef HAVE_DBUS
static DBusConnection *systembus = NULL; -static virOnceControl once = VIR_ONCE_CONTROL_INITIALIZER; -static DBusError dbuserr; +static DBusConnection *sessionbus = NULL;
Explicit initialization to NULL is not required for static variables, but neither does it hurt, and it was just copy-and-paste (gcc is smart enough to stick explicit 0 initialization into .bss).
-static void virDBusSystemBusInit(void) +static DBusConnection *virDBusBusInit(DBusBusType type, DBusError *dbuserr)
Long line; should this be: static DBusConnection* virDBusBusInit(DBusBusType type, DBusError *dbuserr)
@@ -74,7 +83,7 @@ DBusConnection *virDBusGetSystemBus(void) if (!systembus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get DBus system bus connection: %s"), - dbuserr.message ? dbuserr.message : "watch setup failed"); + systemdbuserr.message ? systemdbuserr.message : "watch setup failed");
Worth wrapping this to avoid long lines? ACK
On Tue, Nov 27, 2012 at 01:48:51PM -0500, Eric Blake wrote:
----- Original Message -----
From: Alexander Larsson <alexl@redhat.com>
This splits out some common code from virDBusGetSystemBus and uses it to implement a new virDBusGetSessionBus helper. --- src/libvirt_private.syms | 1 + src/util/virdbus.c | 84 ++++++++++++++++++++++++++++++++++++------------ src/util/virdbus.h | 1 + 3 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 63e187a..8d649bf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1336,6 +1336,7 @@ virConsoleOpen;
# virdbus.h virDBusGetSystemBus; +virDBusGetSessionBus;
Sorting (although I see 4/10 touches this again, and it doesn't affect bisection, so I won't oppose this going in as-is)
I'll fix this while applying
-static void virDBusSystemBusInit(void) +static DBusConnection *virDBusBusInit(DBusBusType type, DBusError *dbuserr)
Long line; should this be:
static DBusConnection* virDBusBusInit(DBusBusType type, DBusError *dbuserr)
Yep, wil fix
@@ -74,7 +83,7 @@ DBusConnection *virDBusGetSystemBus(void) if (!systembus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get DBus system bus connection: %s"), - dbuserr.message ? dbuserr.message : "watch setup failed"); + systemdbuserr.message ? systemdbuserr.message : "watch setup failed");
Worth wrapping this to avoid long lines?
Ok Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/network/bridge_driver.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c153d36..fb167dc 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3303,11 +3303,11 @@ static virNetworkDriver networkDriver = { }; static virStateDriver networkStateDriver = { - "Network", - networkStartup, - networkShutdown, - networkReload, - networkActive, + .name = "Network", + .initialize = networkStartup, + .cleanup = networkShutdown, + .reload = networkReload, + .active = networkActive, }; int networkRegister(void) { -- 1.7.11.7
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/network/bridge_driver.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
ACK
From: "Daniel P. Berrange" <berrange@redhat.com> Sort the symbols listed in libvirt_daemon.syms Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_daemon.syms | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libvirt_daemon.syms b/src/libvirt_daemon.syms index eb6e594..7c914fa 100644 --- a/src/libvirt_daemon.syms +++ b/src/libvirt_daemon.syms @@ -3,8 +3,8 @@ # # libvirt_internal.h -virStateInitialize; +virRegisterStateDriver; +virStateActive; virStateCleanup; +virStateInitialize; virStateReload; -virStateActive; -virRegisterStateDriver; -- 1.7.11.7
From: "Daniel P. Berrange" <berrange@redhat.com>
Sort the symbols listed in libvirt_daemon.syms
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_daemon.syms | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Oh, this _didn't_ touch the unsorted addition in 2/10, since that was to a different file.
diff --git a/src/libvirt_daemon.syms b/src/libvirt_daemon.syms index eb6e594..7c914fa 100644 --- a/src/libvirt_daemon.syms +++ b/src/libvirt_daemon.syms @@ -3,8 +3,8 @@ #
# libvirt_internal.h -virStateInitialize; +virRegisterStateDriver; +virStateActive; virStateCleanup; +virStateInitialize; virStateReload; -virStateActive; -virRegisterStateDriver;
ACK.
From: "Daniel P. Berrange" <berrange@redhat.com> To allow actions to be performed in libvirtd when the host shuts down, or user session exits, introduce a 'stop' method to virDriverState. This will do things like saving the VM state to a file. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/driver.h | 2 ++ src/libvirt.c | 18 ++++++++++++++++++ src/libvirt_daemon.syms | 1 + src/libvirt_internal.h | 1 + 4 files changed, 22 insertions(+) diff --git a/src/driver.h b/src/driver.h index 7ba66ad..7d5a367 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1491,6 +1491,7 @@ typedef int (*virDrvStateInitialize) (int privileged); typedef int (*virDrvStateCleanup) (void); typedef int (*virDrvStateReload) (void); typedef int (*virDrvStateActive) (void); +typedef int (*virDrvStateStop) (void); typedef struct _virStateDriver virStateDriver; typedef virStateDriver *virStateDriverPtr; @@ -1501,6 +1502,7 @@ struct _virStateDriver { virDrvStateCleanup cleanup; virDrvStateReload reload; virDrvStateActive active; + virDrvStateStop stop; }; # endif diff --git a/src/libvirt.c b/src/libvirt.c index 4af6089..86c3deb 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -869,6 +869,24 @@ int virStateActive(void) { return ret; } +/** + * virStateStop: + * + * Run each virtualization driver's "stop" method. + * + * Returns 0 if successful, -1 on failure + */ +int virStateStop(void) { + int i, ret = 0; + + for (i = 0 ; i < virStateDriverTabCount ; i++) { + if (virStateDriverTab[i]->stop && + virStateDriverTab[i]->stop()) + ret = 1; + } + return ret; +} + #endif diff --git a/src/libvirt_daemon.syms b/src/libvirt_daemon.syms index 7c914fa..dde77a6 100644 --- a/src/libvirt_daemon.syms +++ b/src/libvirt_daemon.syms @@ -8,3 +8,4 @@ virStateActive; virStateCleanup; virStateInitialize; virStateReload; +virStateStop; diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 71483e4..a39aaa4 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -32,6 +32,7 @@ int virStateInitialize(int privileged); int virStateCleanup(void); int virStateReload(void); int virStateActive(void); +int virStateStop(void); # endif /* Feature detection. This is a libvirt-private interface for determining -- 1.7.11.7
To allow actions to be performed in libvirtd when the host shuts down, or user session exits, introduce a 'stop' method to virDriverState. This will do things like saving the VM state to a file.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/driver.h | 2 ++ src/libvirt.c | 18 ++++++++++++++++++ src/libvirt_daemon.syms | 1 + src/libvirt_internal.h | 1 + 4 files changed, 22 insertions(+)
ACK.
From: "Daniel P. Berrange" <berrange@redhat.com> When the virStateStop() method is invoked, perform a managed save of all VMs currently running Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c526f5f..8e098ef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -994,6 +994,74 @@ qemuActive(void) { return active; } + +/* + * qemuStop: + * + * Save any VMs in preparation for shutdown + * + */ +static int +qemuStop(void) { + int ret = -1; + const char *uri; + virConnectPtr conn; + int numDomains; + size_t i; + int state; + virDomainPtr *domains = NULL; + unsigned int *flags = NULL; + + qemuDriverLock(qemu_driver); + uri = qemu_driver->privileged ? + "qemu:///system" : + "qemu:///session"; + qemuDriverUnlock(qemu_driver); + + if (!(conn = virConnectOpen(uri))) + return -1; + + if ((numDomains = virConnectListAllDomains(conn, + &domains, + VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0) + goto cleanup; + + if (VIR_ALLOC_N(flags, numDomains) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* First we pause all VMs to make them stop dirtying + pages, etc. We remember if any VMs were paused so + we can restore that on resume. */ + for (i = 0 ; i < numDomains ; i++) { + flags[i] = VIR_DOMAIN_SAVE_RUNNING; + if (virDomainGetState(domains[i], &state, NULL, 0) == 0) { + if (state == VIR_DOMAIN_PAUSED) { + flags[i] = VIR_DOMAIN_SAVE_PAUSED; + } + } + virDomainSuspend(domains[i]); + } + + ret = 0; + /* Then we save the VMs to disk */ + for (i = 0 ; i < numDomains ; i++) + if (virDomainManagedSave(domains[i], flags[i]) < 0) + ret = -1; + + VIR_FREE(domains); + VIR_FREE(flags); + + cleanup: + for (i = 0 ; i < numDomains ; i++) + virDomainFree(domains[i]); + VIR_FREE(domains); + VIR_FREE(flags); + + return ret; +} + /** * qemuShutdown: * @@ -14908,6 +14976,7 @@ static virStateDriver qemuStateDriver = { .cleanup = qemuShutdown, .reload = qemuReload, .active = qemuActive, + .stop = qemuStop, }; int qemuRegister(void) { -- 1.7.11.7
From: "Daniel P. Berrange" <berrange@redhat.com>
When the virStateStop() method is invoked, perform a managed save of all VMs currently running
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
How does this interact with the libvirt-guests init script? I guess that if that is installed, it gets to run first before the system libvirtd is shutdown, so the qemu:///system won't see any running guests, and this patch would just affect qemu:///session? Is managed save always the right thing, or should we allow the user to configure for a graceful shutdown and/or a migration to other host? Do we need timeouts, as managed save may take a while per guest? Similar to how /etc/sysconfig/libvirt-guests can request O_DIRECT to avoid file system pollution, do we need a way for the user to configure the use of that flag? Is doing the same thing for all guests wise, or do we need to allow for the possibility of a per-guest choice of what action to take (maybe via a new <on_host_shutdown> element?) where we only fall back to the configured default if the guest XML didn't request something? Do we correctly and automatically restart all the guests that were saved by this hook the next time libvirtd restarts?
+ /* First we pause all VMs to make them stop dirtying + pages, etc. We remember if any VMs were paused so + we can restore that on resume. */ + for (i = 0 ; i < numDomains ; i++) { + flags[i] = VIR_DOMAIN_SAVE_RUNNING; + if (virDomainGetState(domains[i], &state, NULL, 0) == 0) { + if (state == VIR_DOMAIN_PAUSED) { + flags[i] = VIR_DOMAIN_SAVE_PAUSED; + } + } + virDomainSuspend(domains[i]);
Should you be checking for errors here?
+ } + + ret = 0; + /* Then we save the VMs to disk */ + for (i = 0 ; i < numDomains ; i++) + if (virDomainManagedSave(domains[i], flags[i]) < 0) + ret = -1;
What happens if a guest managed to exit itself after the initial virConnectListAllDomains() and this point? The virDomainManagedSave will fail, but that is not an error since a stopped guest has nothing to save after all. Do you need to be checking specific error codes here before treating this as a failure? In general, I like the idea, but it introduces enough questions that I'm not quite sure about ack'ing it yet. I'll see if reviewing the rest of the series impacts my decision.
On Tue, Nov 27, 2012 at 03:06:45PM -0500, Eric Blake wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When the virStateStop() method is invoked, perform a managed save of all VMs currently running
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
How does this interact with the libvirt-guests init script? I guess that if that is installed, it gets to run first before the system libvirtd is shutdown, so the qemu:///system won't see any running guests, and this patch would just affect qemu:///session?
In theory it can do both qemu:///system and qemu:///session, however, the later patches only wire this up for qemu:///session, precisely because libvirt-guests already exists.
Is managed save always the right thing, or should we allow the user to configure for a graceful shutdown and/or a migration to other host? Do we need timeouts, as managed save may take a while per guest? Similar to how /etc/sysconfig/libvirt-guests can request O_DIRECT to avoid file system pollution, do we need a way for the user to configure the use of that flag? Is doing the same thing for all guests wise, or do we need to allow for the possibility of a per-guest choice of what action to take (maybe via a new <on_host_shutdown> element?) where we only fall back to the configured default if the guest XML didn't request something? Do we correctly and automatically restart all the guests that were saved by this hook the next time libvirtd restarts?
We don't attempt to automatically restart all guests - we will only restart those marked as "autostart", which I think is the right thing todo in general. I think managed save is the only reasonable choice here, because we want something that is going to be reliable, and zero-conf for the user. Migration requires the user to pick a dest host, and is not guaranteed to converge. Graceful shutdown relies on a co-operating guest. So IMHO, neither of those are really satisfactory options for running on desktop logout / host shutdown. Not sure about O_DIRECT - I'm inclined to say we should just *always* use O_DIRECT - unless someone can point out a downside with it ? Long term, I make no secret of the fact that I want to see libvirt-guests die in horribly painful way. This kind of functionality should be brought "in house" to libvirtd and obviously this new code is a start in that direction.
+ /* First we pause all VMs to make them stop dirtying + pages, etc. We remember if any VMs were paused so + we can restore that on resume. */ + for (i = 0 ; i < numDomains ; i++) { + flags[i] = VIR_DOMAIN_SAVE_RUNNING; + if (virDomainGetState(domains[i], &state, NULL, 0) == 0) { + if (state == VIR_DOMAIN_PAUSED) { + flags[i] = VIR_DOMAIN_SAVE_PAUSED; + } + } + virDomainSuspend(domains[i]);
Should you be checking for errors here?
Yes & no. Pausing the guests is just a performance tweak, so if it fails it isn't critical. If pausing fails, chances are the managed save willl fail too, so we'll likely get an erorr regardless.
+ } + + ret = 0; + /* Then we save the VMs to disk */ + for (i = 0 ; i < numDomains ; i++) + if (virDomainManagedSave(domains[i], flags[i]) < 0) + ret = -1;
What happens if a guest managed to exit itself after the initial virConnectListAllDomains() and this point? The virDomainManagedSave will fail, but that is not an error since a stopped guest has nothing to save after all. Do you need to be checking specific error codes here before treating this as a failure?
I guess we could handle that - though the caller will ignore all the errors anyway :-) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Tue, Nov 27, 2012 at 03:06:45PM -0500, Eric Blake wrote:
How does this interact with the libvirt-guests init script? I guess that if that is installed, it gets to run first before the system libvirtd is shutdown, so the qemu:///system won't see any running guests, and this patch would just affect qemu:///session?
In theory it can do both qemu:///system and qemu:///session, however, the later patches only wire this up for qemu:///session, precisely because libvirt-guests already exists.
In other words, right now, there is no interaction, but we may convert libvirt-guests in the future to use this method instead. Fair enough, and shouldn't hold up this series.
Is managed save always the right thing, or should we allow the user to configure for a graceful shutdown and/or a migration to other host? Do we need timeouts, as managed save may take a while per guest? Similar to how /etc/sysconfig/libvirt-guests can request O_DIRECT to avoid file system pollution, do we need a way for the user to configure the use of that flag? Is doing the same thing for all guests wise, or do we need to allow for the possibility of a per-guest choice of what action to take (maybe via a new <on_host_shutdown> element?) where we only fall back to the configured default if the guest XML didn't request something? Do we correctly and automatically restart all the guests that were saved by this hook the next time libvirtd restarts?
We don't attempt to automatically restart all guests - we will only restart those marked as "autostart", which I think is the right thing todo in general.
Well, with 'libvirt-guests', we restarted everything that had been saved, not just those guests marked autostart. (Or another way of looking at it: saving a guest on libvirtd shutdown resulted in the guest behaving as if it had a one-shot autostart). I think it may be a bit awkward if session behaves differently from libvirt-guests regarding the resume of guests that are not autostart.
I think managed save is the only reasonable choice here, because we want something that is going to be reliable, and zero-conf for the user. Migration requires the user to pick a dest host, and is not guaranteed to converge. Graceful shutdown relies on a co-operating guest. So IMHO, neither of those are really satisfactory options for running on desktop logout / host shutdown.
I agree that managed save is the only reasonable default, but I still wonder if we need to allow for non-default configurations, rather than forcing managed save as the only solution.
Not sure about O_DIRECT - I'm inclined to say we should just *always* use O_DIRECT - unless someone can point out a downside with it ?
About the only possible downside I can see is that it _might_ be slightly slower, as it forces libvirt to take all the I/O through a pipe before playing it out to the file system; which is why we exposed the option to request O_DIRECT when I first implemented it. But later, we switched to always using a pipe anyways, because current qemu doesn't know how to throttle when writing to regular files (it only properly does bandwidth throttling on a file descriptor that can report EAGAIN, such as a pipe), so now that we are always using a pipe, always using O_DIRECT might make sense as a default (and make the existing flag for requesting O_DIRECT become a no-op because we are automatically using direct mode to begin with). Thoughts? If we make O_DIRECT unconditional, then we have several followup patches to write.
Long term, I make no secret of the fact that I want to see libvirt-guests die in horribly painful way. This kind of functionality should be brought "in house" to libvirtd and obviously this new code is a start in that direction.
Agree.
+ virDomainSuspend(domains[i]);
Should you be checking for errors here?
Yes & no. Pausing the guests is just a performance tweak, so if it fails it isn't critical. If pausing fails, chances are the managed save willl fail too, so we'll likely get an erorr regardless.
Fair enough.
+ } + + ret = 0; + /* Then we save the VMs to disk */ + for (i = 0 ; i < numDomains ; i++) + if (virDomainManagedSave(domains[i], flags[i]) < 0) + ret = -1;
What happens if a guest managed to exit itself after the initial virConnectListAllDomains() and this point? The virDomainManagedSave will fail, but that is not an error since a stopped guest has nothing to save after all. Do you need to be checking specific error codes here before treating this as a failure?
I guess we could handle that - though the caller will ignore all the errors anyway :-)
At this point, I think you've answered my questions. I'm not sure if you made any minor tweaks based on my feedback, but it looks like this patch is ready to push and any future work can be in followup patches (since we are already planning for followups related to libvirt-guests). ACK.
Not sure about O_DIRECT - I'm inclined to say we should just *always* use O_DIRECT - unless someone can point out a downside with it ?
About the only possible downside I can see is that it _might_ be slightly slower, as it forces libvirt to take all the I/O through a pipe before playing it out to the file system; which is why we exposed the option to request O_DIRECT when I first implemented it.
Another downside - some people like to play with virtual machines in /tmp, but if /tmp is mounted on tmpfs, then O_DIRECT does not work. Seeing as how O_DIRECT is not mandated by POSIX, and several important file systems like tmpfs do not support it, we have to choose whether to default to something that fails on some file systems, and/or make our O_DIRECT code gracefully fall back rather than erroring out when not available.
From: "Daniel P. Berrange" <berrange@redhat.com> The virStateInitialize method and several cgroups methods were using an 'int privileged' parameter or similar for dual-state values. These are better represented with the bool type. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/driver.h | 2 +- src/libvirt.c | 4 ++-- src/libvirt_internal.h | 2 +- src/libxl/libxl_driver.c | 2 +- src/lxc/lxc_driver.c | 4 ++-- src/network/bridge_driver.c | 2 +- src/node_device/node_device_hal.c | 2 +- src/node_device/node_device_udev.c | 2 +- src/nwfilter/nwfilter_driver.c | 2 +- src/parallels/parallels_storage.c | 2 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 2 +- src/remote/remote_driver.c | 6 +++--- src/secret/secret_driver.c | 2 +- src/storage/storage_driver.c | 2 +- src/uml/uml_conf.h | 2 +- src/uml/uml_driver.c | 2 +- src/util/cgroup.c | 34 ++++++++++++++++++---------------- src/util/cgroup.h | 10 +++++----- src/xen/xen_driver.c | 6 +++--- 20 files changed, 47 insertions(+), 45 deletions(-) diff --git a/src/driver.h b/src/driver.h index 7d5a367..622ed87 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1487,7 +1487,7 @@ struct _virStorageDriver { }; # ifdef WITH_LIBVIRTD -typedef int (*virDrvStateInitialize) (int privileged); +typedef int (*virDrvStateInitialize) (bool privileged); typedef int (*virDrvStateCleanup) (void); typedef int (*virDrvStateReload) (void); typedef int (*virDrvStateActive) (void); diff --git a/src/libvirt.c b/src/libvirt.c index 86c3deb..0e73d64 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -789,13 +789,13 @@ virRegisterStateDriver(virStateDriverPtr driver) /** * virStateInitialize: - * @privileged: set to 1 if running with root privilege, 0 otherwise + * @privileged: set to true if running with root privilege, false otherwise * * Initialize all virtualization drivers. * * Returns 0 if all succeed, -1 upon any failure. */ -int virStateInitialize(int privileged) { +int virStateInitialize(bool privileged) { int i; if (virInitialize() < 0) diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index a39aaa4..b85a29d 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -28,7 +28,7 @@ # include "internal.h" # ifdef WITH_LIBVIRTD -int virStateInitialize(int privileged); +int virStateInitialize(bool privileged); int virStateCleanup(void); int virStateReload(void); int virStateActive(void); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index bc75730..ae4451a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -834,7 +834,7 @@ libxlShutdown(void) } static int -libxlStartup(int privileged) { +libxlStartup(bool privileged) { const libxl_version_info *ver_info; char *log_file = NULL; virCommandPtr cmd; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 991b593..6cb3fe2 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -69,7 +69,7 @@ #define LXC_NB_MEM_PARAM 3 -static int lxcStartup(int privileged); +static int lxcStartup(bool privileged); static int lxcShutdown(void); virLXCDriverPtr lxc_driver = NULL; @@ -1397,7 +1397,7 @@ error: } -static int lxcStartup(int privileged) +static int lxcStartup(bool privileged) { char *ld; int rc; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index fb167dc..67ee262 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -330,7 +330,7 @@ firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED, * Initialization function for the QEmu daemon */ static int -networkStartup(int privileged) { +networkStartup(bool privileged) { char *base = NULL; #ifdef HAVE_FIREWALLD DBusConnection *sysbus = NULL; diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index ff73db0..953e1d3 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -588,7 +588,7 @@ static void device_prop_modified(LibHalContext *ctx ATTRIBUTE_UNUSED, -static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) +static int halDeviceMonitorStartup(bool privileged ATTRIBUTE_UNUSED) { LibHalContext *hal_ctx = NULL; char **udi = NULL; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index acd78f2..bc4e2e9 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1604,7 +1604,7 @@ out: return ret; } -static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) +static int udevDeviceMonitorStartup(bool privileged ATTRIBUTE_UNUSED) { udevPrivate *priv = NULL; struct udev *udev = NULL; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 12f47ef..a0ee4f1 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -165,7 +165,7 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) * Initialization function for the QEmu daemon */ static int -nwfilterDriverStartup(int privileged) +nwfilterDriverStartup(bool privileged) { char *base = NULL; DBusConnection *sysbus = NULL; diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index 8d4e2c6..9075dfd 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -121,7 +121,7 @@ parallelsStorageOpen(virConnectPtr conn, { char *base = NULL; virStorageDriverStatePtr storageState; - int privileged = (geteuid() == 0); + bool privileged = (geteuid() == 0); parallelsConnPtr privconn = conn->privateData; size_t i; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 4c729e4..b7378a7 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -56,7 +56,7 @@ struct qemud_driver { virThreadPoolPtr workerPool; - int privileged; + bool privileged; const char *uri; uid_t user; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8e098ef..186b79f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -610,7 +610,7 @@ qemuDomainFindMaxID(void *payload, * Initialization function for the QEmu daemon */ static int -qemuStartup(int privileged) { +qemuStartup(bool privileged) { char *base = NULL; char *driverConf = NULL; int rc; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ec33698..01cebb4 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -68,7 +68,7 @@ # define HYPER_TO_ULONG(_to, _from) (_to) = (_from) #endif -static int inside_daemon = 0; +static bool inside_daemon = false; static virDriverPtr remoteDriver = NULL; struct private_data { @@ -150,12 +150,12 @@ static char *get_transport_from_scheme(char *scheme); #ifdef WITH_LIBVIRTD static int -remoteStartup(int privileged ATTRIBUTE_UNUSED) +remoteStartup(bool privileged ATTRIBUTE_UNUSED) { /* Mark that we're inside the daemon so we can avoid * re-entering ourselves */ - inside_daemon = 1; + inside_daemon = true; return 0; } #endif diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 51e1e46..c7aabfc 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -1073,7 +1073,7 @@ secretDriverCleanup(void) } static int -secretDriverStartup(int privileged) +secretDriverStartup(bool privileged) { char *base = NULL; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 2e33b80..d27bb41 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -128,7 +128,7 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { * Initialization function for the QEmu daemon */ static int -storageDriverStartup(int privileged) +storageDriverStartup(bool privileged) { char *base = NULL; diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h index 274041d..ebae24e 100644 --- a/src/uml/uml_conf.h +++ b/src/uml/uml_conf.h @@ -44,7 +44,7 @@ struct uml_driver { virMutex lock; - int privileged; + bool privileged; unsigned long umlVersion; int nextvmid; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 62077e1..5a87e31 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -419,7 +419,7 @@ cleanup: * Initialization function for the Uml daemon */ static int -umlStartup(int privileged) +umlStartup(bool privileged) { char *base = NULL; char *userdir = NULL; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 41a6e4a..9a564cc 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -526,8 +526,10 @@ static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) return rc; } -static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, - int create, unsigned int flags) +static int virCgroupMakeGroup(virCgroupPtr parent, + virCgroupPtr group, + bool create, + unsigned int flags) { int i; int rc = 0; @@ -641,9 +643,9 @@ err: return rc; } -static int virCgroupAppRoot(int privileged, +static int virCgroupAppRoot(bool privileged, virCgroupPtr *group, - int create) + bool create) { virCgroupPtr rootgrp = NULL; int rc; @@ -924,8 +926,8 @@ cleanup: #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupForDriver(const char *name, virCgroupPtr *group, - int privileged, - int create) + bool privileged, + bool create) { int rc; char *path = NULL; @@ -957,8 +959,8 @@ out: #else int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED, - int privileged ATTRIBUTE_UNUSED, - int create ATTRIBUTE_UNUSED) + bool privileged ATTRIBUTE_UNUSED, + bool create ATTRIBUTE_UNUSED) { /* Claim no support */ return -ENXIO; @@ -979,7 +981,7 @@ int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED, int virCgroupForDomain(virCgroupPtr driver, const char *name, virCgroupPtr *group, - int create) + bool create) { int rc; char *path; @@ -1015,7 +1017,7 @@ int virCgroupForDomain(virCgroupPtr driver, int virCgroupForDomain(virCgroupPtr driver ATTRIBUTE_UNUSED, const char *name ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED, - int create ATTRIBUTE_UNUSED) + bool create ATTRIBUTE_UNUSED) { return -ENXIO; } @@ -1034,7 +1036,7 @@ int virCgroupForDomain(virCgroupPtr driver ATTRIBUTE_UNUSED, int virCgroupForVcpu(virCgroupPtr driver, int vcpuid, virCgroupPtr *group, - int create) + bool create) { int rc; char *path; @@ -1060,7 +1062,7 @@ int virCgroupForVcpu(virCgroupPtr driver, int virCgroupForVcpu(virCgroupPtr driver ATTRIBUTE_UNUSED, int vcpuid ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED, - int create ATTRIBUTE_UNUSED) + bool create ATTRIBUTE_UNUSED) { return -ENXIO; } @@ -1076,8 +1078,8 @@ int virCgroupForVcpu(virCgroupPtr driver ATTRIBUTE_UNUSED, */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupForEmulator(virCgroupPtr driver, - virCgroupPtr *group, - int create) + virCgroupPtr *group, + bool create) { int rc; char *path; @@ -1101,8 +1103,8 @@ int virCgroupForEmulator(virCgroupPtr driver, } #else int virCgroupForEmulator(virCgroupPtr driver ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED, - int create ATTRIBUTE_UNUSED) + virCgroupPtr *group ATTRIBUTE_UNUSED, + bool create ATTRIBUTE_UNUSED) { return -ENXIO; } diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 38fa4b7..e717227 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -44,22 +44,22 @@ VIR_ENUM_DECL(virCgroupController); int virCgroupForDriver(const char *name, virCgroupPtr *group, - int privileged, - int create); + bool privileged, + bool create); int virCgroupForDomain(virCgroupPtr driver, const char *name, virCgroupPtr *group, - int create); + bool create); int virCgroupForVcpu(virCgroupPtr driver, int vcpuid, virCgroupPtr *group, - int create); + bool create); int virCgroupForEmulator(virCgroupPtr driver, virCgroupPtr *group, - int create); + bool create); int virCgroupPathOfController(virCgroupPtr group, int controller, diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index bfdca3d..5a40757 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -90,7 +90,7 @@ static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = { }; #if defined WITH_LIBVIRTD || defined __sun -static int inside_daemon; +static bool inside_daemon = false; #endif /** @@ -201,9 +201,9 @@ done: #ifdef WITH_LIBVIRTD static int -xenInitialize(int privileged ATTRIBUTE_UNUSED) +xenInitialize(bool privileged ATTRIBUTE_UNUSED) { - inside_daemon = 1; + inside_daemon = true; return 0; } -- 1.7.11.7
From: "Daniel P. Berrange" <berrange@redhat.com>
The virStateInitialize method and several cgroups methods were using an 'int privileged' parameter or similar for dual-state values. These are better represented with the bool type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
+++ b/src/driver.h @@ -1487,7 +1487,7 @@ struct _virStorageDriver { };
# ifdef WITH_LIBVIRTD -typedef int (*virDrvStateInitialize) (int privileged); +typedef int (*virDrvStateInitialize) (bool privileged);
ACK.
+++ b/src/remote/remote_driver.c @@ -68,7 +68,7 @@ # define HYPER_TO_ULONG(_to, _from) (_to) = (_from) #endif
-static int inside_daemon = 0; +static bool inside_daemon = false;
Another case of non-necessary static initialization to 0.
From: "Daniel P. Berrange" <berrange@redhat.com> Currently to deal with auto-shutdown libvirtd must periodically poll all stateful drivers. Thus sucks because it requires acquiring both the driver lock and locks on every single virtual machine. Instead pass in a "inhibit" callback to virStateInitialize which drivers can invoke whenever they want to inhibit shutdown due to existance of active VMs. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 30 ++++++++++++++++-------------- src/Makefile.am | 2 ++ src/driver.h | 7 ++++--- src/libvirt.c | 29 +++++++++-------------------- src/libvirt_internal.h | 8 ++++++-- src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.h | 5 +++++ src/libxl/libxl_driver.c | 26 ++++++++++++++++---------- src/lxc/lxc_conf.h | 5 +++++ src/lxc/lxc_driver.c | 29 ++++++----------------------- src/lxc/lxc_process.c | 12 ++++++++++++ src/network/bridge_driver.c | 33 ++++----------------------------- src/node_device/node_device_hal.c | 14 +++----------- src/node_device/node_device_udev.c | 12 +++--------- src/nwfilter/nwfilter_driver.c | 26 +++----------------------- src/qemu/qemu_conf.h | 4 ++++ src/qemu/qemu_driver.c | 30 ++++++------------------------ src/qemu/qemu_process.c | 16 ++++++++++++++++ src/remote/remote_driver.c | 4 +++- src/rpc/virnetserver.c | 29 +++++++++++++++++++++-------- src/rpc/virnetserver.h | 7 ++++--- src/secret/secret_driver.c | 5 +++-- src/storage/storage_driver.c | 32 +++----------------------------- src/uml/uml_conf.h | 3 +++ src/uml/uml_driver.c | 37 ++++++++++++++----------------------- src/xen/xen_driver.c | 4 +++- 26 files changed, 176 insertions(+), 235 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 1ebbac1..f7046f6 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -584,16 +584,6 @@ error: } -static int daemonShutdownCheck(virNetServerPtr srv ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) -{ - if (virStateActive()) - return 0; - - return 1; -} - - /* * Set up the logging environment * By default if daemonized all errors go to the logfile libvirtd.log, @@ -772,6 +762,18 @@ static int daemonSetupSignals(virNetServerPtr srv) return 0; } + +static void daemonInhibitCallback(bool inhibit, void *opaque) +{ + virNetServerPtr srv = opaque; + + if (inhibit) + virNetServerAddShutdownInhibition(srv); + else + virNetServerRemoveShutdownInhibition(srv); +} + + static void daemonRunStateInit(void *opaque) { virNetServerPtr srv = opaque; @@ -780,7 +782,9 @@ static void daemonRunStateInit(void *opaque) * This is deliberately done after telling the parent process * we're ready, since it can take a long time and this will * seriously delay OS bootup process */ - if (virStateInitialize(virNetServerIsPrivileged(srv)) < 0) { + if (virStateInitialize(virNetServerIsPrivileged(srv), + daemonInhibitCallback, + srv) < 0) { VIR_ERROR(_("Driver state initialization failed")); /* Ensure the main event loop quits */ kill(getpid(), SIGTERM); @@ -1270,9 +1274,7 @@ int main(int argc, char **argv) { if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); virNetServerAutoShutdown(srv, - timeout, - daemonShutdownCheck, - NULL); + timeout); } if ((daemonSetupSignals(srv)) < 0) { diff --git a/src/Makefile.am b/src/Makefile.am index 4026a15..6109e56 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1597,12 +1597,14 @@ libvirt_net_rpc_server_la_SOURCES = \ rpc/virnetserver.h rpc/virnetserver.c libvirt_net_rpc_server_la_CFLAGS = \ $(AVAHI_CFLAGS) \ + $(DBUS_CFLAGS) \ $(XDR_CFLAGS) \ $(AM_CFLAGS) \ $(POLKIT_CFLAGS) libvirt_net_rpc_server_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(AVAHI_LIBS) \ + $(DBUS_LIBS) \ $(POLKIT_LIBS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) diff --git a/src/driver.h b/src/driver.h index 622ed87..632f80e 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1487,10 +1487,12 @@ struct _virStorageDriver { }; # ifdef WITH_LIBVIRTD -typedef int (*virDrvStateInitialize) (bool privileged); + +typedef int (*virDrvStateInitialize) (bool privileged, + virStateInhibitCallback callback, + void *opaque); typedef int (*virDrvStateCleanup) (void); typedef int (*virDrvStateReload) (void); -typedef int (*virDrvStateActive) (void); typedef int (*virDrvStateStop) (void); typedef struct _virStateDriver virStateDriver; @@ -1501,7 +1503,6 @@ struct _virStateDriver { virDrvStateInitialize initialize; virDrvStateCleanup cleanup; virDrvStateReload reload; - virDrvStateActive active; virDrvStateStop stop; }; # endif diff --git a/src/libvirt.c b/src/libvirt.c index 0e73d64..78da7c5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -790,12 +790,17 @@ virRegisterStateDriver(virStateDriverPtr driver) /** * virStateInitialize: * @privileged: set to true if running with root privilege, false otherwise + * @callback: callback to invoke to inhibit shutdown of the daemon + * @opaque: data to pass to @callback * * Initialize all virtualization drivers. * * Returns 0 if all succeed, -1 upon any failure. */ -int virStateInitialize(bool privileged) { +int virStateInitialize(bool privileged, + virStateInhibitCallback callback, + void *opaque) +{ int i; if (virInitialize() < 0) @@ -805,7 +810,9 @@ int virStateInitialize(bool privileged) { if (virStateDriverTab[i]->initialize) { VIR_DEBUG("Running global init for %s state driver", virStateDriverTab[i]->name); - if (virStateDriverTab[i]->initialize(privileged) < 0) { + if (virStateDriverTab[i]->initialize(privileged, + callback, + opaque) < 0) { VIR_ERROR(_("Initialization of %s state driver failed"), virStateDriverTab[i]->name); return -1; @@ -852,24 +859,6 @@ int virStateReload(void) { } /** - * virStateActive: - * - * Run each virtualization driver's "active" method. - * - * Returns 0 if none are active, 1 if at least one is. - */ -int virStateActive(void) { - int i, ret = 0; - - for (i = 0 ; i < virStateDriverTabCount ; i++) { - if (virStateDriverTab[i]->active && - virStateDriverTab[i]->active()) - ret = 1; - } - return ret; -} - -/** * virStateStop: * * Run each virtualization driver's "stop" method. diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index b85a29d..2eda156 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -28,10 +28,14 @@ # include "internal.h" # ifdef WITH_LIBVIRTD -int virStateInitialize(bool privileged); +typedef void (*virStateInhibitCallback)(bool inhibit, + void *opaque); + +int virStateInitialize(bool privileged, + virStateInhibitCallback inhibit, + void *opaque); int virStateCleanup(void); int virStateReload(void); -int virStateActive(void); int virStateStop(void); # endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8d649bf..6e99f28 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1580,6 +1580,7 @@ xdr_virNetMessageError; # virnetserver.h virNetServerAddProgram; virNetServerAddService; +virNetServerAddShutdownInhibition; virNetServerAddSignalHandler; virNetServerAutoShutdown; virNetServerClose; @@ -1589,6 +1590,7 @@ virNetServerNew; virNetServerNewPostExecRestart; virNetServerPreExecRestart; virNetServerQuit; +virNetServerRemoveShutdownInhibition; virNetServerRun; virNetServerSetTLSContext; virNetServerUpdateServices; diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 56bf85c..189a4e9 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -61,6 +61,11 @@ struct _libxlDriverPrivate { libxl_ctx ctx; virBitmapPtr reservedVNCPorts; + + size_t nactive; + virStateInhibitCallback inhibitCallback; + void *inhibitOpaque; + virDomainObjList domains; virDomainEventStatePtr domainEventState; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ae4451a..c67d95a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -312,6 +312,10 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); } + driver->nactive--; + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(false, driver->inhibitOpaque); + if ((vm->def->ngraphics == 1) && vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && vm->def->graphics[0]->data.vnc.autoport) { @@ -717,6 +721,10 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto error; + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, restore_fd < 0 ? VIR_DOMAIN_EVENT_STARTED_BOOTED : @@ -782,6 +790,10 @@ libxlReconnectDomain(void *payload, vm->def->id = d_info.domid; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN); + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + /* Recreate domain death et. al. events */ libxlCreateDomEvents(vm); virDomainObjUnlock(vm); @@ -834,7 +846,10 @@ libxlShutdown(void) } static int -libxlStartup(bool privileged) { +libxlStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ const libxl_version_info *ver_info; char *log_file = NULL; virCommandPtr cmd; @@ -1030,14 +1045,6 @@ libxlReload(void) return 0; } -static int -libxlActive(void) -{ - if (!libxl_driver) - return 0; - - return 1; -} static virDrvOpenStatus libxlOpen(virConnectPtr conn, @@ -3969,7 +3976,6 @@ static virStateDriver libxlStateDriver = { .initialize = libxlStartup, .cleanup = libxlShutdown, .reload = libxlReload, - .active = libxlActive, }; diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index 4ae0c5e..ea345be 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -52,6 +52,11 @@ struct _virLXCDriver { virCapsPtr caps; virCgroupPtr cgroup; + + size_t nactive; + virStateInhibitCallback inhibitCallback; + void *inhibitOpaque; + virDomainObjList domains; char *configDir; char *autostartDir; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 6cb3fe2..fbadbcb 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -69,7 +69,9 @@ #define LXC_NB_MEM_PARAM 3 -static int lxcStartup(bool privileged); +static int lxcStartup(bool privileged, + virStateInhibitCallback callback, + void *opaque); static int lxcShutdown(void); virLXCDriverPtr lxc_driver = NULL; @@ -1397,7 +1399,9 @@ error: } -static int lxcStartup(bool privileged) +static int lxcStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { char *ld; int rc; @@ -1563,26 +1567,6 @@ static int lxcShutdown(void) return 0; } -/** - * lxcActive: - * - * Checks if the LXC daemon is active, i.e. has an active domain - * - * Returns 1 if active, 0 otherwise - */ -static int -lxcActive(void) { - int active; - - if (lxc_driver == NULL) - return 0; - - lxcDriverLock(lxc_driver); - active = virDomainObjListNumOfDomains(&lxc_driver->domains, 1); - lxcDriverUnlock(lxc_driver); - - return active; -} static int lxcVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *version) { @@ -2757,7 +2741,6 @@ static virStateDriver lxcStateDriver = { .name = LXC_DRIVER_NAME, .initialize = lxcStartup, .cleanup = lxcShutdown, - .active = lxcActive, .reload = lxcReload, }; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 94a74dd..81124f1 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -251,6 +251,10 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, vm->pid = -1; vm->def->id = -1; + driver->nactive--; + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(false, driver->inhibitOpaque); + for (i = 0 ; i < vm->def->nnets ; i++) { virDomainNetDefPtr iface = vm->def->nets[i]; vport = virDomainNetGetActualVirtPortProfile(iface); @@ -1131,6 +1135,10 @@ int virLXCProcessStart(virConnectPtr conn, virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); priv->doneStopEvent = false; + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + if (lxcContainerWaitForContinue(handshakefds[0]) < 0) { char out[1024]; @@ -1301,6 +1309,10 @@ virLXCProcessReconnectDomain(void *payload, const void *name ATTRIBUTE_UNUSED, v virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN); + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm))) goto error; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 67ee262..d8f5989 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -330,7 +330,10 @@ firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED, * Initialization function for the QEmu daemon */ static int -networkStartup(bool privileged) { +networkStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ char *base = NULL; #ifdef HAVE_FIREWALLD DBusConnection *sysbus = NULL; @@ -464,33 +467,6 @@ networkReload(void) { return 0; } -/** - * networkActive: - * - * Checks if the QEmu daemon is active, i.e. has an active domain or - * an active network - * - * Returns 1 if active, 0 otherwise - */ -static int -networkActive(void) { - unsigned int i; - int active = 0; - - if (!driverState) - return 0; - - networkDriverLock(driverState); - for (i = 0 ; i < driverState->networks.count ; i++) { - virNetworkObjPtr net = driverState->networks.objs[i]; - virNetworkObjLock(net); - if (virNetworkObjIsActive(net)) - active = 1; - virNetworkObjUnlock(net); - } - networkDriverUnlock(driverState); - return active; -} /** * networkShutdown: @@ -3307,7 +3283,6 @@ static virStateDriver networkStateDriver = { .initialize = networkStartup, .cleanup = networkShutdown, .reload = networkReload, - .active = networkActive, }; int networkRegister(void) { diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 953e1d3..080aaed 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -586,9 +586,9 @@ static void device_prop_modified(LibHalContext *ctx ATTRIBUTE_UNUSED, } - - -static int halDeviceMonitorStartup(bool privileged ATTRIBUTE_UNUSED) +static int halDeviceMonitorStartup(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { LibHalContext *hal_ctx = NULL; char **udi = NULL; @@ -736,13 +736,6 @@ static int halDeviceMonitorReload(void) } -static int halDeviceMonitorActive(void) -{ - /* Always ready to deal with a shutdown */ - return 0; -} - - static virDrvOpenStatus halNodeDrvOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -786,7 +779,6 @@ static virStateDriver halStateDriver = { .initialize = halDeviceMonitorStartup, /* 0.5.0 */ .cleanup = halDeviceMonitorShutdown, /* 0.5.0 */ .reload = halDeviceMonitorReload, /* 0.5.0 */ - .active = halDeviceMonitorActive, /* 0.5.0 */ }; int halNodeRegister(void) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index bc4e2e9..c9ca00c 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1604,7 +1604,9 @@ out: return ret; } -static int udevDeviceMonitorStartup(bool privileged ATTRIBUTE_UNUSED) +static int udevDeviceMonitorStartup(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { udevPrivate *priv = NULL; struct udev *udev = NULL; @@ -1723,13 +1725,6 @@ static int udevDeviceMonitorReload(void) } -static int udevDeviceMonitorActive(void) -{ - /* Always ready to deal with a shutdown */ - return 0; -} - - static virDrvOpenStatus udevNodeDrvOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -1772,7 +1767,6 @@ static virStateDriver udevStateDriver = { .initialize = udevDeviceMonitorStartup, /* 0.7.3 */ .cleanup = udevDeviceMonitorShutdown, /* 0.7.3 */ .reload = udevDeviceMonitorReload, /* 0.7.3 */ - .active = udevDeviceMonitorActive, /* 0.7.3 */ }; int udevNodeRegister(void) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index a0ee4f1..cc81914 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -165,7 +165,9 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) * Initialization function for the QEmu daemon */ static int -nwfilterDriverStartup(bool privileged) +nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { char *base = NULL; DBusConnection *sysbus = NULL; @@ -305,27 +307,6 @@ nwfilterDriverReload(void) { return 0; } -/** - * virNWFilterActive: - * - * Checks if the nwfilter driver is active, i.e. has an active nwfilter - * - * Returns 1 if active, 0 otherwise - */ -static int -nwfilterDriverActive(void) { - int ret; - - if (!driverState) - return 0; - - nwfilterDriverLock(driverState); - ret = driverState->nwfilters.count ? 1 : 0; - ret |= driverState->watchingFirewallD; - nwfilterDriverUnlock(driverState); - - return ret; -} /** * virNWFilterIsWatchingFirewallD: @@ -696,7 +677,6 @@ static virStateDriver stateDriver = { .initialize = nwfilterDriverStartup, .cleanup = nwfilterDriverShutdown, .reload = nwfilterDriverReload, - .active = nwfilterDriverActive, }; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index b7378a7..c55a0c0 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -70,6 +70,10 @@ struct qemud_driver { int cgroupControllers; char **cgroupDeviceACL; + size_t nactive; + virStateInhibitCallback inhibitCallback; + void *inhibitOpaque; + virDomainObjList domains; /* These four directories are ones libvirtd uses (so must be root:root diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 186b79f..655055d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -610,7 +610,10 @@ qemuDomainFindMaxID(void *payload, * Initialization function for the QEmu daemon */ static int -qemuStartup(bool privileged) { +qemuStartup(bool privileged, + virStateInhibitCallback callback, + void *opaque) +{ char *base = NULL; char *driverConf = NULL; int rc; @@ -631,6 +634,8 @@ qemuStartup(bool privileged) { qemu_driver->privileged = privileged; qemu_driver->uri = privileged ? "qemu:///system" : "qemu:///session"; + qemu_driver->inhibitCallback = callback; + qemu_driver->inhibitOpaque = opaque; /* Don't have a dom0 so start from 1 */ qemu_driver->nextvmid = 1; @@ -972,28 +977,6 @@ qemuReload(void) { return 0; } -/** - * qemuActive: - * - * Checks if the QEmu daemon is active, i.e. has an active domain or - * an active network - * - * Returns 1 if active, 0 otherwise - */ -static int -qemuActive(void) { - int active = 0; - - if (!qemu_driver) - return 0; - - /* XXX having to iterate here is not great because it requires many locks */ - qemuDriverLock(qemu_driver); - active = virDomainObjListNumOfDomains(&qemu_driver->domains, 1); - qemuDriverUnlock(qemu_driver); - return active; -} - /* * qemuStop: @@ -14975,7 +14958,6 @@ static virStateDriver qemuStateDriver = { .initialize = qemuStartup, .cleanup = qemuShutdown, .reload = qemuReload, - .active = qemuActive, .stop = qemuStop, }; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index cdaa2df..de15494 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3228,6 +3228,10 @@ qemuProcessReconnect(void *opaque) goto error; } + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + endjob: if (!qemuDomainObjEndJob(driver, obj)) obj = NULL; @@ -3436,6 +3440,10 @@ int qemuProcessStart(virConnectPtr conn, qemuDomainSetFakeReboot(driver, vm, false); virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN); + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + /* Run an early hook to set-up missing devices */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); @@ -4002,6 +4010,10 @@ void qemuProcessStop(struct qemud_driver *driver, */ vm->def->id = -1; + driver->nactive--; + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(false, driver->inhibitOpaque); + if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) { /* To not break the normal domain shutdown process, skip the * timestamp log writing if failed on opening log file. */ @@ -4233,6 +4245,10 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, vm->def->id = driver->nextvmid++; + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + if (virFileMakePath(driver->logDir) < 0) { virReportSystemError(errno, _("cannot create log directory %s"), diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 01cebb4..7ef69d6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -150,7 +150,9 @@ static char *get_transport_from_scheme(char *scheme); #ifdef WITH_LIBVIRTD static int -remoteStartup(bool privileged ATTRIBUTE_UNUSED) +remoteStartup(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { /* Mark that we're inside the daemon so we can avoid * re-entering ourselves diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index cf15240..b108399 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -101,8 +101,7 @@ struct _virNetServer { virNetTLSContextPtr tls; unsigned int autoShutdownTimeout; - virNetServerAutoShutdownFunc autoShutdownFunc; - void *autoShutdownOpaque; + size_t autoShutdownInhibitions; virNetServerClientPrivNew clientPrivNew; virNetServerClientPrivPreExecRestart clientPrivPreExecRestart; @@ -707,19 +706,33 @@ bool virNetServerIsPrivileged(virNetServerPtr srv) void virNetServerAutoShutdown(virNetServerPtr srv, - unsigned int timeout, - virNetServerAutoShutdownFunc func, - void *opaque) + unsigned int timeout) { virNetServerLock(srv); srv->autoShutdownTimeout = timeout; - srv->autoShutdownFunc = func; - srv->autoShutdownOpaque = opaque; virNetServerUnlock(srv); } + +void virNetServerAddShutdownInhibition(virNetServerPtr srv) +{ + virNetServerLock(srv); + srv->autoShutdownInhibitions++; + virNetServerUnlock(srv); +} + + +void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) +{ + virNetServerLock(srv); + srv->autoShutdownInhibitions--; + virNetServerUnlock(srv); +} + + + static sig_atomic_t sigErrors = 0; static int sigLastErrno = 0; static int sigWrite = -1; @@ -932,7 +945,7 @@ static void virNetServerAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED, virNetServerLock(srv); - if (srv->autoShutdownFunc(srv, srv->autoShutdownOpaque)) { + if (!srv->autoShutdownInhibitions) { VIR_DEBUG("Automatic shutdown triggered"); srv->quit = 1; } diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index ff2dc94..38cccfe 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -60,9 +60,10 @@ typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque); bool virNetServerIsPrivileged(virNetServerPtr srv); void virNetServerAutoShutdown(virNetServerPtr srv, - unsigned int timeout, - virNetServerAutoShutdownFunc func, - void *opaque); + unsigned int timeout); + +void virNetServerAddShutdownInhibition(virNetServerPtr srv); +void virNetServerRemoveShutdownInhibition(virNetServerPtr srv); typedef void (*virNetServerSignalFunc)(virNetServerPtr srv, siginfo_t *info, void *opaque); diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index c7aabfc..d9ba42b 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -1073,7 +1073,9 @@ secretDriverCleanup(void) } static int -secretDriverStartup(bool privileged) +secretDriverStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { char *base = NULL; @@ -1166,7 +1168,6 @@ static virStateDriver stateDriver = { .initialize = secretDriverStartup, .cleanup = secretDriverCleanup, .reload = secretDriverReload, - .active = NULL /* All persistent state is immediately saved to disk */ }; int diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index d27bb41..3cb2312 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -128,7 +128,9 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { * Initialization function for the QEmu daemon */ static int -storageDriverStartup(bool privileged) +storageDriverStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { char *base = NULL; @@ -202,33 +204,6 @@ storageDriverReload(void) { return 0; } -/** - * virStorageActive: - * - * Checks if the storage driver is active, i.e. has an active pool - * - * Returns 1 if active, 0 otherwise - */ -static int -storageDriverActive(void) { - unsigned int i; - int active = 0; - - if (!driverState) - return 0; - - storageDriverLock(driverState); - - for (i = 0 ; i < driverState->pools.count ; i++) { - virStoragePoolObjLock(driverState->pools.objs[i]); - if (virStoragePoolObjIsActive(driverState->pools.objs[i])) - active = 1; - virStoragePoolObjUnlock(driverState->pools.objs[i]); - } - - storageDriverUnlock(driverState); - return active; -} /** * virStorageShutdown: @@ -2445,7 +2420,6 @@ static virStateDriver stateDriver = { .initialize = storageDriverStartup, .cleanup = storageDriverShutdown, .reload = storageDriverReload, - .active = storageDriverActive, }; int storageRegister(void) { diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h index ebae24e..9bddedc 100644 --- a/src/uml/uml_conf.h +++ b/src/uml/uml_conf.h @@ -45,11 +45,14 @@ struct uml_driver { virMutex lock; bool privileged; + virStateInhibitCallback inhibitCallback; + void *inhibitOpaque; unsigned long umlVersion; int nextvmid; virDomainObjList domains; + size_t nactive; char *configDir; char *autostartDir; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 5a87e31..7eedaf1 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -372,6 +372,11 @@ reread: } dom->def->id = driver->nextvmid++; + + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + driver->nactive++; + virDomainObjSetState(dom, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); @@ -419,7 +424,9 @@ cleanup: * Initialization function for the Uml daemon */ static int -umlStartup(bool privileged) +umlStartup(bool privileged, + virStateInhibitCallback callback, + void *opaque) { char *base = NULL; char *userdir = NULL; @@ -428,6 +435,8 @@ umlStartup(bool privileged) return -1; uml_driver->privileged = privileged; + uml_driver->inhibitCallback = callback; + uml_driver->inhibitOpaque = opaque; if (virMutexInit(¨_driver->lock) < 0) { VIR_FREE(uml_driver); @@ -585,27 +594,6 @@ umlReload(void) { return 0; } -/** - * umlActive: - * - * Checks if the Uml daemon is active, i.e. has an active domain or - * an active network - * - * Returns 1 if active, 0 otherwise - */ -static int -umlActive(void) { - int active = 0; - - if (!uml_driver) - return 0; - - umlDriverLock(uml_driver); - active = virDomainObjListNumOfDomains(¨_driver->domains, 1); - umlDriverUnlock(uml_driver); - - return active; -} static void umlShutdownOneVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -1154,6 +1142,10 @@ static void umlShutdownVMDaemon(struct uml_driver *driver, vm->def->id = -1; vm->newDef = NULL; } + + driver->nactive--; + if (!driver->nactive && driver->inhibitCallback) + driver->inhibitCallback(false, driver->inhibitOpaque); } @@ -2634,7 +2626,6 @@ static virStateDriver umlStateDriver = { .initialize = umlStartup, .cleanup = umlShutdown, .reload = umlReload, - .active = umlActive, }; int umlRegister(void) { diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 5a40757..15e760d 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -201,7 +201,9 @@ done: #ifdef WITH_LIBVIRTD static int -xenInitialize(bool privileged ATTRIBUTE_UNUSED) +xenInitialize(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { inside_daemon = true; return 0; -- 1.7.11.7
Currently to deal with auto-shutdown libvirtd must periodically poll all stateful drivers. Thus sucks because it requires acquiring both the driver lock and locks on every single virtual machine. Instead pass in a "inhibit" callback to virStateInitialize which drivers can invoke whenever they want to inhibit shutdown due to existance of active VMs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
@@ -772,6 +762,18 @@ static int daemonSetupSignals(virNetServerPtr srv) return 0; }
+ +static void daemonInhibitCallback(bool inhibit, void *opaque) +{ + virNetServerPtr srv = opaque; + + if (inhibit) + virNetServerAddShutdownInhibition(srv); + else + virNetServerRemoveShutdownInhibition(srv); +}
Nice. Is the intent that drivers call this once on the first guest to start, and again on the last guest stopped, or once on every single guest start/stop action? Either way, it seems like the inhibition has to be reference counted to deal with more than one driver having a reason for inhibition among a single libvirtd service.
+++ b/src/nwfilter/nwfilter_driver.c @@ -165,7 +165,9 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) * Initialization function for the QEmu daemon */ static int -nwfilterDriverStartup(bool privileged) +nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) {
Here, you aren't remembering the callback...
char *base = NULL; DBusConnection *sysbus = NULL; @@ -305,27 +307,6 @@ nwfilterDriverReload(void) { return 0; }
-/** - * virNWFilterActive: - * - * Checks if the nwfilter driver is active, i.e. has an active nwfilter - * - * Returns 1 if active, 0 otherwise - */ -static int -nwfilterDriverActive(void) { - int ret; - - if (!driverState) - return 0; - - nwfilterDriverLock(driverState); - ret = driverState->nwfilters.count ? 1 : 0; - ret |= driverState->watchingFirewallD; - nwfilterDriverUnlock(driverState); - - return ret;
But the old code could inhibit shutdown if a nwfilter was active. Is this intentional?
+ +void virNetServerAddShutdownInhibition(virNetServerPtr srv) +{ + virNetServerLock(srv); + srv->autoShutdownInhibitions++; + virNetServerUnlock(srv); +} + + +void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) +{ + virNetServerLock(srv); + srv->autoShutdownInhibitions--; + virNetServerUnlock(srv); +}
Do these have to obtain full-blown locks, or can you use atomic increments outside of any other locking?
static int -storageDriverStartup(bool privileged) +storageDriverStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED)
Another case of ignoring the callback...
{ char *base = NULL;
@@ -202,33 +204,6 @@ storageDriverReload(void) { return 0; }
-/** - * virStorageActive: - * - * Checks if the storage driver is active, i.e. has an active pool - * - * Returns 1 if active, 0 otherwise - */ -static int -storageDriverActive(void) { - unsigned int i; - int active = 0; - - if (!driverState) - return 0; - - storageDriverLock(driverState); - - for (i = 0 ; i < driverState->pools.count ; i++) { - virStoragePoolObjLock(driverState->pools.objs[i]); - if (virStoragePoolObjIsActive(driverState->pools.objs[i])) - active = 1; - virStoragePoolObjUnlock(driverState->pools.objs[i]); - } - - storageDriverUnlock(driverState); - return active;
...where the old code could inhibit shutdown. Intentional? Overall, the idea is nice, but answers to my questions will determine whether you need a v2.
On Tue, Nov 27, 2012 at 03:53:48PM -0500, Eric Blake wrote:
Currently to deal with auto-shutdown libvirtd must periodically poll all stateful drivers. Thus sucks because it requires acquiring both the driver lock and locks on every single virtual machine. Instead pass in a "inhibit" callback to virStateInitialize which drivers can invoke whenever they want to inhibit shutdown due to existance of active VMs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
@@ -772,6 +762,18 @@ static int daemonSetupSignals(virNetServerPtr srv) return 0; }
+ +static void daemonInhibitCallback(bool inhibit, void *opaque) +{ + virNetServerPtr srv = opaque; + + if (inhibit) + virNetServerAddShutdownInhibition(srv); + else + virNetServerRemoveShutdownInhibition(srv); +}
Nice. Is the intent that drivers call this once on the first guest to start, and again on the last guest stopped, or once on every single guest start/stop action? Either way, it seems like the inhibition has to be reference counted to deal with more than one driver having a reason for inhibition among a single libvirtd service.
The only real requirement is that the drivers have a matching number of calls to turn it on/off. The way I've coded things though, the drivers only call it in first guest start, and last guest stop. virNetServer does indeed do reference counting.
+++ b/src/nwfilter/nwfilter_driver.c @@ -165,7 +165,9 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) * Initialization function for the QEmu daemon */ static int -nwfilterDriverStartup(bool privileged) +nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) {
Here, you aren't remembering the callback...
Yes, this is technically a semantic change, I could have pulled into a separate patch. Basically there is no compelling reason for the nwfilter driver to inhibit shutdown. Active NWfilters are all associated with active domains, which will already be inhibiting shutdown.
+ +void virNetServerAddShutdownInhibition(virNetServerPtr srv) +{ + virNetServerLock(srv); + srv->autoShutdownInhibitions++; + virNetServerUnlock(srv); +} + + +void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) +{ + virNetServerLock(srv); + srv->autoShutdownInhibitions--; + virNetServerUnlock(srv); +}
Do these have to obtain full-blown locks, or can you use atomic increments outside of any other locking?
In the next method, these functions grow more code, so the locks are actually protecting something reasonable
static int -storageDriverStartup(bool privileged) +storageDriverStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED)
Another case of ignoring the callback...
{ char *base = NULL;
@@ -202,33 +204,6 @@ storageDriverReload(void) { return 0; }
-/** - * virStorageActive: - * - * Checks if the storage driver is active, i.e. has an active pool - * - * Returns 1 if active, 0 otherwise - */ -static int -storageDriverActive(void) { - unsigned int i; - int active = 0; - - if (!driverState) - return 0; - - storageDriverLock(driverState); - - for (i = 0 ; i < driverState->pools.count ; i++) { - virStoragePoolObjLock(driverState->pools.objs[i]); - if (virStoragePoolObjIsActive(driverState->pools.objs[i])) - active = 1; - virStoragePoolObjUnlock(driverState->pools.objs[i]); - } - - storageDriverUnlock(driverState); - return active;
...where the old code could inhibit shutdown. Intentional?
Again this is because, IMHO, there is no compelling reason for active storage pools to inhibit libvirtd shutdown. Indeed inhibiting it makes auto-shutdown pretty much useless, since you'll almost always have an active directory based storage pool. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Currently to deal with auto-shutdown libvirtd must periodically poll all stateful drivers. Thus sucks because it requires acquiring both the driver lock and locks on every single virtual machine. Instead pass in a "inhibit" callback to virStateInitialize which drivers can invoke whenever they want to inhibit shutdown due to existance of active VMs.
+nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) {
Here, you aren't remembering the callback...
Yes, this is technically a semantic change, I could have pulled into a separate patch.
Basically there is no compelling reason for the nwfilter driver to inhibit shutdown. Active NWfilters are all associated with active domains, which will already be inhibiting shutdown.
True.
+storageDriverStartup(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED)
Another case of ignoring the callback...
Again this is because, IMHO, there is no compelling reason for active storage pools to inhibit libvirtd shutdown. Indeed inhibiting it makes auto-shutdown pretty much useless, since you'll almost always have an active directory based storage pool.
Probably worth mentioning these intentional changes in the commit log, but you've convinced me that the code is sane. ACK.
From: "Daniel P. Berrange" <berrange@redhat.com> Use the freedesktop inhibition DBus service to prevent host shutdown or session logout while any VMs are running. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index b108399..aaf98a9 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -27,6 +27,10 @@ #include <string.h> #include <fcntl.h> +#ifdef HAVE_DBUS +# include <dbus/dbus.h> +#endif + #include "virnetserver.h" #include "logging.h" #include "memory.h" @@ -37,6 +41,7 @@ #include "virfile.h" #include "event.h" #include "virnetservermdns.h" +#include "virdbus.h" #ifndef SA_SIGINFO # define SA_SIGINFO 0 @@ -102,6 +107,8 @@ struct _virNetServer { unsigned int autoShutdownTimeout; size_t autoShutdownInhibitions; + bool autoShutdownCallingInhibit; + int autoShutdownInhibitFd; virNetServerClientPrivNew clientPrivNew; virNetServerClientPrivPreExecRestart clientPrivPreExecRestart; @@ -391,6 +398,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->clientPrivFree = clientPrivFree; srv->clientPrivOpaque = clientPrivOpaque; srv->privileged = geteuid() == 0 ? true : false; + srv->autoShutdownInhibitFd = -1; if (mdnsGroupName && !(srv->mdnsGroupName = strdup(mdnsGroupName))) { @@ -716,10 +724,104 @@ void virNetServerAutoShutdown(virNetServerPtr srv, } +#ifdef HAVE_DBUS +static void virNetServerGotInhibitReply(DBusPendingCall *pending, + void *opaque) +{ + virNetServerPtr srv = opaque; + DBusMessage *reply; + int fd; + + virNetServerLock(srv); + srv->autoShutdownCallingInhibit = false; + + VIR_DEBUG("srv=%p", srv); + + reply = dbus_pending_call_steal_reply(pending); + if (reply == NULL) + goto cleanup; + + if (dbus_message_get_args(reply, NULL, + DBUS_TYPE_UNIX_FD, &fd, + DBUS_TYPE_INVALID)) { + if (srv->autoShutdownInhibitions) { + srv->autoShutdownInhibitFd = fd; + } else { + /* We stopped the last VM since we made the inhibit call */ + VIR_FORCE_CLOSE(fd); + } + } + dbus_message_unref(reply); + +cleanup: + virNetServerUnlock(srv); +} + + +/* As per: http://www.freedesktop.org/wiki/Software/systemd/inhibit */ +static void virNetServerCallInhibit(virNetServerPtr srv, + const char *what, + const char *who, + const char *why, + const char *mode) +{ + DBusMessage *message; + DBusPendingCall *pendingReply; + DBusConnection *systemBus; + + VIR_DEBUG("srv=%p what=%s who=%s why=%s mode=%s", + srv, NULLSTR(what), NULLSTR(who), NULLSTR(why), NULLSTR(mode)); + + if (!(systemBus = virDBusGetSystemBus())) + return; + + /* Only one outstanding call at a time */ + if (srv->autoShutdownCallingInhibit) + return; + + message = dbus_message_new_method_call("org.freedesktop.login1", + "/org/freedesktop/login1", + "org.freedesktop.login1.Manager", + "Inhibit"); + if (message == NULL) + return; + + dbus_message_append_args(message, + DBUS_TYPE_STRING, &what, + DBUS_TYPE_STRING, &who, + DBUS_TYPE_STRING, &why, + DBUS_TYPE_STRING, &mode, + DBUS_TYPE_INVALID); + + pendingReply = NULL; + if (dbus_connection_send_with_reply(systemBus, message, + &pendingReply, + 25*1000)) { + dbus_pending_call_set_notify(pendingReply, + virNetServerGotInhibitReply, + srv, NULL); + srv->autoShutdownCallingInhibit = true; + } + dbus_message_unref(message); +} +#endif + void virNetServerAddShutdownInhibition(virNetServerPtr srv) { virNetServerLock(srv); srv->autoShutdownInhibitions++; + + VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions); + +#ifdef HAVE_DBUS + if (srv->autoShutdownInhibitions == 1) + virNetServerCallInhibit(srv, + "shutdown", + _("Libvirt"), + _("Virtual machines need to be saved"), + "delay"); +#endif + virNetServerUnlock(srv); } @@ -728,6 +830,12 @@ void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) { virNetServerLock(srv); srv->autoShutdownInhibitions--; + + VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions); + + if (srv->autoShutdownInhibitions == 0) + VIR_FORCE_CLOSE(srv->autoShutdownInhibitFd); + virNetServerUnlock(srv); } @@ -1065,6 +1173,8 @@ void virNetServerDispose(void *obj) virNetServerPtr srv = obj; int i; + VIR_FORCE_CLOSE(srv->autoShutdownInhibitFd); + for (i = 0 ; i < srv->nservices ; i++) virNetServerServiceToggle(srv->services[i], false); -- 1.7.11.7
Use the freedesktop inhibition DBus service to prevent host shutdown or session logout while any VMs are running.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)
+++ b/src/rpc/virnetserver.c @@ -27,6 +27,10 @@ #include <string.h> #include <fcntl.h>
+#ifdef HAVE_DBUS +# include <dbus/dbus.h>
Do we really need this header...
+#endif + #include "virnetserver.h" #include "logging.h" #include "memory.h" @@ -37,6 +41,7 @@ #include "virfile.h" #include "event.h" #include "virnetservermdns.h" +#include "virdbus.h"
...or is this local header sufficient? (That is, should you rework this patch to put the raw dbus_* calls isolated into virdbus.[ch], rathar than having this file have to use conditional compilation)?
@@ -391,6 +398,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->clientPrivFree = clientPrivFree; srv->clientPrivOpaque = clientPrivOpaque; srv->privileged = geteuid() == 0 ? true : false;
Pre-existing, but I find 'some_bool_expr ? true : false' to be overkill, compared to 'some_bool_expr'.
+ +/* As per: http://www.freedesktop.org/wiki/Software/systemd/inhibit */ +static void virNetServerCallInhibit(virNetServerPtr srv, + const char *what, + const char *who, + const char *why, + const char *mode) +{ + DBusMessage *message; + DBusPendingCall *pendingReply; + DBusConnection *systemBus; + + VIR_DEBUG("srv=%p what=%s who=%s why=%s mode=%s",
syntax-check didn't complain about mode=%s? (I know it complains about mode=%d, and favors mode=%o instead; but I guess a string mode should not be an error).
void virNetServerAddShutdownInhibition(virNetServerPtr srv) { virNetServerLock(srv); srv->autoShutdownInhibitions++; + + VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions);
Should this debug line be hoisted into the earlier patch that introduced inhibition callbacks?
@@ -728,6 +830,12 @@ void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) { virNetServerLock(srv); srv->autoShutdownInhibitions--; + + VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions);
Again, should this debug be hoisted into an earlier patch? Overall, looks pretty slick!
On Tue, Nov 27, 2012 at 04:20:49PM -0500, Eric Blake wrote:
Use the freedesktop inhibition DBus service to prevent host shutdown or session logout while any VMs are running.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)
+++ b/src/rpc/virnetserver.c @@ -27,6 +27,10 @@ #include <string.h> #include <fcntl.h>
+#ifdef HAVE_DBUS +# include <dbus/dbus.h>
Do we really need this header...
No, its obsolete.
+#endif + #include "virnetserver.h" #include "logging.h" #include "memory.h" @@ -37,6 +41,7 @@ #include "virfile.h" #include "event.h" #include "virnetservermdns.h" +#include "virdbus.h"
...or is this local header sufficient? (That is, should you rework this patch to put the raw dbus_* calls isolated into virdbus.[ch], rathar than having this file have to use conditional compilation)?
Correct.
void virNetServerAddShutdownInhibition(virNetServerPtr srv) { virNetServerLock(srv); srv->autoShutdownInhibitions++; + + VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions);
Should this debug line be hoisted into the earlier patch that introduced inhibition callbacks?
@@ -728,6 +830,12 @@ void virNetServerRemoveShutdownInhibition(virNetServerPtr srv) { virNetServerLock(srv); srv->autoShutdownInhibitions--; + + VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions);
Again, should this debug be hoisted into an earlier patch?
I don't think it really matters - this patch is the first time that it is interesting to see the debug info :-) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Use the freedesktop inhibition DBus service to prevent host shutdown or session logout while any VMs are running.
+#ifdef HAVE_DBUS +# include <dbus/dbus.h>
Do we really need this header...
No, its obsolete.
Well, I _did_ see some dbus_ calls in this file: +#ifdef HAVE_DBUS +static void virNetServerGotInhibitReply(DBusPendingCall *pending, + void *opaque) +{ + + reply = dbus_pending_call_steal_reply(pending);
...or is this local header sufficient? (That is, should you rework this patch to put the raw dbus_* calls isolated into virdbus.[ch], rathar than having this file have to use conditional compilation)?
Correct.
So I'm assuming here I should wait for you to post a v2 that actually does this refactoring.
+ VIR_DEBUG("srv=%p inhibitions=%zu", srv, srv->autoShutdownInhibitions);
Again, should this debug be hoisted into an earlier patch?
I don't think it really matters - this patch is the first time that it is interesting to see the debug info :-)
Fair enough on this point, though :)
From: Alexander Larsson <alexl@redhat.com> When the session dies or when the system is going to be shut down we issue a virStateStop() call to instruct drivers to prepare to be stopped. This will remove any previously acquire inhibitions. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index f7046f6..4fc065c 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -98,6 +98,11 @@ #include "configmake.h" +#ifdef HAVE_DBUS +# include <dbus/dbus.h> +# include "virdbus.h" +#endif + #if HAVE_SASL virNetSASLContextPtr saslCtxt = NULL; #endif @@ -774,6 +779,65 @@ static void daemonInhibitCallback(bool inhibit, void *opaque) } +#ifdef HAVE_DBUS +static DBusConnection *sessionBus; +static DBusConnection *systemBus; + +static void daemonStopWorker(void *opaque) +{ + virNetServerPtr srv = opaque; + + VIR_DEBUG("Begin stop srv=%p", srv); + + ignore_value(virStateStop()); + + VIR_DEBUG("Completed stop srv=%p", srv); + + /* Exit libvirtd cleanly */ + virNetServerQuit(srv); +} + + +/* We do this in a thread to not block the main loop */ +static void daemonStop(virNetServerPtr srv) +{ + virThread thr; + virObjectRef(srv); + if (virThreadCreate(&thr, false, daemonStopWorker, srv) < 0) + virObjectUnref(srv); +} + + +static DBusHandlerResult handleSessionMessageFunc(DBusConnection *connection ATTRIBUTE_UNUSED, + DBusMessage *message, + void *opaque) +{ + virNetServerPtr srv = opaque; + + VIR_DEBUG("srv=%p", srv); + + if (dbus_message_is_signal(message, DBUS_INTERFACE_LOCAL, "Disconnected")) + daemonStop(srv); + + return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; +} + +static DBusHandlerResult handleSystemMessageFunc(DBusConnection *connection ATTRIBUTE_UNUSED, + DBusMessage *message, + void *opaque) +{ + virNetServerPtr srv = opaque; + + VIR_DEBUG("srv=%p", srv); + + if (dbus_message_is_signal(message, "org.freedesktop.login1.Manager", "PrepareForShutdown")) + daemonStop(srv); + + return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; +} +#endif + + static void daemonRunStateInit(void *opaque) { virNetServerPtr srv = opaque; @@ -792,6 +856,26 @@ static void daemonRunStateInit(void *opaque) return; } +#ifdef HAVE_DBUS + /* Tie the non-priviledged libvirtd to the session/shutdown lifecycle */ + if (!virNetServerIsPrivileged(srv)) { + + sessionBus = virDBusGetSessionBus(); + if (sessionBus != NULL) + dbus_connection_add_filter(sessionBus, + handleSessionMessageFunc, srv, NULL); + + systemBus = virDBusGetSystemBus(); + if (systemBus != NULL) { + dbus_connection_add_filter(systemBus, + handleSystemMessageFunc, srv, NULL); + dbus_bus_add_match(systemBus, + "type='signal',sender='org.freedesktop.login1', interface='org.freedesktop.login1.Manager'", + NULL); + } + } +#endif + /* Only now accept clients from network */ virNetServerUpdateServices(srv, true); virObjectUnref(srv); -- 1.7.11.7
When the session dies or when the system is going to be shut down we issue a virStateStop() call to instruct drivers to prepare to be stopped. This will remove any previously acquire inhibitions.
s/acquire/acquired/
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)
+++ b/daemon/libvirtd.c @@ -98,6 +98,11 @@
#include "configmake.h"
+#ifdef HAVE_DBUS +# include <dbus/dbus.h>
Again, is this necessary,
+# include "virdbus.h"
or should we be hiding the interface into virdbus.h, which can be unconditionally included?
+static void daemonStopWorker(void *opaque) +{ + virNetServerPtr srv = opaque; + + VIR_DEBUG("Begin stop srv=%p", srv); + + ignore_value(virStateStop()); + + VIR_DEBUG("Completed stop srv=%p", srv);
I think the VIR_DEBUG should log the return value of virStateStop(), rather than blindly ignoring the possibility of a failed shutdown.
+static DBusHandlerResult handleSessionMessageFunc(DBusConnection *connection ATTRIBUTE_UNUSED,
Long lines; could be shortened by using: static DBusHandlerResult handleSessionMessageFunc(...) Again, looks slick; and my objection earlier in the series about qemu:///system vs. libvirt-guests init script may have been premature, seeing as how you are tying this inhibition handling only to sessions. Still, I'd be interested in your responses to my questions before granting ack.
On Tue, Nov 27, 2012 at 04:31:16PM -0500, Eric Blake wrote:
When the session dies or when the system is going to be shut down we issue a virStateStop() call to instruct drivers to prepare to be stopped. This will remove any previously acquire inhibitions.
s/acquire/acquired/
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)
+++ b/daemon/libvirtd.c @@ -98,6 +98,11 @@
#include "configmake.h"
+#ifdef HAVE_DBUS +# include <dbus/dbus.h>
Again, is this necessary,
+# include "virdbus.h"
or should we be hiding the interface into virdbus.h, which can be unconditionally included?
Correct, it is bogus
+static void daemonStopWorker(void *opaque) +{ + virNetServerPtr srv = opaque; + + VIR_DEBUG("Begin stop srv=%p", srv); + + ignore_value(virStateStop()); + + VIR_DEBUG("Completed stop srv=%p", srv);
I think the VIR_DEBUG should log the return value of virStateStop(), rather than blindly ignoring the possibility of a failed shutdown.
Good idea.
+static DBusHandlerResult handleSessionMessageFunc(DBusConnection *connection ATTRIBUTE_UNUSED,
Long lines; could be shortened by using:
static DBusHandlerResult handleSessionMessageFunc(...)
Again, looks slick; and my objection earlier in the series about qemu:///system vs. libvirt-guests init script may have been premature, seeing as how you are tying this inhibition handling only to sessions. Still, I'd be interested in your responses to my questions before granting ack.
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
When the session dies or when the system is going to be shut down we issue a virStateStop() call to instruct drivers to prepare to be stopped. This will remove any previously acquire inhibitions.
+#ifdef HAVE_DBUS +# include <dbus/dbus.h>
Again, is this necessary,
+# include "virdbus.h"
or should we be hiding the interface into virdbus.h, which can be unconditionally included?
Correct, it is bogus
But that still raises the question of whether this file should be making raw dbus_* calls, or whether that should be factored into virdbus.h.
Again, looks slick; and my objection earlier in the series about qemu:///system vs. libvirt-guests init script may have been premature, seeing as how you are tying this inhibition handling only to sessions. Still, I'd be interested in your responses to my questions before granting ack.
I think at this point I have given ack to 1-8, and am waiting to see if you plan a v2 for patches 9 and 10 to push more raw dbus_* actions into virdbus.c.
participants (2)
-
Daniel P. Berrange -
Eric Blake