- Devel - libvirt List Archives

[libvirt] [PATCH] libvirt-python : PyObject memory leak
by Peng Hao 07 Dec '17

07 Dec '17

libvirt_virConnectDomainEventTunableCallback leak a PyObject Signed-off-by: Peng Hao <peng.hao2(a)zte.com.cn> Signed-off-by: Wang Yechao <wang.yechao255(a)zte.com.cn> --- libvirt-override.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libvirt-override.c b/libvirt-override.c index d746350..eab9b62 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -6665,6 +6665,7 @@ libvirt_virConnectDomainEventTunableCallback(virConnectPtr conn ATTRIBUTE_UNUSED Py_XDECREF(pyobj_dict); } else { Py_DECREF(pyobj_ret); + Py_DECREF(pyobj_dict); ret = 0; } -- 1.8.3.1

2 1

[libvirt] [REBASE PATCH v2 00/10] Implement query-dump command
by John Ferlan 07 Dec '17

07 Dec '17

Original v2: https://www.redhat.com/archives/libvir-list/2017-November/msg00779.html Rebase due to additional capabilitiies v2 Cover: v1: https://www.redhat.com/archives/libvir-list/2017-November/msg00731.html Differences from v1 (besides more patches) Rather than use a timing loop in order to fetch the dump stats and the presence of the 'query-dump' command, let's use the DUMP_COMPLETED event and wire that up in order to determine when the dump has been completed. In the mean time, the previous timing loop is converted into a qemuDomainGetJobInfoDumpStats to be called if the "right conditions" exist. Also found/fixed a couple of minor issues from v1 regarding usage "dumpformat" instead of just "format" from my patch 2 adjustment and a type for qemuMonitorDumpStatus (was qemuMontiorDumpStatus in v1). NB: (from v1 cover): Details in the patches. Essentially though QEMU 2.6 added the ability to perform the 'dump-guest-memory' via a thread by adding a 'detach' boolean to the command. In order to watch the progress of the thread the 'query-dump' command was added. The query-dump will return just the current status, the total size to be dumped, and the current progress. So using the migrate stats data in the DomainJobInfo we can then save our current progress allowing tools such as virsh to watch that progress. As an added benefit, the dump-guest-memory is now truly asynchronous. John Ferlan (10): qemu: Clean up style for the qemuDumpToFd definition qemu: Alter dump-guest-memory command generation qemu: Add support for DUMP_COMPLETED event qemu: Introduce qemuProcessHandleDumpCompleted qemu: Introduce qemuMonitor[JSON]QueryDump qemu: Add new parameter to qemuMonitorDumpToFd qemu: Introduce qemuDomainGetJobInfoMigrationStats qemu: Introduce qemuDomainGetJobInfoDumpStats qemu: Add dump completed event to the capabilities qemu: Allow showing the dump progress for memory only dump src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 157 ++++++++++++++++++--- src/qemu/qemu_monitor.c | 39 ++++- src/qemu/qemu_monitor.h | 33 ++++- src/qemu/qemu_monitor_json.c | 122 ++++++++++++++-- src/qemu/qemu_monitor_json.h | 7 +- src/qemu/qemu_process.c | 23 +++ .../caps_2.10.0-gicv2.aarch64.xml | 1 + .../caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + tests/qemumonitorjsontest.c | 3 +- 27 files changed, 365 insertions(+), 42 deletions(-) -- 2.13.6

2 12

[libvirt] [PATCHv2 0/7] Remove some repetitive code from qemuDomainNamespace* functions
by Ján Tomko 07 Dec '17

07 Dec '17

