- Devel - libvirt List Archives

[libvirt] [PATCH v3] qemu: qemuDomainRename and virDomainObjListNumOfDomains ABBA deadlock fix
by Nikolay Shirokovskiy 03 Feb '16

03 Feb '16

A pretty nasty deadlock occurs while trying to rename a VM in parallel with virDomainObjListNumOfDomains. The short description of the problem is as follows: Thread #1: qemuDomainRename: ------> aquires domain lock by qemuDomObjFromDomain ---------> waits for domain list lock in any of the listed functions: - virDomainObjListFindByName - virDomainObjListRenameAddNew - virDomainObjListRenameRemove Thread #2: virDomainObjListNumOfDomains: ------> aquires domain list lock ---------> waits for domain lock in virDomainObjListCount Introduce generic virDomainObjListRename function for renaming domains. It aquires list lock in right order to avoid deadlock. Callback is used to make driver specific domain updates. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/conf/virdomainobjlist.c | 98 +++++++++++++++++++----------- src/conf/virdomainobjlist.h | 16 +++-- src/libvirt_private.syms | 3 +- src/qemu/qemu_driver.c | 142 ++++++++++++++++++++------------------------ 4 files changed, 142 insertions(+), 117 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 7568f93..1f7ddaf 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -313,40 +313,6 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, } -int -virDomainObjListRenameAddNew(virDomainObjListPtr doms, - virDomainObjPtr vm, - const char *name) -{ - int ret = -1; - virObjectLock(doms); - - /* Add new name into the hash table of domain names. */ - if (virHashAddEntry(doms->objsName, name, vm) < 0) - goto cleanup; - - /* Okay, this is crazy. virHashAddEntry() does not increment - * the refcounter of @vm, but virHashRemoveEntry() does - * decrement it. We need to work around it. */ - virObjectRef(vm); - - ret = 0; - cleanup: - virObjectUnlock(doms); - return ret; -} - - -int -virDomainObjListRenameRemove(virDomainObjListPtr doms, const char *name) -{ - virObjectLock(doms); - virHashRemoveEntry(doms->objsName, name); - virObjectUnlock(doms); - return 0; -} - - /* * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else @@ -372,6 +338,70 @@ void virDomainObjListRemove(virDomainObjListPtr doms, } +/* + * The caller must hold a lock on dom. Callbacks should not sleep/wait othewise + * operations on all domains will be blocked as the callback is called with + * domains lock hold. Domain lock is dropped/reaquired during this operation + * thus domain consistency must not rely on this lock solely. + */ + +int +virDomainObjListRename(virDomainObjListPtr doms, + virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + virDomainObjListRenameCallback callback, + void *opaque) +{ + int ret = -1; + char *old_name = NULL; + int rc; + + /* doms and dom locks must be attained in right order thus relock dom. */ + /* dom reference is touched for the benefit of those callers that + * hold a lock on dom but not refcount it. */ + virObjectRef(dom); + virObjectUnlock(dom); + virObjectLock(doms); + virObjectLock(dom); + virObjectUnref(dom); + + if (STREQ(dom->def->name, new_name)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Can't rename domain to itself")); + goto cleanup; + } + + if (virHashLookup(doms->objsName, new_name) != NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain with name '%s' already exists"), + new_name); + goto cleanup; + } + + if (VIR_STRDUP(old_name, dom->def->name) < 0) + goto cleanup; + + if (virHashAddEntry(doms->objsName, new_name, dom) < 0) + goto cleanup; + + /* Okay, this is crazy. virHashAddEntry() does not increment + * the refcounter of @dom, but virHashRemoveEntry() does + * decrement it. We need to work around it. */ + virObjectRef(dom); + + rc = callback(dom, new_name, flags, opaque); + virHashRemoveEntry(doms->objsName, rc < 0 ? new_name : old_name); + if (rc < 0) + goto cleanup; + + ret = 0; + cleanup: + virObjectUnlock(doms); + VIR_FREE(old_name); + return ret; +} + /* The caller must hold lock on 'doms' in addition to 'virDomainObjListRemove' * requirements * diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h index f479598..c0f856c 100644 --- a/src/conf/virdomainobjlist.h +++ b/src/conf/virdomainobjlist.h @@ -51,11 +51,17 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, unsigned int flags, virDomainDefPtr *oldDef); -int virDomainObjListRenameAddNew(virDomainObjListPtr doms, - virDomainObjPtr vm, - const char *name); -int virDomainObjListRenameRemove(virDomainObjListPtr doms, - const char *name); +typedef int (*virDomainObjListRenameCallback)(virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + void *opaque); +int virDomainObjListRename(virDomainObjListPtr doms, + virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + virDomainObjListRenameCallback callback, + void *opaque); + void virDomainObjListRemove(virDomainObjListPtr doms, virDomainObjPtr dom); void virDomainObjListRemoveLocked(virDomainObjListPtr doms, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3d0ec9d..3ef3468 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -893,8 +893,7 @@ virDomainObjListNew; virDomainObjListNumOfDomains; virDomainObjListRemove; virDomainObjListRemoveLocked; -virDomainObjListRenameAddNew; -virDomainObjListRenameRemove; +virDomainObjListRename; # cpu/cpu.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abcdbe6..c0e750d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19976,14 +19976,14 @@ qemuDomainSetUserPassword(virDomainPtr dom, } -static int qemuDomainRename(virDomainPtr dom, - const char *new_name, - unsigned int flags) +static int +qemuDomainRenameCallback(virDomainObjPtr vm, + const char *new_name, + unsigned int flags, + void *opaque) { - virQEMUDriverPtr driver = dom->conn->privateData; + virQEMUDriverPtr driver = opaque; virQEMUDriverConfigPtr cfg = NULL; - virDomainObjPtr vm = NULL; - virDomainObjPtr tmp_dom = NULL; virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; @@ -19993,73 +19993,16 @@ static int qemuDomainRename(virDomainPtr dom, virCheckFlags(0, ret); - if (!(vm = qemuDomObjFromDomain(dom))) - goto cleanup; - - if (virDomainRenameEnsureACL(dom->conn, vm->def) < 0) - goto cleanup; - cfg = virQEMUDriverGetConfig(driver); - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - - if (virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot rename active domain")); - goto endjob; - } - - if (!vm->persistent) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot rename a transient domain")); - goto endjob; - } - - if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain has to be shutoff before renaming")); - goto endjob; - } - - if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot rename domain with snapshots")); - goto endjob; - } - - if (STREQ(vm->def->name, new_name)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Can't rename domain to itself")); - goto endjob; - } - - /* - * This is a rather racy check, but still better than reporting - * internal error. And since new_name != name here, there's no - * deadlock imminent. - */ - tmp_dom = virDomainObjListFindByName(driver->domains, new_name); - if (tmp_dom) { - virObjectUnlock(tmp_dom); - virObjectUnref(tmp_dom); - virReportError(VIR_ERR_OPERATION_INVALID, - _("domain with name '%s' already exists"), - new_name); - goto endjob; - } - if (VIR_STRDUP(new_dom_name, new_name) < 0) - goto endjob; + goto cleanup; if (!(old_dom_cfg_file = virDomainConfigFile(cfg->configDir, vm->def->name))) { - goto endjob; + goto cleanup; } - if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) - goto endjob; - event_old = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_RENAMED); @@ -20080,21 +20023,12 @@ static int qemuDomainRename(virDomainPtr dom, goto rollback; } - /* Remove old domain name from table. */ - virDomainObjListRenameRemove(driver->domains, old_dom_name); - event_new = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, VIR_DOMAIN_EVENT_DEFINED_RENAMED); - - /* Success, domain has been renamed. */ ret = 0; - endjob: - qemuDomainObjEndJob(driver, vm); - cleanup: - virDomainObjEndAPI(&vm); VIR_FREE(old_dom_cfg_file); VIR_FREE(old_dom_name); VIR_FREE(new_dom_name); @@ -20109,9 +20043,65 @@ static int qemuDomainRename(virDomainPtr dom, vm->def->name = old_dom_name; old_dom_name = NULL; } + goto cleanup; +} - virDomainObjListRenameRemove(driver->domains, new_name); - goto endjob; +static int qemuDomainRename(virDomainPtr dom, + const char *new_name, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(0, ret); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainRenameEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot rename active domain")); + goto endjob; + } + + if (!vm->persistent) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot rename a transient domain")); + goto endjob; + } + + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain has to be shutoff before renaming")); + goto endjob; + } + + if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot rename domain with snapshots")); + goto endjob; + } + + if (virDomainObjListRename(driver->domains, vm, new_name, flags, + qemuDomainRenameCallback, driver) < 0) + goto endjob; + + /* Success, domain has been renamed. */ + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; } static virHypervisorDriver qemuHypervisorDriver = { -- 1.8.3.1

