- Devel - libvirt List Archives

[libvirt] [PATCH] nwfilter: fix deadlock caused updating network device and nwfilter
by Pavel Hrdina 12 Nov '14

12 Nov '14

Commit 6e5c79a1 tried to fix deadlock between nwfilter{Define,Undefine} and starting of guest, but this same deadlock is also for updating/attaching network device to domain. The deadlock was introduced by removing global QEMU driver lock because nwfilter was counting on this lock and ensure that all driver locks are locked inside of nwfilter{Define,Undefine}. This patch extends usage of virNWFilterReadLockFilterUpdates to prevent the deadlock for all possible paths in QEMU driver. LXC and UML drivers still have global lock. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143780 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- This is temporary fix for the deadlock issue as I'm planning to create global libvirt jobs (similar to QEMU domain jobs) and use it for other drivers and for example for nwfilters too. src/qemu/qemu_driver.c | 12 ++++++++++++ src/qemu/qemu_migration.c | 3 +++ src/qemu/qemu_process.c | 4 ++++ 3 files changed, 19 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6acaea8..9e6f505 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5937,6 +5937,8 @@ qemuDomainRestoreFlags(virConnectPtr conn, def = tmp; } + virNWFilterReadLockFilterUpdates(); + if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -5978,6 +5980,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, virFileWrapperFdFree(wrapperFd); if (vm) virObjectUnlock(vm); + virNWFilterUnlockFilterUpdates(); return ret; } @@ -7502,6 +7505,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG); + virNWFilterReadLockFilterUpdates(); + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -7614,6 +7619,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); + virNWFilterUnlockFilterUpdates(); return ret; } @@ -7644,6 +7650,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); + virNWFilterReadLockFilterUpdates(); + cfg = virQEMUDriverGetConfig(driver); affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG); @@ -7760,6 +7768,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); + virNWFilterUnlockFilterUpdates(); return ret; } @@ -14510,6 +14519,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * and use of FORCE can cause multiple transitions. */ + virNWFilterReadLockFilterUpdates(); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) return -1; @@ -14831,6 +14842,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); + virNWFilterUnlockFilterUpdates(); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 94a4cf6..18242ae 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2666,6 +2666,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto cleanup; } + virNWFilterReadLockFilterUpdates(); + if (!(vm = virDomainObjListAdd(driver->domains, *def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -2825,6 +2827,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, qemuDomainEventQueue(driver, event); qemuMigrationCookieFree(mig); virObjectUnref(caps); + virNWFilterUnlockFilterUpdates(); return ret; stop: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 26d4948..409a672 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3422,6 +3422,8 @@ qemuProcessReconnect(void *opaque) VIR_FREE(data); + virNWFilterReadLockFilterUpdates(); + virObjectLock(obj); cfg = virQEMUDriverGetConfig(driver); @@ -3573,6 +3575,7 @@ qemuProcessReconnect(void *opaque) virObjectUnref(conn); virObjectUnref(cfg); + virNWFilterUnlockFilterUpdates(); return; @@ -3608,6 +3611,7 @@ qemuProcessReconnect(void *opaque) } virObjectUnref(conn); virObjectUnref(cfg); + virNWFilterUnlockFilterUpdates(); } static int -- 2.0.4

3 6

[libvirt] [PATCH] conf: Fix crash when src->hosts = NULL in virStorageFileBackendGlusterInit
by Luyao Huang 12 Nov '14

12 Nov '14