Rebased + rename path to paths in one more function. Ján Tomko (7): qemuDomainNamespaceTeardownHostdev: rename path to paths qemuDomainNamespaceSetupHostdev: rename path to paths Introduce qemuDomainNamespaceMknodPaths Introduce qemuDomainNamespaceMknodPath Introduce qemuDomainNamespaceUnlinkPaths Introduce qemuDomainNamespaceUnlinkPath Do not pass driver to qemuDomainNamespace{Setup,Teardown} functions src/qemu/qemu_domain.c | 394 ++++++++++++++++++------------------------------ src/qemu/qemu_domain.h | 30 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 42 +++--- 4 files changed, 176 insertions(+), 292 deletions(-) -- 2.13.6

3 11

[libvirt] [PATCH] qemuDomainNamespaceSetupDisk: initialize npaths
by Ján Tomko 07 Dec '17

07 Dec '17

Introduced by commit d3db304. Reported-by: John Ferlan <jferlan(a)redhat.com> --- Pushed as trivial. src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 75cecb1bb..d7150cae1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10034,7 +10034,7 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm, { virStorageSourcePtr next; char **paths = NULL; - size_t npaths; + size_t npaths = 0; int ret = -1; if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) -- 2.13.6

1 0

[libvirt] [PATCH] docs: domain: Fix documentation of the 'snapshot' attribute for <disk>
by Peter Krempa 07 Dec '17

07 Dec '17

Emphasise the valid values by wrapping them in <code> and reword the last sentence so that the invalid value example can be dropped. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1523070 --- docs/formatdomain.html.in | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 29a9681467..01db83e608 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2584,17 +2584,16 @@ <dt><code>snapshot</code></dt> <dd> Indicates the default behavior of the disk during disk snapshots: - "internal" requires a file format such as qcow2 that can store - both the snapshot and the data changes since the snapshot; - "external" will separate the snapshot from the live data; and - "no" means the disk will not participate in snapshots. Read-only - disks default to "no", while the default for other disks depends - on the hypervisor's capabilities. Some hypervisors allow a - per-snapshot choice as well, during + "<code>internal</code>" requires a file format such as qcow2 that + can store both the snapshot and the data changes since the snapshot; + "<code>external</code>" will separate the snapshot from the live + data; and "<code>no</code>" means the disk will not participate in + snapshots. Read-only disks default to "<code>no</code>", while the + default for other disks depends on the hypervisor's capabilities. + Some hypervisors allow a per-snapshot choice as well, during <a href="formatsnapshot.html">domain snapshot creation</a>. - Not all snapshot modes are supported; for example, - <code>snapshot='yes'</code> with a transient disk generally - does not make sense. + Not all snapshot modes are supported; for example, enabling + snapshots with a transient disk generally does not make sense. <span class="since">Since 0.9.5</span> </dd> </dl> -- 2.15.0

2 1

[libvirt] [PATCH] virerror: mark VIR_ERR_AGENT_UNSYNCED as DEPRECATED
by Chen Hanxiao 07 Dec '17

07 Dec '17

From: Chen Hanxiao <chenhanxiao(a)gmail.com> Since commit 5e5019bf, we've no longer use VIR_ERR_AGENT_UNSYNCED anymore. Mark it as DEPRECATED. Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com> --- include/libvirt/virterror.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index d2b5f1fd4..91ba29784 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -318,7 +318,7 @@ typedef enum { VIR_ERR_NO_SERVER = 95, /* Server was not found */ VIR_ERR_NO_CLIENT = 96, /* Client was not found */ VIR_ERR_AGENT_UNSYNCED = 97, /* guest agent replies with wrong id - to guest-sync command */ + to guest-sync command (DEPRECATED)*/ VIR_ERR_LIBSSH = 98, /* error in libssh transport driver */ } virErrorNumber; -- 2.14.3

2 3

[libvirt] Snapshot: Request to review patch
by Madhu Pavan 07 Dec '17

07 Dec '17

Hi all, Could someone please review https://www.redhat.com/archives/libvir-list/2017-October/msg01333.html This patch is based on comments from Jiri Denemark ( https://www.redhat.com/archives/libvir-list/2017-October/msg01275.html ) This patch potentially fixes https://bugzilla.redhat.com/show_bug.cgi?id=1494471 Thanks, Madhu.