2 2

[libvirt] [PATCH 0/5] Export remote typed params methods internally via util
by Erik Skultety 03 Feb '16

03 Feb '16

Erik Skultety (5): cfg.mk: Adjust sc_prohibit_int_ijk to support 'exempt from syntax-check' util: Introduce virTypedParameterRemote datatype util: Export remoteDeserializeTypedParameters internally via util util: Export remoteFreeTypedParameters internally via util util: Export remoteSerializeTypedParameters internally via util cfg.mk | 3 +- daemon/remote.c | 218 ++++++--------------------------------- src/libvirt_private.syms | 3 + src/remote/remote_driver.c | 196 +++--------------------------------- src/rpc/gendispatch.pl | 9 +- src/util/virtypedparam.c | 246 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 40 ++++++++ 7 files changed, 341 insertions(+), 374 deletions(-) -- 2.4.3

2 9

[libvirt] [PATCH v2] security_selinux: Fix crash in virSecuritySELinuxRestoreFileLabel
by Shanzhi Yu 03 Feb '16

03 Feb '16

virSecuritySELinuxRestoreFileLabel should never be called with NULL path add check before call this function in case of causeing libvirtd crash https://bugzilla.redhat.com/show_bug.cgi?id=1300532 Signed-off-by: Shanzhi Yu <shyu(a)redhat.com> --- src/security/security_selinux.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9e98635..77e55a3 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1098,7 +1098,8 @@ virSecuritySELinuxRestoreInputLabel(virSecurityManagerPtr mgr, switch ((virDomainInputType) input->type) { case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: - rc = virSecuritySELinuxRestoreFileLabel(mgr, input->source.evdev); + if (input->source.evdev) + rc = virSecuritySELinuxRestoreFileLabel(mgr, input->source.evdev); break; case VIR_DOMAIN_INPUT_TYPE_MOUSE: @@ -1171,7 +1172,9 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr, switch (tpm->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: tpmdev = tpm->data.passthrough.source.data.file.path; - rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev); + + if (tpmdev) + rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev); if ((cancel_path = virTPMCreateCancelPath(tpmdev)) != NULL) { if (virSecuritySELinuxRestoreFileLabel(mgr, cancel_path) < 0) @@ -1722,7 +1725,9 @@ virSecuritySELinuxRestoreHostdevCapsLabel(virSecurityManagerPtr mgr, if (VIR_STRDUP(path, dev->source.caps.u.storage.block) < 0) return -1; } - ret = virSecuritySELinuxRestoreFileLabel(mgr, path); + if (path) + ret = virSecuritySELinuxRestoreFileLabel(mgr, path); + VIR_FREE(path); break; } @@ -1736,7 +1741,8 @@ virSecuritySELinuxRestoreHostdevCapsLabel(virSecurityManagerPtr mgr, if (VIR_STRDUP(path, dev->source.caps.u.misc.chardev) < 0) return -1; } - ret = virSecuritySELinuxRestoreFileLabel(mgr, path); + if (path) + ret = virSecuritySELinuxRestoreFileLabel(mgr, path); VIR_FREE(path); break; } @@ -1876,13 +1882,15 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr, switch (dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) - goto done; + if (dev_source->data.file.path) { + if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) + goto done; + } ret = 0; break; case VIR_DOMAIN_CHR_TYPE_UNIX: - if (!dev_source->data.nix.listen) { + if (!dev_source->data.nix.listen && dev_source->data.file.path) { if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) goto done; } @@ -1898,7 +1906,8 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr, (virSecuritySELinuxRestoreFileLabel(mgr, in) < 0)) { goto done; } - } else if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) { + } else if (dev_source->data.file.path && + virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) { goto done; } ret = 0; -- 1.8.3.1

2 1

[libvirt] [PATCH] qemuDomainResume: allow to resume domain with guest panicked
by Dmitry Andreev 03 Feb '16

03 Feb '16

In case of guest panicked, preserved crashed domain has stopped CPUs. It's not possible to use tools like WinDbg for the problem investigation until we start CPUs back. --- src/qemu/qemu_driver.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e46404b..0930280 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1873,6 +1873,7 @@ static int qemuDomainResume(virDomainPtr dom) int ret = -1; virObjectEventPtr event = NULL; int state; + int reason; virQEMUDriverConfigPtr cfg = NULL; if (!(vm = qemuDomObjFromDomain(dom))) @@ -1892,12 +1893,14 @@ static int qemuDomainResume(virDomainPtr dom) goto endjob; } - state = virDomainObjGetState(vm, NULL); + state = virDomainObjGetState(vm, &reason); if (state == VIR_DOMAIN_PMSUSPENDED) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is pmsuspended")); goto endjob; - } else if (state == VIR_DOMAIN_PAUSED) { + } else if ((state == VIR_DOMAIN_CRASHED && + reason == VIR_DOMAIN_CRASHED_PANICKED) || + state == VIR_DOMAIN_PAUSED) { if (qemuProcessStartCPUs(driver, vm, dom->conn, VIR_DOMAIN_RUNNING_UNPAUSED, QEMU_ASYNC_JOB_NONE) < 0) { -- 1.8.3.1

2 2

[libvirt] [PATCH v3] qemu: return -1 on error paths in qemuDomainSaveImageStartVM
by Nikolay Shirokovskiy 03 Feb '16

03 Feb '16