https://bugzilla.redhat.com/show_bug.cgi?id=1162974 When do external snapshot for a gluster disk with no host name(ip) in snapshot xml, libvirtd will crash. Because when node do not have a children in virDomainStorageHostParse, libvirt will return 0, but donnot get hosts for virStorageFileBackendGlusterInit. snpahost.xml: <domainsnapshot> <name>snapshot_test</name> <description>Snapshot Test</description> <disks> <disk name='vda' snapshot='external' type='network'> <source protocol='gluster' name='gluster-vol1/gluster.img.snap'/> </disk> </disks> </domainsnapshot> Back trace: virsh snapshot-create r6 snapshot.xml --disk-only 0 virStorageFileBackendGlusterInit (src=0x7fc760007ca0) at storage/storage_backend_gluster.c:577 1 0x00007fc76d678e22 in virStorageFileInitAs (src=0x7fc760007ca0, uid=uid@entry=4294967295, gid=gid@entry=4294967295) at storage/storage_driver.c:2547 2 0x00007fc76d678e9c in virStorageFileInit (src=<optimized out>) at storage/storage_driver.c:2567 3 0x00007fc76bc13f9c in qemuDomainSnapshotPrepareDiskExternal (reuse=false, active=true, snapdisk=0x7fc7600019b8, disk=0x7fc7641e4880, conn=0x7fc76426cc10) at qemu/qemu_driver.c:12995 4 qemuDomainSnapshotPrepare (flags=<synthetic pointer>, def=0x7fc760002570, vm=0x7fc76422b530, conn=0x7fc76426cc10) at qemu/qemu_driver.c:13156 5 qemuDomainSnapshotCreateXML (domain=0x7fc760001f30, xmlDesc=<optimized out>, flags=16) at qemu/qemu_driver.c:13896 6 0x00007fc782d4de4d in virDomainSnapshotCreateXML (domain=domain@entry=0x7fc760001f30, xmlDesc=0x7fc760001b80 "<domainsnapshot>\n<name>snapshot_test</name>\n<description>Snapshot Test</description>\n<disks>\n<disk name='vda' snapshot='external' type='network'>\n<source protocol='gluster' name='gluster-vol1/gluster."..., flags=16) at libvirt.c:18488 7 0x00007fc7837cb44c in remoteDispatchDomainSnapshotCreateXML (server=<optimized out>, msg=<optimized out>, ret=0x7fc760000a60, args=0x7fc760001f90, rerr=0x7fc77344dc80, client=<optimized out>) at remote_dispatch.h:8605 Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- src/conf/domain_conf.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2c65276..34c1c12 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4150,7 +4150,12 @@ virDomainStorageHostParse(xmlNodePtr node, memset(&host, 0, sizeof(host)); - child = node->children; + if ((child = node->children) == NULL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Can not find a host in xml")); + goto cleanup; + } + while (child != NULL) { if (child->type == XML_ELEMENT_NODE && xmlStrEqual(child->name, BAD_CAST "host")) { -- 1.8.3.1

3 2

[libvirt] [PATCH] Do not crash on gluster snapshots with no host name
by Ján Tomko 12 Nov '14

12 Nov '14

virStorageFileBackendGlusterInit did not check nhosts. https://bugzilla.redhat.com/show_bug.cgi?id=1162974 --- src/storage/storage_backend_gluster.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 8a7d7e5..b79b634 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -571,9 +571,17 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) { virStorageFileBackendGlusterPrivPtr priv = NULL; virStorageNetHostDefPtr host = &(src->hosts[0]); - const char *hostname = host->name; + const char *hostname; int port = 0; + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Expected exactly 1 host for the gluster volume")); + return -1; + } + + hostname = host->name; + VIR_DEBUG("initializing gluster storage file %p (gluster://%s:%s/%s%s)", src, hostname, host->port ? host->port : "0", NULLSTR(src->volume), src->path); -- 2.0.4

2 1

[libvirt] Problem with virInterfaceCreate(), IFF_UP, and NetworkManager
by Laine Stump 11 Nov '14

11 Nov '14