2 1

[libvirt] [PATCH] storage: Don't dereference driver object if virStorageSource is not initialized
by Peter Krempa 07 Dec '17

07 Dec '17

virStorageFileReportBrokenChain uses data from the driver private data pointer to print the user and group. This would lead to a crash in call paths where we did not initialize the storage backend as recently added in commit 24e47ee2b93 to qemuDomainDetermineDiskChain. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1522682 --- Once pushed I'll also backport it to a maint branch of the last release if that's desired. src/storage/storage_source.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index 4586ef4ad4..a5df84bae3 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -419,19 +419,33 @@ virStorageFileReportBrokenChain(int errcode, virStorageSourcePtr src, virStorageSourcePtr parent) { - unsigned int access_user = src->drv->uid; - unsigned int access_group = src->drv->gid; - - if (src == parent) { - virReportSystemError(errcode, - _("Cannot access storage file '%s' " - "(as uid:%u, gid:%u)"), - src->path, access_user, access_group); + + if (src->drv) { + unsigned int access_user = src->drv->uid; + unsigned int access_group = src->drv->gid; + + if (src == parent) { + virReportSystemError(errcode, + _("Cannot access storage file '%s' " + "(as uid:%u, gid:%u)"), + src->path, access_user, access_group); + } else { + virReportSystemError(errcode, + _("Cannot access backing file '%s' " + "of storage file '%s' (as uid:%u, gid:%u)"), + src->path, parent->path, access_user, access_group); + } } else { - virReportSystemError(errcode, - _("Cannot access backing file '%s' " - "of storage file '%s' (as uid:%u, gid:%u)"), - src->path, parent->path, access_user, access_group); + if (src == parent) { + virReportSystemError(errcode, + _("Cannot access storage file '%s' "), + src->path); + } else { + virReportSystemError(errcode, + _("Cannot access backing file '%s' " + "of storage file '%s'"), + src->path, parent->path); + } } } -- 2.15.0

2 2

[libvirt] [PATCH php v2] Fix crash in VIRT_HASH_CURRENT_KEY_INFO macro
by Dawid Zamirski 07 Dec '17

07 Dec '17

The PHP7 variant of the macro wasn't safe if the hash key was not a string type. This was found when running php script with just libvirt_connect call under xdebug session which segfaulted. This patch makes the following changes: * make sure that tmp_name is initialized to NULL * set the key name only when zend_hash_get_current_key_ex did set it to something which happens only when type is HASH_KEY_IS_STRING * stash the key index in out php_libvirt_hash_key_info struct because it wasn't there before and separate variable had to be used. --- v1: https://www.redhat.com/archives/libvir-list/2017-December/msg00151.html Changes since v1: * use zend_ulong in php_libvirt_hash_key_info struct so that no type cast is needed src/libvirt-connection.c | 8 +++----- src/libvirt-php.c | 6 ++---- src/libvirt-php.h | 1 + src/util.h | 16 +++++++++------- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/libvirt-connection.c b/src/libvirt-connection.c index 181b266..2d59d82 100644 --- a/src/libvirt-connection.c +++ b/src/libvirt-connection.c @@ -131,8 +131,6 @@ PHP_FUNCTION(libvirt_connect) HashPosition pointer; int array_count; - zend_ulong index; - unsigned long libVer; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|sba", &url, &url_len, &readonly, &zcreds) == FAILURE) { @@ -176,13 +174,13 @@ PHP_FUNCTION(libvirt_connect) VIRT_FOREACH(arr_hash, pointer, data) { if (Z_TYPE_P(data) == IS_STRING) { php_libvirt_hash_key_info info; - VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, info); + VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, info); if (info.type == HASH_KEY_IS_STRING) { PHPWRITE(info.name, info.length); } else { - DPRINTF("%s: credentials index %d\n", PHPFUNC, (int)index); - creds[j].type = index; + DPRINTF("%s: credentials index %d\n", PHPFUNC, info.index); + creds[j].type = info.index; creds[j].result = (char *)emalloc(Z_STRLEN_P(data) + 1); memset(creds[j].result, 0, Z_STRLEN_P(data) + 1); creds[j].resultlen = Z_STRLEN_P(data); diff --git a/src/libvirt-php.c b/src/libvirt-php.c index ef057fe..efbef58 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -1921,7 +1921,6 @@ long get_next_free_numeric_value(virDomainPtr domain, char *xpath) HashPosition pointer; // int array_count; zval *data; - unsigned long index; long max_slot = -1; xml = virDomainGetXMLDesc(domain, VIR_DOMAIN_XML_INACTIVE); @@ -1934,7 +1933,7 @@ long get_next_free_numeric_value(virDomainPtr domain, char *xpath) VIRT_FOREACH(arr_hash, pointer, data) { if (Z_TYPE_P(data) == IS_STRING) { php_libvirt_hash_key_info info; - VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, info); + VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, info); if (info.type != HASH_KEY_IS_STRING) { long num = -1; @@ -2439,7 +2438,6 @@ void parse_array(zval *arr, tVMDisk *disk, tVMNetwork *network) zval *data; php_libvirt_hash_key_info key; HashPosition pointer; - unsigned long index; arr_hash = Z_ARRVAL_P(arr); //array_count = zend_hash_num_elements(arr_hash); @@ -2451,7 +2449,7 @@ void parse_array(zval *arr, tVMDisk *disk, tVMNetwork *network) VIRT_FOREACH(arr_hash, pointer, data) { if ((Z_TYPE_P(data) == IS_STRING) || (Z_TYPE_P(data) == IS_LONG)) { - VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, key); + VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, key); if (key.type == HASH_KEY_IS_STRING) { if (disk != NULL) { if ((Z_TYPE_P(data) == IS_STRING) && strcmp(key.name, "path") == 0) diff --git a/src/libvirt-php.h b/src/libvirt-php.h index 8d13a6b..aea43a2 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -137,6 +137,7 @@ typedef struct tVMNetwork { typedef struct _php_libvirt_hash_key_info { char *name; unsigned int length; + zend_ulong index; unsigned int type; } php_libvirt_hash_key_info; diff --git a/src/util.h b/src/util.h index ecb3a1f..fcd4075 100644 --- a/src/util.h +++ b/src/util.h @@ -135,12 +135,14 @@ # define VIRT_FOREACH_END(_dummy) -# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \ +# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _info) \ do { \ - zend_string *tmp_key_info; \ - _info.type = zend_hash_get_current_key_ex(_ht, &tmp_key_info, &_idx, &_pos); \ - _info.name = ZSTR_VAL(tmp_key_info); \ - _info.length = ZSTR_LEN(tmp_key_info); \ + zend_string *tmp_name = NULL; \ + _info.type = zend_hash_get_current_key_ex(_ht, &tmp_name, &_info.index, &_pos); \ + if (tmp_name) { \ + _info.name = ZSTR_VAL(tmp_name); \ + _info.length = ZSTR_LEN(tmp_name); \ + } \ } while(0) # define VIRT_ARRAY_INIT(_name) do { \ @@ -213,9 +215,9 @@ # define VIRT_FOREACH_END(_dummy) \ }} -# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \ +# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _info) \ do { \ - _info.type = zend_hash_get_current_key_ex(_ht, &_info.name, &_info.length, &_idx, 0, &_pos); \ + _info.type = zend_hash_get_current_key_ex(_ht, &_info.name, &_info.length, &_info.index, 0, &_pos); \ } while(0) # define VIRT_ARRAY_INIT(_name) do {\ -- 2.14.3

2 1

[libvirt] [PATCH] nwfilter: fix deadlock on firewalld restart/reload
by Nikolay Shirokovskiy 07 Dec '17

07 Dec '17

Deadlock scenario: 1. domain is being started and domain driver takes read lock for nwfilter update lock thru virNWFilterReadLockFilterUpdates. 2. firewalld restarted and event loop hangs trying to take write lock for nwfilter update lock in nwfilterStateReload. This is deadlock because for thread 1 to proceed we need a response from qemu/timeout but whole event loop is stuck by 2. Let's just offload nwfilter driver reload to a distinct thread. Besides resolving a deadlock this change offloads rather heavy operation (many hypervisor drivers X many domains for a driver) off the event loop. --- Concrete stacktrace. Thread 17 (Thread 0x7fbd5a22d880 (LWP 7329)): 0 0x00007fbd56bb903e in pthread_rwlock_wrlock () from /lib64/libpthread.so.0 1 0x00007fbd3d511297 in nwfilterStateReload () from /usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so 2 0x00007fbd3d511355 in nwfilterFirewalldDBusFilter () from /usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so 3 0x00007fbd57bc5dfe in dbus_connection_dispatch () from /lib64/libdbus-1.so.3 4 0x00007fbd5953e641 in virDBusWatchCallback () from /lib64/libvirt.so.0 5 0x00007fbd5954868e in virEventPollRunOnce () from /lib64/libvirt.so.0 6 0x00007fbd59547942 in virEventRunDefaultImpl () from /lib64/libvirt.so.0 7 0x00007fbd596a94ed in virNetDaemonRun () from /lib64/libvirt.so.0 8 0x00007fbd5a28b07c in main () Thread 3 (Thread 0x7fbd49553700 (LWP 8029)): 0 0x00007fbd56bb96d5 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 1 0x00007fbd59596816 in virCondWait () from /lib64/libvirt.so.0 2 0x00007fbd3d0183db in qemuMonitorSend () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 3 0x00007fbd3d02c8d0 in qemuMonitorJSONCommandWithFd () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 4 0x00007fbd3d02e1a1 in qemuMonitorJSONSetCapabilities () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 5 0x00007fbd3cff965c in qemuConnectMonitor () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 6 0x00007fbd3cffcb70 in qemuProcessWaitForMonitor () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 7 0x00007fbd3d004668 in qemuProcessLaunch () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 8 0x00007fbd3d0062a8 in qemuProcessStart () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 9 0x00007fbd3d0679cb in qemuDomainObjStart.constprop.50 () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 10 0x00007fbd3d068106 in qemuDomainCreateWithFlags () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 11 0x00007fbd5964247d in virDomainCreateWithFlags () from /lib64/libvirt.so.0 12 0x00007fbd5a2a3b81 in remoteDispatchDomainCreateWithFlagsHelper () 13 0x00007fbd596af3c3 in virNetServerProgramDispatch () from /lib64/libvirt.so.0 14 0x00007fbd5a2c56cd in virNetServerHandleJob () 15 0x00007fbd59597221 in virThreadPoolWorker () from /lib64/libvirt.so.0 16 0x00007fbd595965a8 in virThreadHelper () from /lib64/libvirt.so.0 17 0x00007fbd56bb5dc5 in start_thread () from /lib64/libpthread.so.0 18 0x00007fbd568e473d in clone () from /lib64/libc.so.6 src/nwfilter/nwfilter_driver.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 2f9a51c..bcb4400 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -81,6 +81,12 @@ static void nwfilterDriverUnlock(void) #if HAVE_FIREWALLD +static void +nwfilterStateReloadAdapter(void *opaque ATTRIBUTE_UNUSED) +{ + nwfilterStateReload(); +} + static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, @@ -90,8 +96,12 @@ nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { + virThread thread; + VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); - nwfilterStateReload(); + if (virThreadCreate(&thread, false, nwfilterStateReloadAdapter, NULL) < 0) + VIR_ERROR(_("Could not create thread. Network filter " + "driver reload failed")); } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1

2 2