Error paths after sending the event that domain is started written as if ret = -1 which is set at the beginning of the function. It's common idioma to keep 'ret' equal to -1 until the end of function where it is set to 0. But here we use ret to keep result of restore operation too and thus breaks the idioma and its users :) Let's use different variable to hold restore result. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/qemu/qemu_driver.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abcdbe6..f612cf0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6732,6 +6732,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, qemuDomainAsyncJob asyncJob) { int ret = -1; + bool restored = false; virObjectEventPtr event; int intermediatefd = -1; virCommandPtr cmd = NULL; @@ -6758,13 +6759,14 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ - ret = qemuProcessStart(conn, driver, vm, asyncJob, - "stdio", *fd, path, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, - VIR_QEMU_PROCESS_START_PAUSED); + if (qemuProcessStart(conn, driver, vm, asyncJob, + "stdio", *fd, path, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, + VIR_QEMU_PROCESS_START_PAUSED) == 0) + restored = true; if (intermediatefd != -1) { - if (ret < 0) { + if (!restored) { /* if there was an error setting up qemu, the intermediate * process will wait forever to write to stdout, so we * must manually kill it. @@ -6775,7 +6777,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (virCommandWait(cmd, NULL) < 0) { qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0); - ret = -1; + restored = false; } VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf)); } @@ -6783,18 +6785,16 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (VIR_CLOSE(*fd) < 0) { virReportSystemError(errno, _("cannot close file: %s"), path); - ret = -1; + restored = false; } - if (ret < 0) { - virDomainAuditStart(vm, "restored", false); + virDomainAuditStart(vm, "restored", restored); + if (!restored) goto cleanup; - } event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); - virDomainAuditStart(vm, "restored", true); qemuDomainEventQueue(driver, event); -- 1.8.3.1

2 1

[libvirt] [PATCH 1/2] Move qemuGetDHCPInterfaces to separate file
by Guido Günther 03 Feb '16

03 Feb '16

so we can use it from the LXC driver as well. --- I couldn't find a nice place to add this so I went for a separate file. I'm happy to move this elsewhere. Cheers, -- Guido po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 4 ++ src/qemu/qemu_driver.c | 104 +---------------------------------- src/util/virdhcpinterfaces.c | 127 +++++++++++++++++++++++++++++++++++++++++++ src/util/virdhcpinterfaces.h | 34 ++++++++++++ 6 files changed, 169 insertions(+), 102 deletions(-) create mode 100644 src/util/virdhcpinterfaces.c create mode 100644 src/util/virdhcpinterfaces.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 82e8d3e..64eaa9d 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -186,6 +186,7 @@ src/util/vircommand.c src/util/virconf.c src/util/vircrypto.c src/util/virdbus.c +src/util/virdhcpinterfaces.c src/util/virdnsmasq.c src/util/virerror.c src/util/virerror.h diff --git a/src/Makefile.am b/src/Makefile.am index a4aef0f..bddfd21 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -104,6 +104,7 @@ UTIL_SOURCES = \ util/virconf.c util/virconf.h \ util/vircrypto.c util/vircrypto.h \ util/virdbus.c util/virdbus.h util/virdbuspriv.h \ + util/virdhcpinterfaces.c util/virdhcpinterfaces.h \ util/virdnsmasq.c util/virdnsmasq.h \ util/virebtables.c util/virebtables.h \ util/virendian.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3d0ec9d..ea3e83c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1377,6 +1377,10 @@ virDBusMessageRead; virDBusSetSharedBus; +# util/virdhcpinterfaces.h +virGetDHCPInterfaces; + + # util/virdnsmasq.h dnsmasqAddDhcpHost; dnsmasqAddHost; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abcdbe6..fe1cb6d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -101,6 +101,7 @@ #include "virnuma.h" #include "dirname.h" #include "network/bridge_driver.h" +#include "virdhcpinterfaces.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -173,10 +174,6 @@ static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, const char *path, int oflags, bool *needUnlink, bool *bypassSecurityDriver); -static int qemuGetDHCPInterfaces(virDomainPtr dom, - virDomainObjPtr vm, - virDomainInterfacePtr **ifaces); - virQEMUDriverPtr qemu_driver = NULL; @@ -19794,7 +19791,7 @@ qemuDomainInterfaceAddresses(virDomainPtr dom, switch (source) { case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE: - ret = qemuGetDHCPInterfaces(dom, vm, ifaces); + ret = virGetDHCPInterfaces(dom, vm, ifaces); break; case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT: @@ -19825,103 +19822,6 @@ qemuDomainInterfaceAddresses(virDomainPtr dom, return ret; } -static int -qemuGetDHCPInterfaces(virDomainPtr dom, - virDomainObjPtr vm, - virDomainInterfacePtr **ifaces) -{ - int rv = -1; - int n_leases = 0; - size_t i, j; - size_t ifaces_count = 0; - virNetworkPtr network = NULL; - char macaddr[VIR_MAC_STRING_BUFLEN]; - virDomainInterfacePtr iface = NULL; - virNetworkDHCPLeasePtr *leases = NULL; - virDomainInterfacePtr *ifaces_ret = NULL; - - if (!dom->conn->networkDriver || - !dom->conn->networkDriver->networkGetDHCPLeases) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Network driver does not support DHCP lease query")); - return -1; - } - - for (i = 0; i < vm->def->nnets; i++) { - if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK) - continue; - - virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr); - virObjectUnref(network); - network = virNetworkLookupByName(dom->conn, - vm->def->nets[i]->data.network.name); - - if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, - &leases, 0)) < 0) - goto error; - - if (n_leases) { - if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0) - goto error; - - if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0) - goto error; - - iface = ifaces_ret[ifaces_count - 1]; - /* Assuming each lease corresponds to a separate IP */ - iface->naddrs = n_leases; - - if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0) - goto error; - - if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0) - goto cleanup; - - if (VIR_STRDUP(iface->hwaddr, macaddr) < 0) - goto cleanup; - } - - for (j = 0; j < n_leases; j++) { - virNetworkDHCPLeasePtr lease = leases[j]; - virDomainIPAddressPtr ip_addr = &iface->addrs[j]; - - if (VIR_STRDUP(ip_addr->addr, lease->ipaddr) < 0) - goto cleanup; - - ip_addr->type = lease->type; - ip_addr->prefix = lease->prefix; - } - - for (j = 0; j < n_leases; j++) - virNetworkDHCPLeaseFree(leases[j]); - - VIR_FREE(leases); - } - - *ifaces = ifaces_ret; - ifaces_ret = NULL; - rv = ifaces_count; - - cleanup: - virObjectUnref(network); - if (leases) { - for (i = 0; i < n_leases; i++) - virNetworkDHCPLeaseFree(leases[i]); - } - VIR_FREE(leases); - - return rv; - - error: - if (ifaces_ret) { - for (i = 0; i < ifaces_count; i++) - virDomainInterfaceFree(ifaces_ret[i]); - } - VIR_FREE(ifaces_ret); - - goto cleanup; -} - static int qemuDomainSetUserPassword(virDomainPtr dom, diff --git a/src/util/virdhcpinterfaces.c b/src/util/virdhcpinterfaces.c new file mode 100644 index 0000000..80135f7 --- /dev/null +++ b/src/util/virdhcpinterfaces.c @@ -0,0 +1,127 @@ +/* + * virdhcpinterfaces.c: get a domains dhcp managed interfaces + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange(a)redhat.com> + */ + +#include <config.h> + +#include "virdhcpinterfaces.h" +#include "viralloc.h" +#include "virstring.h" +#include "datatypes.h" + +#define VIR_FROM_THIS VIR_FROM_AUDIT + +int +virGetDHCPInterfaces(virDomainPtr dom, + virDomainObjPtr vm, + virDomainInterfacePtr **ifaces) +{ + int rv = -1; + int n_leases = 0; + size_t i, j; + size_t ifaces_count = 0; + virNetworkPtr network = NULL; + char macaddr[VIR_MAC_STRING_BUFLEN]; + virDomainInterfacePtr iface = NULL; + virNetworkDHCPLeasePtr *leases = NULL; + virDomainInterfacePtr *ifaces_ret = NULL; + + if (!dom->conn->networkDriver || + !dom->conn->networkDriver->networkGetDHCPLeases) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Network driver does not support DHCP lease query")); + return -1; + } + + for (i = 0; i < vm->def->nnets; i++) { + if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr); + virObjectUnref(network); + network = virNetworkLookupByName(dom->conn, + vm->def->nets[i]->data.network.name); + + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto error; + + if (n_leases) { + if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0) + goto error; + + if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0) + goto error; + + iface = ifaces_ret[ifaces_count - 1]; + /* Assuming each lease corresponds to a separate IP */ + iface->naddrs = n_leases; + + if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0) + goto error; + + if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0) + goto cleanup; + + if (VIR_STRDUP(iface->hwaddr, macaddr) < 0) + goto cleanup; + } + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + virDomainIPAddressPtr ip_addr = &iface->addrs[j]; + + if (VIR_STRDUP(ip_addr->addr, lease->ipaddr) < 0) + goto cleanup; + + ip_addr->type = lease->type; + ip_addr->prefix = lease->prefix; + } + + for (j = 0; j < n_leases; j++) + virNetworkDHCPLeaseFree(leases[j]); + + VIR_FREE(leases); + } + + *ifaces = ifaces_ret; + ifaces_ret = NULL; + rv = ifaces_count; + + cleanup: + virObjectUnref(network); + if (leases) { + for (i = 0; i < n_leases; i++) + virNetworkDHCPLeaseFree(leases[i]); + } + VIR_FREE(leases); + + return rv; + + error: + if (ifaces_ret) { + for (i = 0; i < ifaces_count; i++) + virDomainInterfaceFree(ifaces_ret[i]); + } + VIR_FREE(ifaces_ret); + + goto cleanup; +} diff --git a/src/util/virdhcpinterfaces.h b/src/util/virdhcpinterfaces.h new file mode 100644 index 0000000..c7acb8f --- /dev/null +++ b/src/util/virdhcpinterfaces.h @@ -0,0 +1,34 @@ +/* + * virdhcpinterfaces.h: get a domains dhcp managed interfaces + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange(a)redhat.com> + */ + +#ifndef __VIR_DHCP_INTERFACES_H__ +# define __VIR_DHCP_INTERFACES_H__ + +# include "internal.h" +# include "virdomainobjlist.h" + +int +virGetDHCPInterfaces(virDomainPtr dom, + virDomainObjPtr vm, + virDomainInterfacePtr **ifaces); + +#endif /* __VIR_DHCP_INTERFACES_H__ */ -- 2.7.0.rc3

2 3

[libvirt] [PATCH v2 00/21] vcpu info storage refactors - part 2
by Peter Krempa 03 Feb '16

03 Feb '16

Some of patches were pushed. The rest fixes most of the feedback that made sense that was against v1. Peter Krempa (21): cgroup: Clean up virCgroupGetPercpuStats conf: Add helper to retrieve bitmap of active vcpus for a definition cgroup: Prepare for sparse vCPU topologies in virCgroupGetPercpuStats qemu: cpu hotplug: Set vcpu state directly in the new structure qemu: Move and rename qemuProcessDetectVcpuPIDs to qemuDomainDetectVcpuPids qemu: domain: Prepare qemuDomainDetectVcpuPids for reuse qemu: Reuse qemuDomainDetectVcpuPids in cpu hot(un)plug conf: Don't copy def->cpumask into cpu pinning info conf: Split out logic to determine whether cpupin was provided conf: Store cpu pinning data in def->vcpus conf: remove unused cpu pinning helpers and data structures conf: Extract code that formats <cputune> util: bitmap: Introduce bitmap subtraction conf: Add helper to return a bitmap of active iothread ids conf: Extract code for parsing thread resource scheduler info conf: Don't store vcpusched orthogonally to other vcpu info conf: Fix how iothread scheduler info is stored qemu: vcpu: Aggregate code to set vCPU tuning qemu: vcpu: Reuse qemuProcessSetupVcpu in vcpu hotplug qemu: iothread: Aggregate code to set IOTrhead tuning qemu: iothread: Reuse qemuProcessSetupIOThread in iothread hotplug src/conf/domain_conf.c | 940 +++++++++++---------- src/conf/domain_conf.h | 45 +- src/libvirt_private.syms | 10 +- src/libxl/libxl_domain.c | 20 +- src/libxl/libxl_driver.c | 41 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_cgroup.c | 195 ----- src/qemu/qemu_cgroup.h | 2 - src/qemu/qemu_domain.c | 84 ++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 405 +++------ src/qemu/qemu_process.c | 478 ++++++----- src/qemu/qemu_process.h | 6 + src/test/test_driver.c | 45 +- src/util/virbitmap.c | 21 + src/util/virbitmap.h | 3 + src/util/vircgroup.c | 74 +- src/util/vircgroup.h | 3 +- src/vz/vz_sdk.c | 4 +- .../qemuxml2xmlout-cputune-iothreadsched.xml | 3 +- tests/virbitmaptest.c | 55 ++ tests/vircgrouptest.c | 2 +- 22 files changed, 1148 insertions(+), 1292 deletions(-) -- 2.6.2

2 43

[libvirt] [PATCH v4] qemu: Connect to guest agent iff needed
by Michal Privoznik 03 Feb '16

03 Feb '16

https://bugzilla.redhat.com/show_bug.cgi?id=1293351 Since we already have virtio channel events, we know when guest agent within guest has (dis-)connected. Instead of us blindly connecting to a socket that no one is listening to, we can just follow what qemu-ga does. This has a nice benefit that we don't need to 'guest-ping' the agent just to timeout and find out nobody is listening. The way that this commit is implemented: - don't connect in qemuProcessStart directly, defer that to event callback (which already follows the agent). - after migration is settled, before we resume vCPUs, ask qemu whether somebody is listening on the socket and if so, connect to it. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- diff to v3: -Move cap detection into qemuConnectAgent src/qemu/qemu_driver.c | 13 ++++++++++++- src/qemu/qemu_migration.c | 12 ++++++++++++ src/qemu/qemu_process.c | 24 ++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ccf68b..3751ba6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6650,12 +6650,13 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, bool start_paused, qemuDomainAsyncJob asyncJob) { - int ret = -1; + int rc, ret = -1; virObjectEventPtr event; int intermediatefd = -1; virCommandPtr cmd = NULL; char *errbuf = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; if ((header->version == 2) && (header->compressed != QEMU_SAVE_FORMAT_RAW)) { @@ -6710,6 +6711,16 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, goto cleanup; } + if ((rc = qemuConnectAgent(driver, vm)) < 0) { + if (rc == -2) + goto cleanup; + + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 51e7125..9678adf 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5795,6 +5795,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, unsigned short port; unsigned long long timeReceived = 0; virObjectEventPtr event; + int rc; VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d", @@ -5863,6 +5864,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver, if (qemuMigrationStopNBDServer(driver, vm, mig) < 0) goto endjob; + if ((rc = qemuConnectAgent(driver, vm)) < 0) { + if (rc == -2) + goto endjob; + + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + + if (flags & VIR_MIGRATE_PERSIST_DEST) { if (qemuMigrationPersist(driver, vm, mig, !v3proto) < 0) { /* Hmpf. Migration was successful, but making it persistent diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 05cbda2..e1a25dd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -77,6 +77,10 @@ VIR_LOG_INIT("qemu.qemu_process"); +static int +qemuProcessReconnectRefreshChannelVirtioState(virQEMUDriverPtr driver, + virDomainObjPtr vm); + /** * qemuProcessRemoveDomainStatus * @@ -208,6 +212,26 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) if (!config) return 0; + if (priv->agent) + return 0; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE) && + config->state != VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { + /* So qemu is capable of sending us event whenever guest agent + * (dis-)connects. And we reflect its state in config->state. Well, + * nearly. It may happen that what we think about the guest agent state + * is not accurate, e.g. right after migration. Ask qemu to be sure. */ + + if (qemuProcessReconnectRefreshChannelVirtioState(driver, vm) < 0) + goto cleanup; + + /* Now we are sure about the channel state. */ + if (config->state != VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { + VIR_DEBUG("Deferring connecting to guest agent"); + return 0; + } + } + if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager, vm->def) < 0) { VIR_ERROR(_("Failed to set security context for agent for %s"), -- 2.4.10

2 3

[libvirt] [PATCH] qemu: Don't crash when create fails early
by Martin Kletzander 02 Feb '16

02 Feb '16

Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9 we are generating socket path later than before -- when starting a domain. That makes one particular inconsistent state of a chardev, which was not possible before, currently valid. However, SELinux security driver forgot to guard the main restoring function by a check for NULL-paths. So make it no-op for NULL paths, as in the DAC driver. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1300532 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/security/security_selinux.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9e986350fbb1..6a32f0a27f12 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1024,6 +1024,12 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr, char *newpath = NULL; char ebuf[1024]; + /* Some paths are auto-generated, so let's be safe here and do + * nothing if nothing is needed. + */ + if (!path) + return 0; + VIR_INFO("Restoring SELinux context on '%s'", path); if (virFileResolveLink(path, &newpath) < 0) { -- 2.7.0

2 1

[libvirt] oVirt considers using macTableManager='libvirt'
by Ido Barkan 02 Feb '16

02 Feb '16

Hi guys, We, at oVirt, are considring using the automatic bridge management feature of libvirt for our hosts (macTableManager='libvirt'). I could find any discussion in the mailing list archives about the motivation for this feature. (was there?). If there wasn't I would like to start a new one, about the possible trade offs it would have in oVirt. Specifically I have a few questions: 1) The obvious performance motivation is clear: considering N hosts with M vms each connected to the same LAN, the first packet to any unknown yet host will flood all the vms in all N bridges. -- was there any other motivation that I do not understand (apart from slightly better security? 2) oVirt uses TC for port mirroring, in case this is requested by users, to mirror traffic from a chosen bridge to a chosen NIC in the host. I could not understand if macTableManager will interfere with it, or not. 3) Are there any drawbacks to enabling this feature? 4) We aim for rhel7.2. Will the feature be supported (or partially supported) for kernels older then 3.17? And if so, in what way? 5) oVirt currently builds its own bridges and tell libvirt about them. Does that have any affect on the functionality of that feature? 6) are there any plans to support OVS for this feature in the future? -- Thanks, Ido Barkan

4 7

[libvirt] [PATCH] conf: qemu: Add support for more HyperV Enlightenment features
by Maxim Nestratov 02 Feb '16

02 Feb '16

This patch adds support for "vpindex", "runtime", "synic" and "stimer" features available in qemu 2.5+. - When Hyper-V "vpindex" is on, guest can use MSR HV_X64_MSR_VP_INDEX to get virtual processor ID. - Hyper-V "runtime" enlightement feature allows to use MSR HV_X64_MSR_VP_RUNTIME to get the time the virtual processor consumes running guest code, as well as the time the hypervisor spends running code on behalf of that guest. - Hyper-V "synic" stands for Synthetic Interrupt Controller, which is lapic extension controlled via MSRs. - Hyper-V "stimer" switches on Hyper-V SynIC timers MSR's support. Guest can setup and use fired by host events (SynIC interrupt and appropriate timer expiration message) as guest clock events Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com> --- docs/formatdomain.html.in | 28 ++++++++++++++++++++++ docs/schemas/domaincommon.rng | 20 ++++++++++++++++ src/conf/domain_conf.c | 18 +++++++++++++- src/conf/domain_conf.h | 4 ++++ src/qemu/qemu_command.c | 8 +++++++ tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml | 4 ++++ tests/qemuxml2argvdata/qemuxml2argv-hyperv.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml | 4 ++++ 8 files changed, 86 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b3187bb..3edd0a2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1460,6 +1460,10 @@ <relaxed state='on'/> <vapic state='on'/> <spinlocks state='on' retries='4096'/> + <vpindex state='on'/> + <runtime state='on'/> + <synic state='on'/> + <stimer state='on'/> </hyperv> <kvm> <hidden state='on'/> @@ -1535,6 +1539,30 @@ <td>on, off; retries - at least 4095</td> <td>1.1.0 (QEMU only)</td> </tr> + <tr> + <td>vpindex</td> + <td>Virtual processor index</td> + <td> on, off</td> + <td>2.5.0 (QEMU only)</td> + </tr> + <tr> + <td>runtime</td> + <td>Processor time spent on running guest code and on behalf of guest code</td> + <td> on, off</td> + <td>2.5.0 (QEMU only)</td> + </tr> + <tr> + <td>synic</td> + <td>Enable Synthetic Interrupt Controller (SyNIC)</td> + <td> on, off</td> + <td>2.5.0 (QEMU only)</td> + </tr> + <tr> + <td>stimer</td> + <td>Enable SyNIC timers</td> + <td> on, off</td> + <td>2.5.0 (QEMU only)</td> + </tr> </table> </dd> <dt><code>pvspinlock</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5deb17b..e079e86 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4858,6 +4858,26 @@ </optional> </element> </optional> + <optional> + <element name="vpindex"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="runtime"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="synic"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="stimer"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1ea74a6..bbc14fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -145,7 +145,11 @@ VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, VIR_ENUM_IMPL(virDomainHyperv, VIR_DOMAIN_HYPERV_LAST, "relaxed", "vapic", - "spinlocks") + "spinlocks", + "vpindex", + "runtime", + "synic", + "stimer") VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden") @@ -15460,6 +15464,10 @@ virDomainDefParseXML(xmlDocPtr xml, switch ((virDomainHyperv) feature) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: if (!(tmp = virXPathString("string(./@state)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -17508,6 +17516,10 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: if (src->hyperv_features[i] != dst->hyperv_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of HyperV enlightenment " @@ -22099,6 +22111,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, switch ((virDomainHyperv) j) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: if (def->hyperv_features[j]) virBufferAsprintf(buf, "<%s state='%s'/>\n", virDomainHypervTypeToString(j), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0141009..c96e0c4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1700,6 +1700,10 @@ typedef enum { VIR_DOMAIN_HYPERV_RELAXED = 0, VIR_DOMAIN_HYPERV_VAPIC, VIR_DOMAIN_HYPERV_SPINLOCKS, + VIR_DOMAIN_HYPERV_VPINDEX, + VIR_DOMAIN_HYPERV_RUNTIME, + VIR_DOMAIN_HYPERV_SYNIC, + VIR_DOMAIN_HYPERV_STIMER, VIR_DOMAIN_HYPERV_LAST } virDomainHyperv; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d3ab3a..fc55a82 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7739,6 +7739,10 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(&buf, ",hv_%s", virDomainHypervTypeToString(i)); @@ -12790,6 +12794,10 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, switch ((virDomainHyperv) f) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: if (value) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("HyperV feature '%s' should not " diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml index 1067f64..438f4a5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml @@ -14,6 +14,10 @@ <relaxed state='off'/> <vapic state='off'/> <spinlocks state='off'/> + <vpindex state='off'/> + <runtime state='off'/> + <synic state='off'/> + <stimer state='off'/> </hyperv> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args index a6f37e3..e4b2af5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args @@ -8,7 +8,7 @@ QEMU_AUDIO_DRV=none \ -name QEMUGuest1 \ -S \ -M pc \ --cpu qemu32,hv_relaxed,hv_vapic,hv_spinlocks=0x2fff \ +-cpu qemu32,hv_relaxed,hv_vapic,hv_spinlocks=0x2fff,hv_vpindex,hv_runtime,hv_synic,hv_stimer \ -m 214 \ -smp 6 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml index 2b8f332..2844b24 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml @@ -14,6 +14,10 @@ <relaxed state='on'/> <vapic state='on'/> <spinlocks state='on' retries='12287'/> + <vpindex state='on'/> + <runtime state='on'/> + <synic state='on'/> + <stimer state='on'/> </hyperv> </features> <clock offset='utc'/> -- 1.8.3.1

1 0

[libvirt] [PATCH v2 0/6] Some logical pool/volume changes
by John Ferlan 02 Feb '16

02 Feb '16

v1: http://www.redhat.com/archives/libvir-list/2016-January/msg01023.html Differences to v1: Patch 1 is the former patch 2 with the following adjuments: - Change helper name to be virStorageBackendLogicalParseVolExtents - Include all 'extent' parsing needs - including fetch/parse of 'stripes' to determine 'nextents' value, allocation of vol->source.extents, parse of groups[6] to be 'length', parse of groups[7] to be 'size' Patch 2 is new, but replaces some of the concepts from patch 3: - I rototilled the virStorageBackendLogicalParseVolExtents to remove the unnecessary trip through regex/regcomp. - Used virStringSplitCount instread and then parse the resultant string. - Removed the need for the 'stripes' fetch since it really wasn't doing what it was supposed to. - I adjusted the virstringtest to exhibit what is being parsed (it can be removed if you think that's excessive). Patches 3-5 are new - These could be combined. I left them separate to show the process... - Essentially adding a new "source" field to be used to display in the vol-dumpxml output Patch 6 is part of the old patch 4: - Since the old patch 3 isn't necessary - it's essentially gone. This just takes the patch 4 concept of changing the regex, but also applies it to the current code. John Ferlan (6): logical: Create helper virStorageBackendLogicalParseVolExtents logical: Fix parsing of the 'devices' field lvs output logical: Search for a segtype of "thin" and mark lv as sparse vol: Add thin_pool to _virStorageVolSource logical: Add capability to get the thin pool name logical: Display thin lv's found in a libvirt managed volume group docs/formatstorage.html.in | 9 +- src/conf/storage_conf.c | 5 + src/conf/storage_conf.h | 1 + src/storage/storage_backend_logical.c | 221 ++++++++++++++++------------------ tests/virstringtest.c | 11 ++ 5 files changed, 127 insertions(+), 120 deletions(-) -- 2.5.0

2 23

Re: [libvirt] [PATCH] virt-aa-helper: better write denials handling
by Guido Günther 02 Feb '16

02 Feb '16

Hi, On Mon, Jan 18, 2016 at 01:45:08PM +0100, Cédric Bosdonnat wrote: > Better fix replacing c726af2d: introducing an 'R' permission to > add read rule, but no explicit deny write rule. > --- > src/security/virt-aa-helper.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > index a2d7226..d0c5246 100644 > --- a/src/security/virt-aa-helper.c > +++ b/src/security/virt-aa-helper.c > @@ -785,12 +785,18 @@ get_definition(vahControl * ctl, const char *xmlStr) > return rc; > } > > +/** > + * The permissions allowed are apparmor valid permissions and 'R'. 'R' stands for > + * read with no explicit deny rule. > + */ > static int > vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursive) > { > char *tmp = NULL; > int rc = -1; > bool readonly = true; > + bool explicit_deny_rule = true; > + char *sub = NULL; > > if (path == NULL) > return rc; > @@ -815,8 +821,16 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi > return rc; > } > > - if (strchr(perms, 'w') != NULL) > + if (strchr(perms, 'w') != NULL) { > readonly = false; > + explicit_deny_rule = false; > + } > + > + if ((sub = strchr(perms, 'R')) != NULL) { > + /* Don't write the invalid R permission, replace it with 'r' */ > + sub[0] = 'r'; > + explicit_deny_rule = false; > + } > > rc = valid_path(tmp, readonly); > if (rc != 0) { > @@ -831,7 +845,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi > tmp[strlen(tmp) - 1] = '\0'; > > virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms); > - if (readonly) { > + if (explicit_deny_rule) { > virBufferAddLit(buf, " # don't audit writes to readonly files\n"); > virBufferAsprintf(buf, " deny \"%s%s\" w,\n", tmp, recursive ? "/**" : ""); > } > @@ -1130,7 +1144,7 @@ get_files(vahControl * ctl) > /* We don't need to add deny rw rules for readonly mounts, > * this can only lead to troubles when mounting / readonly. > */ > - if (vah_add_path(&buf, fs->src, "rw", true) != 0) > + if (vah_add_path(&buf, fs->src, fs->readonly ? "R" : "rw", true) != 0) > goto cleanup; > } > } this looks good to me but it would be great to have Jamie's input on that. Ceers, -- Guido