Due to "a checkered past" (a myriad of minor issues, changing over time), libvirt's semi-official position on the virInterface*() APIs and NetworkManager is that virInterface*() is only supported if NM is disabled. We do still attempt to make it work as well as possible, but normally I only test those APIs on systems that have NM disabled and use the network service (RHEL/Fedora/CentOS systems here) instead. On a seemingly unrelated note, a few months ago mprivozn pushed a patch that makes it an error to call virInterfaceCreate() (i.e. "ifup") for an interface that is already active. (the "active" state of an interface is determined by looking at an interface's IFF_UP flag (and also IFF_RUNNING, if the interface isn't a bridge device). Previously, this was allowed, as it is common practive to ifup an interface to make new config take effect. Last week, I happened to test the "virsh iface-bridge" command on a system with NM enabled. That command gave an error about the interface being already active, so I tried again, this time ifdowning the interface in advance - I *still* got the error. Further investigation and questioning of NM developers led me to the realization that when NM is enabled, all interfaces *always* have IFF_UP and IFF_RUNNING set, even if they are ifdowned. Further, if NM is active there is no way to determine an interface's "active" status via iotctl() or netlink; instead, must query to determine if NM is active, and if it is you must call a NM API instead (I got this much information from NM developers directly; haven't investigated yet exactly what the API is). NM developers say that this pinning-up of the IFF_UP flag has been done for a long time, and is necessary to do interface auto-config. I think it is violating a long-standing assumption (if not a standard) about the meaning of IFF_UP, and I'm not convinced that it really is a necessity (certainly once a config file is present for an interface, it shouldn't be needed), but then I haven't spent as much time in that problem space as they have. In the meantime, the virInterfaceCreate() API fails 100% of the time on any system that has NM enabled. My dilemma now is whether to attempt to affect change in NM's use of IFF_UP so that it once again can be used as an indicator of whether or not an interface is active, or to just give in and 1) officially declare that virInterface*() isn't supported if NM is enabled until 2) we add code to netcf that detects when NM is active and learns how to query interface status from NM instead of the standard ioctl(SIOCGIFFLAGS). And if the latter is preferred, should we in the meantime perhaps revert the patch that made virInterfaceCreate() an error if the interface was active? Or just leave it completely broken? Any opinions?

3 3

[libvirt] [PATCH 0/2] Fixes for libssh2
by Cédric Bosdonnat 11 Nov '14

11 Nov '14

Hi all, Here are two fixes to get libssh2 authentication working. Cédric Bosdonnat (2): Fix test wanting a negative size_t Fix handling keyboard-interactive callbacks for libssh2 src/rpc/virnetsshsession.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 1.8.4.5

2 4

[libvirt] [PATCH 0/2] Fix errors on unsuccessful chardev hotplug
by Ján Tomko 11 Nov '14

11 Nov '14

Ján Tomko (2): Fix crash after attempting to hotplug a spicevmc channel Display nicer error message for unsupported chardev hotplug src/conf/domain_conf.c | 3 +-- src/qemu/qemu_monitor_json.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) -- 2.0.4

3 7

[libvirt] [PATCH v7 4/7] qemu: Add bps_max and friends qemu driver
by Matthias Gatto 11 Nov '14

11 Nov '14

Add support for bps_max and friends in the driver part. In the part checking if a qemu is running, check if the running binary support bps_max, if not print an error message, if yes add it to "info" variable This patch follow therse patchs: http://www.redhat.com/archives/libvir-list/2014-November/msg00271.html I've fix the syntax error and the nparams detection problem Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com> --- include/libvirt/libvirt-domain.h | 56 +++++++++++ src/qemu/qemu_driver.c | 197 ++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 10 +- src/qemu/qemu_monitor.h | 6 +- src/qemu/qemu_monitor_json.c | 11 ++- src/qemu/qemu_monitor_json.h | 6 +- tests/qemumonitorjsontest.c | 4 +- 7 files changed, 264 insertions(+), 26 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 966a9ae..1fac2a3 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3251,6 +3251,62 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, # define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC "blkdeviotune.write_iops_sec" /** + * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX: + * + * Marco represents the total throughput limit in maximum bytes per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX "blkdeviotune.total_bytes_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX: + * + * Marco represents the read throughput limit in maximum bytes per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX "blkdeviotune.read_bytes_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX: + * + * Macro represents the write throughput limit in maximum bytes per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX "blkdeviotune.write_bytes_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX: + * + * Macro represents the total maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX "blkdeviotune.total_iops_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX: + * + * Macro represents the read maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX "blkdeviotune.read_iops_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX: + * + * Macro represents the write maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX "blkdeviotune.write_iops_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC: + * + * Macro represents the size maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC "blkdeviotune.size_iops_sec" + +/** * virConnectDomainEventTunableCallback: * @conn: connection object * @dom: domain on which the event occurred diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5eccacc..242b30e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -105,6 +105,7 @@ VIR_LOG_INIT("qemu.qemu_driver"); #define QEMU_NB_MEM_PARAM 3 #define QEMU_NB_BLOCK_IO_TUNE_PARAM 6 +#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX 13 #define QEMU_NB_NUMA_PARAM 2 @@ -16520,6 +16521,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, int conf_idx = -1; bool set_bytes = false; bool set_iops = false; + bool set_bytes_max = false; + bool set_iops_max = false; + bool set_size_iops = false; + bool supportMaxOptions = true; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; virObjectEventPtr event = NULL; @@ -16542,6 +16547,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, NULL) < 0) return -1; @@ -16634,6 +16653,69 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC, param->value.ul) < 0) goto endjob; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX)) { + info.total_bytes_sec_max = param->value.ul; + set_bytes_max = true; + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX, + param->value.ul) < 0) + goto endjob; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX)) { + info.read_bytes_sec_max = param->value.ul; + set_bytes_max = true; + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX, + param->value.ul) < 0) + goto endjob; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX)) { + info.write_bytes_sec_max = param->value.ul; + set_bytes_max = true; + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX, + param->value.ul) < 0) + goto endjob; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX)) { + info.total_iops_sec_max = param->value.ul; + set_iops_max = true; + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX, + param->value.ul) < 0) + goto endjob; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX)) { + info.read_iops_sec_max = param->value.ul; + set_iops_max = true; + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX, + param->value.ul) < 0) + goto endjob; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX)) { + info.write_iops_sec_max = param->value.ul; + set_iops_max = true; + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX, + param->value.ul) < 0) + goto endjob; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC)) { + info.size_iops_sec = param->value.ul; + set_size_iops = true; + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC, + param->value.ul) < 0) + goto endjob; } } @@ -16651,6 +16733,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } + if ((info.total_bytes_sec_max && info.read_bytes_sec_max) || + (info.total_bytes_sec_max && info.write_bytes_sec_max)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of bytes_sec_max cannot be set at the same time")); + goto endjob; + } + + if ((info.total_iops_sec_max && info.read_iops_sec_max) || + (info.total_iops_sec_max && info.write_iops_sec_max)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of iops_sec_max cannot be set at the same time")); + goto endjob; + } + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if ((conf_idx = virDomainDiskIndexByName(persistentDef, disk, true)) < 0) { virReportError(VIR_ERR_INVALID_ARG, @@ -16661,6 +16757,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE); if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("block I/O throttling not supported with this " @@ -16668,6 +16765,13 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } + if (!supportMaxOptions && (set_iops_max || set_bytes_max)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("a block I/O throttling parameter is not supported with this " + "QEMU binary")); + goto endjob; + } + if (!(device = qemuDiskPathToAlias(vm, disk, &idx))) goto endjob; @@ -16680,13 +16784,25 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, info.read_bytes_sec = oldinfo->read_bytes_sec; info.write_bytes_sec = oldinfo->write_bytes_sec; } + if (!set_bytes_max) { + info.total_bytes_sec_max = oldinfo->total_bytes_sec_max; + info.read_bytes_sec_max = oldinfo->read_bytes_sec_max; + info.write_bytes_sec_max = oldinfo->write_bytes_sec_max; + } if (!set_iops) { info.total_iops_sec = oldinfo->total_iops_sec; info.read_iops_sec = oldinfo->read_iops_sec; info.write_iops_sec = oldinfo->write_iops_sec; } + if (!set_iops_max) { + info.total_iops_sec_max = oldinfo->total_iops_sec_max; + info.read_iops_sec_max = oldinfo->read_iops_sec_max; + info.write_iops_sec_max = oldinfo->write_iops_sec_max; + } + if (!set_size_iops) + info.size_iops_sec = oldinfo->size_iops_sec; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info); + ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, supportMaxOptions); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; @@ -16730,6 +16846,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } endjob: + if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; @@ -16753,13 +16870,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - qemuDomainObjPrivatePtr priv; + qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo reply; char *device = NULL; int ret = -1; size_t i; virCapsPtr caps = NULL; + bool supportMaxOptions = true; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -16777,13 +16895,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if ((*nparams) == 0) { - /* Current number of parameters supported by QEMU Block I/O Throttling */ - *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; - ret = 0; - goto cleanup; - } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -16791,6 +16902,18 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, &persistentDef) < 0) goto endjob; + if ((*nparams) == 0) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + priv = vm->privateData; + /* If the VM is running, we can check if the current VM can use optional parameters or not */ + /* We didn't made this check sooner because we need the VM data to do so */ + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX); + } + *nparams = supportMaxOptions ? QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX : QEMU_NB_BLOCK_IO_TUNE_PARAM; + ret = 0; + goto endjob; + } + device = qemuDiskPathToAlias(vm, disk, NULL); if (!device) { goto endjob; @@ -16799,7 +16922,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply); + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; @@ -16816,7 +16939,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, reply = persistentDef->disks[idx]->blkdeviotune; } - for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM && i < *nparams; i++) { + for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX && i < *nparams; i++) { virTypedParameterPtr param = &params[i]; switch (i) { @@ -16862,14 +16985,64 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, reply.write_iops_sec) < 0) goto endjob; break; + case 6: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.total_bytes_sec_max) < 0) + goto endjob; + break; + case 7: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.read_bytes_sec_max) < 0) + goto endjob; + break; + case 8: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.write_bytes_sec_max) < 0) + goto endjob; + break; + case 9: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.total_iops_sec_max) < 0) + goto endjob; + break; + case 10: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.read_iops_sec_max) < 0) + goto endjob; + break; + case 11: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.write_iops_sec_max) < 0) + goto endjob; + break; + case 12: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.size_iops_sec) < 0) + goto endjob; /* coverity[dead_error_begin] */ default: break; } } - if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM) + if (!supportMaxOptions && *nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM) *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; + else if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX) + *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX; ret = 0; endjob: diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 36ef4e1..276649d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3514,14 +3514,15 @@ qemuMonitorBlockJobInfo(qemuMonitorPtr mon, int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info) + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions) { int ret; VIR_DEBUG("mon=%p, device=%p, info=%p", mon, device, info); if (mon->json) { - ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info); + ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info, supportMaxOptions); } else { ret = qemuMonitorTextSetBlockIoThrottle(mon, device, info); } @@ -3530,14 +3531,15 @@ int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply) + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions) { int ret; VIR_DEBUG("mon=%p, device=%p, reply=%p", mon, device, reply); if (mon->json) { - ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply); + ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply, supportMaxOptions); } else { ret = qemuMonitorTextGetBlockIoThrottle(mon, device, reply); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 750b3dc..76c91a3 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -771,11 +771,13 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon, int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info); + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions); int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply); + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions); int qemuMonitorSystemWakeup(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7870664..c506c31 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4293,7 +4293,8 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, static int qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, const char *device, - virDomainBlockIoTuneInfoPtr reply) + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions ATTRIBUTE_UNUSED) { virJSONValuePtr io_throttle; int ret = -1; @@ -4364,7 +4365,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info) + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions ATTRIBUTE_UNUSED) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -4404,7 +4406,8 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply) + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -4431,7 +4434,7 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, } if (ret == 0) - ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply); + ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply, supportMaxOptions); virJSONValueFree(cmd); virJSONValueFree(result); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 6cdaf18..3b6159a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -325,11 +325,13 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info); + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions); int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply); + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions); int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ad33c10..1b202fb 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1847,7 +1847,7 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) goto cleanup; if (qemuMonitorJSONGetBlockIoThrottle(qemuMonitorTestGetMonitor(test), - "drive-virtio-disk0", &info) < 0) + "drive-virtio-disk0", &info, false) < 0) goto cleanup; if (memcmp(&info, &expectedInfo, sizeof(info) != 0)) { @@ -1857,7 +1857,7 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) } if (qemuMonitorJSONSetBlockIoThrottle(qemuMonitorTestGetMonitor(test), - "drive-virtio-disk1", &info) < 0) + "drive-virtio-disk1", &info, false) < 0) goto cleanup; ret = 0; -- 1.8.3.1

3 3

[libvirt] [PATCH V2 0/3] Fix some problems of numatune
by Wang Rui 11 Nov '14

11 Nov '14

Fix startup failing with memory mode(strict, preferred or interleave) in numatune V1: https://www.redhat.com/archives/libvir-list/2014-November/msg00057.html V2 has been revised as Martin' suggestion. Wang Rui (3): qemu: don't setup cpuset.mems if memory mode in numatune is not 'strict' lxc: don't setup cpuset.mems if memory mode in numatune is not 'strict' qemu: fix domain startup failing with 'strict' mode in numatune src/lxc/lxc_cgroup.c | 4 ++++ src/qemu/qemu_cgroup.c | 14 +++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) -- 1.7.12.4

2 4

[libvirt] [libvirt-python][PATCH] Add dict check for setTime and allow pass 'seconds' parameter
by Michal Privoznik 11 Nov '14

11 Nov '14

From: Luyao Huang <lhuang(a)redhat.com> When pass None or a empty dictionary to time, it will report error. This commit allows a one-element dictionary which contains just 'seconds' field, which results in the same as passing 0 for 'nseconds' field. Moreover, dict is checked for unknown fields. Signed-off-by: Luyao Huang <lhuang(a)redhat.com> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- libvirt-override-virDomain.py | 4 ++-- libvirt-override.c | 39 +++++++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py index a50ec0d..fa5f75f 100644 --- a/libvirt-override-virDomain.py +++ b/libvirt-override-virDomain.py @@ -67,8 +67,8 @@ return ret def setTime(self, time=None, flags=0): - """Set guest time to the given value. @time is a dict conatining - 'seconds' field for seconds and 'nseconds' field for nanosecons """ + """Set guest time to the given value. @time is a dict containing + 'seconds' field for seconds and 'nseconds' field for nanoseconds """ ret = libvirtmod.virDomainSetTime(self._o, time, flags) if ret == -1: raise libvirtError ('virDomainSetTime() failed', dom=self) return ret diff --git a/libvirt-override.c b/libvirt-override.c index c01e52f..57f0ccf 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -7785,12 +7785,14 @@ static PyObject * libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval = NULL; PyObject *pyobj_domain; + PyObject *pyobj_seconds; + PyObject *pyobj_nseconds; PyObject *py_dict; virDomainPtr domain; long long seconds = 0; unsigned int nseconds = 0; unsigned int flags; - ssize_t py_dict_size; + ssize_t py_dict_size = 0; int c_retval; if (!PyArg_ParseTuple(args, (char*)"OOI:virDomainSetTime", @@ -7798,26 +7800,31 @@ libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); - py_dict_size = PyDict_Size(py_dict); - - if (py_dict_size == 2) { - PyObject *pyobj_seconds, *pyobj_nseconds; - - if (!(pyobj_seconds = PyDict_GetItemString(py_dict, "seconds")) || - (libvirt_longlongUnwrap(pyobj_seconds, &seconds) < 0)) { - PyErr_Format(PyExc_LookupError, "malformed or missing 'seconds'"); + if (PyDict_Check(py_dict)) { + py_dict_size = PyDict_Size(py_dict); + if ((pyobj_seconds = PyDict_GetItemString(py_dict, "seconds"))) { + if (libvirt_longlongUnwrap(pyobj_seconds, &seconds) < 0) { + PyErr_Format(PyExc_LookupError, "malformed 'seconds'"); + goto cleanup; + } + } else { + PyErr_Format(PyExc_LookupError, "Dictionary must contains 'seconds'"); goto cleanup; } - if (!(pyobj_nseconds = PyDict_GetItemString(py_dict, "nseconds")) || - (libvirt_uintUnwrap(pyobj_nseconds, &nseconds) < 0)) { - PyErr_Format(PyExc_LookupError, "malformed or missing 'nseconds'"); + if ((pyobj_nseconds = PyDict_GetItemString(py_dict, "nseconds"))) { + if (libvirt_uintUnwrap(pyobj_nseconds, &nseconds) < 0) { + PyErr_Format(PyExc_LookupError, "malformed 'nseconds'"); + goto cleanup; + } + } else if (py_dict_size > 1) { + PyErr_Format(PyExc_LookupError, "Dictionary contains unknown key"); goto cleanup; } - } else if (py_dict_size > 0) { - PyErr_Format(PyExc_LookupError, "Dictionary must contain " - "'seconds' and 'nseconds'"); - goto cleanup; + } else if (py_dict != Py_None || !flags) { + PyErr_Format(PyExc_TypeError, "time must be a dictionary " + "or None with flags set"); + return NULL; } LIBVIRT_BEGIN_ALLOW_THREADS; -- 2.0.4

2 1

[libvirt] [PATCH] selinux: ignore missing imagelabel in SetSavedStateLabel
by Ján Tomko 11 Nov '14

11 Nov '14

If we have no imagelabel to set (e.g. the domain was created by qemu-attach where we don't generate an imagelabel) return success instead of crashing. https://bugzilla.redhat.com/show_bug.cgi?id=1161831 --- src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index f96be50..db0df7d 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1970,7 +1970,7 @@ virSecuritySELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virSecurityLabelDefPtr secdef; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); - if (!secdef || !secdef->relabel) + if (!secdef || !secdef->relabel || !secdef->imagelabel) return 0; return virSecuritySELinuxSetFilecon(savefile, secdef->imagelabel); -- 2.0.4

2 1