1 0

[libvirt] [PATCH] tests: avoid realpath in test-lib.sh
by Eric Blake 01 Feb '16

01 Feb '16

Ever since commit ace4aecd, running 'make check' on RHEL 6 produces: ./test-lib.sh: line 21: realpath: command not found for every shell script test, because 'realpath' was not part of coreutils back then. * tests/test-lib.sh (_scriptdir): Compute with only portable shell. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- A warning message is cosmetic, and not quite a build-breaker, so I'll wait for a review before pushing this. tests/test-lib.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-lib.sh b/tests/test-lib.sh index 920b01e..8e0ce83 100644 --- a/tests/test-lib.sh +++ b/tests/test-lib.sh @@ -1,6 +1,6 @@ # test-lib.sh: source this file; set up for tests -# Copyright (C) 2008-2013 Red Hat, Inc. +# Copyright (C) 2008-2013, 2016 Red Hat, Inc. # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -18,7 +18,7 @@ # # Based on an idea from GNU coreutils -_scriptdir="$(realpath $(dirname $0))" +_scriptdir="$(unset CDPATH; cd $(dirname $0) && pwd)" test -z "$abs_srcdir" && abs_srcdir=$_scriptdir test -z "$abs_builddir" && abs_builddir=$_scriptdir test -z "$abs_top_srcdir" && abs_top_srcdir=$_scriptdir/.. -- 1.7.1

2 1

[libvirt] [PATCH v3 0/4] virshBlockJobWait improvements
by Michael Chapman 01 Feb '16

01 Feb '16

v2 discussion at: http://www.redhat.com/archives/libvir-list/2016-January/msg01063.html As requested I've split this into separate commits. All of the comment nitpicks have been applied. With regards to using "break" or "goto cleanup" to exit the loop, I decided to use "goto" on all the error cases and "break" everywhere else. I had noticed another bug where the SIGINT signal action wasn't reset if the virTimeMillisNow() call failed; that's fixed in patch 3 in this series. Michael Chapman (4): virsh: avoid unnecessary progress updates virsh: be consistent with style of loop exit virsh: ensure SIGINT action is reset on all errors virsh: improve waiting for block job readiness tools/virsh-domain.c | 65 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 28 deletions(-) -- 2.4.3

2 8

[libvirt] [PATCH] qemu: Mark some functions as static
by Cole Robinson 01 Feb '16

01 Feb '16

--- src/qemu/qemu_command.c | 16 +++++++++++++--- src/qemu/qemu_command.h | 11 ----------- src/qemu/qemu_process.h | 2 -- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d3ab3a..8943270 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1775,6 +1775,16 @@ qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus) } +static int +qemuAssignDevicePCISlots(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainPCIAddressSetPtr addrs); +static int +qemuDomainAssignPCIAddresses(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainObjPtr obj); + + int qemuDomainAssignAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj) @@ -1801,7 +1811,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, } -virDomainPCIAddressSetPtr +static virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, unsigned int nbuses, bool dryRun) @@ -2238,7 +2248,7 @@ qemuValidateDevicePCISlotsChipsets(virDomainDefPtr def, return 0; } -int +static int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj) @@ -2451,7 +2461,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * function must only try to reserve addresses if info.type == NONE and * skip over info.type == PCI */ -int +static int qemuAssignDevicePCISlots(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainPCIAddressSetPtr addrs) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a96e57d..53bfda5 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -274,17 +274,6 @@ void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, const char *devstr); -int qemuDomainAssignPCIAddresses(virDomainDefPtr def, - virQEMUCapsPtr qemuCaps, - virDomainObjPtr obj); -virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, - unsigned int nbuses, - bool dryRun); - -int qemuAssignDevicePCISlots(virDomainDefPtr def, - virQEMUCapsPtr qemuCaps, - virDomainPCIAddressSetPtr addrs); - int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); int qemuDomainNetVLAN(virDomainNetDefPtr def); int qemuAssignDeviceNetAlias(virDomainDefPtr def, virDomainNetDefPtr net, int idx); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index c674111..cb5cee1 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -42,8 +42,6 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver, void qemuProcessAutostartAll(virQEMUDriverPtr driver); void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver); -int qemuProcessAssignPCIAddresses(virDomainDefPtr def); - typedef struct _qemuProcessIncomingDef qemuProcessIncomingDef; typedef qemuProcessIncomingDef *qemuProcessIncomingDefPtr; struct _qemuProcessIncomingDef { -- 2.5.0

2 1

[libvirt] [PATCH] include: Handle case when builddir == srcdir
by Michal Privoznik 01 Feb '16

01 Feb '16

In my previous commit a70f3b1c779120129 I've tried to fix case when building from VPATH and a file wasn't being installed. However, my fix broke non-VPATH build. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- include/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/Makefile.am b/include/Makefile.am index 6b0b41e..754c298 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -19,7 +19,7 @@ virincdir = $(includedir)/libvirt allheaders = $(wildcard $(srcdir)/libvirt/*.h) -virinc_HEADERS = $(filter-out $(srcdir)/libvirt/libvirt-admin.h, $(allheaders)) +virinc_HEADERS = $(filter-out $(srcdir)/libvirt/libvirt-admin.h $(srcdir)/libvirt/libvirt-common.h, $(allheaders)) virinc_HEADERS += libvirt/libvirt-common.h EXTRA_DIST = libvirt/libvirt-common.h.in -- 2.4.10

1 0

[libvirt] [PATCH] qemu: Align dump options for watchdog and on_crash events
by Boris Fiuczynski 01 Feb '16

01 Feb '16

Having on_crash set to either coredump-destroy or coredump-restart creates core dumps with option memory-only in the directory specified by auto_dump_path. When a watchdog is triggered with the action dump the core dump is also placed into the directory specified by auto_dump_path but is created without the option memory-only. This patch sets the option memory-only also for core dumps created by the watchdog event. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f429832..88ecf48 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3894,7 +3894,7 @@ static void processWatchdogEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, in case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: { char *dumpfile; - unsigned int flags = 0; + unsigned int flags = VIR_DUMP_MEMORY_ONLY; if (virAsprintf(&dumpfile, "%s/%s-%u", cfg->autoDumpPath, -- 2.3.0

2 2

[libvirt] [PATCH v2] qemu: return -1 on error paths in qemuDomainSaveImageStartVM
by Nikolay Shirokovskiy 01 Feb '16

01 Feb '16

Error paths after sending the event that domain is started written as if ret = -1 which is set at the beginning of the function. It's common idioma to keep 'ret' equal to -1 until the end of function where it is set to 0. But here we use ret to keep result of restore operation too and thus breaks the idioma and its users :) Let's use different variable to hold restore result. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/qemu/qemu_driver.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 351e529..ced105b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6732,6 +6732,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, qemuDomainAsyncJob asyncJob) { int ret = -1; + bool restored; virObjectEventPtr event; int intermediatefd = -1; virCommandPtr cmd = NULL; @@ -6758,13 +6759,13 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ - ret = qemuProcessStart(conn, driver, vm, asyncJob, - "stdio", *fd, path, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, - VIR_QEMU_PROCESS_START_PAUSED); + restored = qemuProcessStart(conn, driver, vm, asyncJob, + "stdio", *fd, path, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, + VIR_QEMU_PROCESS_START_PAUSED) >= 0; if (intermediatefd != -1) { - if (ret < 0) { + if (!restored) { /* if there was an error setting up qemu, the intermediate * process will wait forever to write to stdout, so we * must manually kill it. @@ -6775,7 +6776,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (virCommandWait(cmd, NULL) < 0) { qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0); - ret = -1; + restored = false; } VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf)); } @@ -6783,18 +6784,16 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (VIR_CLOSE(*fd) < 0) { virReportSystemError(errno, _("cannot close file: %s"), path); - ret = -1; + restored = false; } - if (ret < 0) { - virDomainAuditStart(vm, "restored", false); + virDomainAuditStart(vm, "restored", restored); + if (!restored) goto cleanup; - } event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); - virDomainAuditStart(vm, "restored", true); qemuDomainEventQueue(driver, event); -- 1.8.3.1

3 3

[libvirt] [libvirt-perl][PATCH] Add VIR_STORAGE_VOL_WIPE_ALG_TRIM constant
by Michal Privoznik 01 Feb '16

01 Feb '16

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Changes | 1 + Virt.xs | 1 + lib/Sys/Virt/StorageVol.pm | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/Changes b/Changes index cfcc640..cb57a4d 100644 --- a/Changes +++ b/Changes @@ -2,6 +2,7 @@ Revision history for perl module Sys::Virt 1.3.2 2016-02-00 + - Add VIR_STORAGE_VOL_WIPE_ALG_TRIM constant - Add VIR_FROM_XENXL constant - Add VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION event handling callback diff --git a/Virt.xs b/Virt.xs index e47d52e..06111b4 100644 --- a/Virt.xs +++ b/Virt.xs @@ -8187,6 +8187,7 @@ BOOT: REGISTER_CONSTANT(VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7, WIPE_ALG_PFITZNER7); REGISTER_CONSTANT(VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33, WIPE_ALG_PFITZNER33); REGISTER_CONSTANT(VIR_STORAGE_VOL_WIPE_ALG_RANDOM, WIPE_ALG_RANDOM); + REGISTER_CONSTANT(VIR_STORAGE_VOL_WIPE_ALG_TRIM, WIPE_ALG_TRIM); REGISTER_CONSTANT(VIR_STORAGE_VOL_RESIZE_ALLOCATE, RESIZE_ALLOCATE); REGISTER_CONSTANT(VIR_STORAGE_VOL_RESIZE_DELTA, RESIZE_DELTA); diff --git a/lib/Sys/Virt/StorageVol.pm b/lib/Sys/Virt/StorageVol.pm index 84ba4e0..461a9fe 100644 --- a/lib/Sys/Virt/StorageVol.pm +++ b/lib/Sys/Virt/StorageVol.pm @@ -260,6 +260,10 @@ Cryptography" (1996) 1-pass, all zeroes +=item Sys::Virt::StorageVol::WIPE_ALG_TRIM + +1-pass, trim all data on the volume by using TRIM or DISCARD + =back VOLUME RESIZE CONSTANTS -- 2.4.10

2 1

[libvirt] ABI compatibility report
by Ponomarenko Andrey 01 Feb '16

01 Feb '16

Hello, I've started to maintain ABI compatibility report for the library here: http://abi-laboratory.pro/tracker/timeline/libvirt/ Seems that ABI compatibility is excellent from the beginning of time) Thank you.

2 1

[libvirt] [PATCH] includes: Install libvirt-common.h
by Michal Privoznik 01 Feb '16

01 Feb '16

The libvirt-common.h is build time generated file from .in. Obviously, it's generated into builddir and not srcdir. Problem is, the list of header files to install, virinc_HEADERS contains only $(srcdir)/*.h and this misses libvirt-common.h. This problem is pretty obvious when doing a VPATH build. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Steps to reproduce: 1) make distclean 2) mkdir _build _install 3) cd _build 4) ../configure --prefix=$(pwd)/../_install 5) make && make install 6) find ../_install -name libvirt-common.h include/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/include/Makefile.am b/include/Makefile.am index d626963..6b0b41e 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -20,6 +20,7 @@ virincdir = $(includedir)/libvirt allheaders = $(wildcard $(srcdir)/libvirt/*.h) virinc_HEADERS = $(filter-out $(srcdir)/libvirt/libvirt-admin.h, $(allheaders)) +virinc_HEADERS += libvirt/libvirt-common.h EXTRA_DIST = libvirt/libvirt-common.h.in -- 2.4.10

2 1

[libvirt] [PATCH v3] qemu: qemuDomainRename and virDomainObjListNumOfDomains ABBA deadlock fix
by Nikolay Shirokovskiy 01 Feb '16

01 Feb '16

A pretty nasty deadlock occurs while trying to rename a VM in parallel with virDomainObjListNumOfDomains. The short description of the problem is as follows: Thread #1: qemuDomainRename: ------> aquires domain lock by qemuDomObjFromDomain ---------> waits for domain list lock in any of the listed functions: - virDomainObjListFindByName - virDomainObjListRenameAddNew - virDomainObjListRenameRemove Thread #2: virDomainObjListNumOfDomains: ------> aquires domain list lock ---------> waits for domain lock in virDomainObjListCount Introduce generic virDomainObjListRename function for renaming domains. It aquires list lock in right order to avoid deadlock. Callback is used to make driver specific domain updates. --- src/conf/virdomainobjlist.c | 98 +++++++++++++++++++----------- src/conf/virdomainobjlist.h | 16 +++-- src/libvirt_private.syms | 3 +- src/qemu/qemu_driver.c | 142 ++++++++++++++++++++------------------------ 4 files changed, 142 insertions(+), 117 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 7568f93..6c0be62 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -313,40 +313,6 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, } -int -virDomainObjListRenameAddNew(virDomainObjListPtr doms, - virDomainObjPtr vm, - const char *name) -{ - int ret = -1; - virObjectLock(doms); - - /* Add new name into the hash table of domain names. */ - if (virHashAddEntry(doms->objsName, name, vm) < 0) - goto cleanup; - - /* Okay, this is crazy. virHashAddEntry() does not increment - * the refcounter of @vm, but virHashRemoveEntry() does - * decrement it. We need to work around it. */ - virObjectRef(vm); - - ret = 0; - cleanup: - virObjectUnlock(doms); - return ret; -} - - -int -virDomainObjListRenameRemove(virDomainObjListPtr doms, const char *name) -{ - virObjectLock(doms); - virHashRemoveEntry(doms->objsName, name); - virObjectUnlock(doms); - return 0; -} - - /* * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else @@ -372,6 +338,70 @@ void virDomainObjListRemove(virDomainObjListPtr doms, } +/* + * The caller must hold a lock on dom. Callbacks should not sleep/wait othewise + * operations on all domains will be blocked as the callback is called with + * domains lock hold. Domain lock is dropped/reaquired during this operation + * thus domain consistency must not rely on this lock solely. + */ + +int +virDomainObjListRename(virDomainObjListPtr doms, + virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + virDomainObjListRenameCallback callback, + void *opaque) +{ + int ret = -1; + char *old_name = NULL; + int rc; + + /* doms and dom locks must be attained in right order thus relock dom. */ + /* dom reference is touched for the benefit of those callers that + * hold a lock on dom but not refcount it. */ + virObjectRef(dom); + virObjectUnlock(dom); + virObjectLock(doms); + virObjectLock(dom); + virObjectUnref(dom); + + if (STREQ(dom->def->name, new_name)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Can't rename domain to itself")); + goto cleanup; + } + + if (virHashLookup(doms->objsName, new_name) != NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain with name '%s' already exists"), + new_name); + goto cleanup; + } + + if (virHashAddEntry(doms->objsName, new_name, dom) < 0) + goto cleanup; + + /* Okay, this is crazy. virHashAddEntry() does not increment + * the refcounter of @dom, but virHashRemoveEntry() does + * decrement it. We need to work around it. */ + virObjectRef(dom); + + if (VIR_STRDUP(old_name, dom->def->name) < 0) + goto cleanup; + + rc = callback(dom, new_name, flags, opaque); + virHashRemoveEntry(doms->objsName, rc < 0 ? new_name : old_name); + if (rc < 0) + goto cleanup; + + ret = 0; + cleanup: + virObjectUnlock(doms); + VIR_FREE(old_name); + return ret; +} + /* The caller must hold lock on 'doms' in addition to 'virDomainObjListRemove' * requirements * diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h index f479598..c0f856c 100644 --- a/src/conf/virdomainobjlist.h +++ b/src/conf/virdomainobjlist.h @@ -51,11 +51,17 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, unsigned int flags, virDomainDefPtr *oldDef); -int virDomainObjListRenameAddNew(virDomainObjListPtr doms, - virDomainObjPtr vm, - const char *name); -int virDomainObjListRenameRemove(virDomainObjListPtr doms, - const char *name); +typedef int (*virDomainObjListRenameCallback)(virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + void *opaque); +int virDomainObjListRename(virDomainObjListPtr doms, + virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + virDomainObjListRenameCallback callback, + void *opaque); + void virDomainObjListRemove(virDomainObjListPtr doms, virDomainObjPtr dom); void virDomainObjListRemoveLocked(virDomainObjListPtr doms, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3d0ec9d..3ef3468 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -893,8 +893,7 @@ virDomainObjListNew; virDomainObjListNumOfDomains; virDomainObjListRemove; virDomainObjListRemoveLocked; -virDomainObjListRenameAddNew; -virDomainObjListRenameRemove; +virDomainObjListRename; # cpu/cpu.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abcdbe6..c0e750d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19976,14 +19976,14 @@ qemuDomainSetUserPassword(virDomainPtr dom, } -static int qemuDomainRename(virDomainPtr dom, - const char *new_name, - unsigned int flags) +static int +qemuDomainRenameCallback(virDomainObjPtr vm, + const char *new_name, + unsigned int flags, + void *opaque) { - virQEMUDriverPtr driver = dom->conn->privateData; + virQEMUDriverPtr driver = opaque; virQEMUDriverConfigPtr cfg = NULL; - virDomainObjPtr vm = NULL; - virDomainObjPtr tmp_dom = NULL; virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; @@ -19993,73 +19993,16 @@ static int qemuDomainRename(virDomainPtr dom, virCheckFlags(0, ret); - if (!(vm = qemuDomObjFromDomain(dom))) - goto cleanup; - - if (virDomainRenameEnsureACL(dom->conn, vm->def) < 0) - goto cleanup; - cfg = virQEMUDriverGetConfig(driver); - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - - if (virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot rename active domain")); - goto endjob; - } - - if (!vm->persistent) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot rename a transient domain")); - goto endjob; - } - - if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain has to be shutoff before renaming")); - goto endjob; - } - - if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot rename domain with snapshots")); - goto endjob; - } - - if (STREQ(vm->def->name, new_name)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Can't rename domain to itself")); - goto endjob; - } - - /* - * This is a rather racy check, but still better than reporting - * internal error. And since new_name != name here, there's no - * deadlock imminent. - */ - tmp_dom = virDomainObjListFindByName(driver->domains, new_name); - if (tmp_dom) { - virObjectUnlock(tmp_dom); - virObjectUnref(tmp_dom); - virReportError(VIR_ERR_OPERATION_INVALID, - _("domain with name '%s' already exists"), - new_name); - goto endjob; - } - if (VIR_STRDUP(new_dom_name, new_name) < 0) - goto endjob; + goto cleanup; if (!(old_dom_cfg_file = virDomainConfigFile(cfg->configDir, vm->def->name))) { - goto endjob; + goto cleanup; } - if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) - goto endjob; - event_old = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_RENAMED); @@ -20080,21 +20023,12 @@ static int qemuDomainRename(virDomainPtr dom, goto rollback; } - /* Remove old domain name from table. */ - virDomainObjListRenameRemove(driver->domains, old_dom_name); - event_new = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, VIR_DOMAIN_EVENT_DEFINED_RENAMED); - - /* Success, domain has been renamed. */ ret = 0; - endjob: - qemuDomainObjEndJob(driver, vm); - cleanup: - virDomainObjEndAPI(&vm); VIR_FREE(old_dom_cfg_file); VIR_FREE(old_dom_name); VIR_FREE(new_dom_name); @@ -20109,9 +20043,65 @@ static int qemuDomainRename(virDomainPtr dom, vm->def->name = old_dom_name; old_dom_name = NULL; } + goto cleanup; +} - virDomainObjListRenameRemove(driver->domains, new_name); - goto endjob; +static int qemuDomainRename(virDomainPtr dom, + const char *new_name, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(0, ret); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainRenameEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot rename active domain")); + goto endjob; + } + + if (!vm->persistent) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot rename a transient domain")); + goto endjob; + } + + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain has to be shutoff before renaming")); + goto endjob; + } + + if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot rename domain with snapshots")); + goto endjob; + } + + if (virDomainObjListRename(driver->domains, vm, new_name, flags, + qemuDomainRenameCallback, driver) < 0) + goto endjob; + + /* Success, domain has been renamed. */ + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; } static virHypervisorDriver qemuHypervisorDriver = { -- 1.8.3.1

1 1

[libvirt] [libvirt-php] add flags support to virStorageVolCreateXML
by Vasiliy Tolstov 31 Jan '16

31 Jan '16

Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru> --- src/libvirt-php.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index f3b3f9f81e6d..a0c960957edc 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -1238,6 +1238,8 @@ PHP_MINIT_FUNCTION(libvirt) REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_ALLOCATE", 1, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_DELTA", 2, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_SHRINK", 4, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA", VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_CREATE_REFLINK", VIR_STORAGE_VOL_CREATE_REFLINK, CONST_CS | CONST_PERSISTENT); /* Domain vCPU flags */ REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_CONFIG", VIR_DOMAIN_VCPU_CONFIG, CONST_CS | CONST_PERSISTENT); @@ -7105,6 +7107,7 @@ PHP_FUNCTION(libvirt_storagevolume_get_xml_desc) * Description: Function is used to create the new storage pool and return the handle to new storage pool * Arguments: @res [resource]: libvirt storagepool resource * @xml [string]: XML string to create the storage volume in the storage pool + * @flags [int]: virStorageVolCreateXML flags * Returns: libvirt storagevolume resource */ PHP_FUNCTION(libvirt_storagevolume_create_xml) @@ -7114,11 +7117,12 @@ PHP_FUNCTION(libvirt_storagevolume_create_xml) zval *zpool; virStorageVolPtr volume=NULL; char *xml; + long flags = 0; int xml_len; - GET_STORAGEPOOL_FROM_ARGS("rs",&zpool,&xml,&xml_len); + GET_STORAGEPOOL_FROM_ARGS("rs|l",&zpool,&xml,&xml_len, &flags); - volume=virStorageVolCreateXML(pool->pool,xml,0); + volume=virStorageVolCreateXML(pool->pool, xml, flags); DPRINTF("%s: virStorageVolCreateXML(%p, <xml>, 0) returned %p\n", PHPFUNC, pool->pool, volume); if (volume==NULL) RETURN_FALSE; -- 2.7.0

2 1

[libvirt] [PATCH v2] qemu: qemuDomainRename and virDomainObjListNumOfDomains ABBA deadlock fix
by Nikolay Shirokovskiy 30 Jan '16

30 Jan '16

A pretty nasty deadlock occurs while trying to rename a VM in parallel with virDomainObjListNumOfDomains. The short description of the problem is as follows: Thread #1: qemuDomainRename: ------> aquires domain lock by qemuDomObjFromDomain ---------> waits for domain list lock in any of the listed functions: - virDomainObjListFindByName - virDomainObjListRenameAddNew - virDomainObjListRenameRemove Thread #2: virDomainObjListNumOfDomains: ------> aquires domain list lock ---------> waits for domain lock in virDomainObjListCount Consider another solution for the deadlock besides those proposed by Maxim and Michael. Simply unlock before calling any function that internally grabs domains list lock. First it looks correct. We use qemuDomainObjBeginJob/qemuDomainObjEndJob to wrap the operation so no one will run another operation on that domain in parallel. In THREADS.txt this unlocking/locking is described explicitly for the cases when we need to wait/sleep. Here we use same means just to different purpose. Also called functions do not use domain internals in any way. Second I find approach proposed by Michael difficult to implement correctly. Imagine we write such a generic rename function with callbacks. Then it would look like this: lock domains list ... add new name to list unlock domain list call callback to change driver state lock domain list remove old or new name (depends on callback result) unlock domain list We need to unlock domain list or we block operations on all domains while we wait for job conditions in the callback. But now we are in potential trouble as we leaving list in an inconsistent state. We can't move remove operation before calling callback or we will be in trouble on callback failures - we would need to add the old name back and this is not always possible. Current rename implementation leaves config with new name on disk in worst case on rollback but does not break the list. --- src/qemu/qemu_driver.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abcdbe6..26a8fb0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19987,6 +19987,7 @@ static int qemuDomainRename(virDomainPtr dom, virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; + int rc; char *new_dom_name = NULL; char *old_dom_name = NULL; char *old_dom_cfg_file = NULL; @@ -20036,10 +20037,11 @@ static int qemuDomainRename(virDomainPtr dom, /* * This is a rather racy check, but still better than reporting - * internal error. And since new_name != name here, there's no - * deadlock imminent. + * internal error. */ + virObjectUnlock(vm); tmp_dom = virDomainObjListFindByName(driver->domains, new_name); + virObjectLock(vm); if (tmp_dom) { virObjectUnlock(tmp_dom); virObjectUnref(tmp_dom); @@ -20057,7 +20059,10 @@ static int qemuDomainRename(virDomainPtr dom, goto endjob; } - if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) + virObjectUnlock(vm); + rc = virDomainObjListRenameAddNew(driver->domains, vm, new_name); + virObjectLock(vm); + if (rc < 0) goto endjob; event_old = virDomainEventLifecycleNewFromObj(vm, @@ -20081,7 +20086,9 @@ static int qemuDomainRename(virDomainPtr dom, } /* Remove old domain name from table. */ + virObjectUnlock(vm); virDomainObjListRenameRemove(driver->domains, old_dom_name); + virObjectLock(vm); event_new = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, @@ -20110,7 +20117,9 @@ static int qemuDomainRename(virDomainPtr dom, old_dom_name = NULL; } + virObjectUnlock(vm); virDomainObjListRenameRemove(driver->domains, new_name); + virObjectLock(vm); goto endjob; } -- 1.8.3.1

1 1