September 2017 - Devel - libvirt List Archives

Re: [libvirt] [Qemu-devel] [PATCH 1/2] vl: Eliminate defconfig variable
by Eduardo Habkost 28 Sep '17

28 Sep '17

(CCing libvir-list) On Thu, Sep 28, 2017 at 12:42:02PM +0100, Daniel P. Berrange wrote: > On Wed, Sep 27, 2017 at 05:32:24PM -0300, Eduardo Habkost wrote: > > Both -nodefconfig and -no-user-config options do the same thing > > today, we only need one variable to keep track of them. > > What reason for picking -nodefconfig instead of -no-user-config to > deprecate ? -nodefconfig predates -no-user-config by a few years, > and is what libvirt has historically used. So from libvirt POV > we'd have a slight preference to deprecate -no-user-config instead > and keep -nodefconfig, simply to avoid need to add conditional > logic to libvirt to pick which to use. libvirt already prefers -no-user-config, which is the right thing for libvirt because it needs a mechanism to disable user-provided configuration only, not QEMU-provided data files (in case they exist). In other words, we have an important use case for the -no-user-config semantics in libvirt, and I am not aware of any existing user of the -nodefconfig semantics. (For the record, I think the existing documented -nodefconfig semantics is awful and useless, but that's the one we got. Old discussions about this can be seen at: https://www.mail-archive.com/libvir-list@redhat.com/msg53083.html ) > > > > > Suggested-by: Markus Armbruster <armbru(a)redhat.com> > > Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com> > > --- > > vl.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/vl.c b/vl.c > > index 4fd01fda91..b347a97a5b 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -3093,7 +3093,6 @@ int main(int argc, char **argv, char **envp) > > const char *qtest_log = NULL; > > const char *pid_file = NULL; > > const char *incoming = NULL; > > - bool defconfig = true; > > bool userconfig = true; > > bool nographic = false; > > DisplayType display_type = DT_DEFAULT; > > @@ -3194,8 +3193,6 @@ int main(int argc, char **argv, char **envp) > > popt = lookup_opt(argc, argv, &optarg, &optind); > > switch (popt->index) { > > case QEMU_OPTION_nodefconfig: > > - defconfig = false; > > - break; > > case QEMU_OPTION_nouserconfig: > > userconfig = false; > > break; > > @@ -3203,7 +3200,7 @@ int main(int argc, char **argv, char **envp) > > } > > } > > > > - if (defconfig && userconfig) { > > + if (userconfig) { > > if (qemu_read_default_config_file() < 0) { > > exit(1); > > } > > -- > > 2.13.5 > > > > > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- Eduardo

1 0

[libvirt] [PATCH] nwfilter: Fix possible segfault on sometimes consumed variable
by John Ferlan 28 Sep '17

28 Sep '17

The virNWFilterIPAddrMapAddIPAddr code can consume the @addr parameter on success when the @ifname is found in the ipAddressMap->hashTable hash table in the call to virNWFilterVarValueAddValue; however, if not found in the hash table, then @addr is formatted into a @val which is stored in the table and on return the caller would be expected to free @addr. Thus, the caller has no way to determine on success whether @addr was consumed, so in order to fix this create a @tmp variable which will be stored/consumed when virNWFilterVarValueAddValue succeeds. That way the caller can free @addr whether the function returns success or failure. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/conf/nwfilter_ipaddrmap.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/conf/nwfilter_ipaddrmap.c b/src/conf/nwfilter_ipaddrmap.c index 9c8584ce2..5668f366d 100644 --- a/src/conf/nwfilter_ipaddrmap.c +++ b/src/conf/nwfilter_ipaddrmap.c @@ -26,7 +26,9 @@ #include "internal.h" +#include "viralloc.h" #include "virerror.h" +#include "virstring.h" #include "datatypes.h" #include "nwfilter_params.h" #include "nwfilter_ipaddrmap.h" @@ -52,6 +54,7 @@ virNWFilterIPAddrMapAddIPAddr(const char *ifname, char *addr) { int ret = -1; virNWFilterVarValuePtr val; + char *tmp = NULL; virMutexLock(&ipAddressMapLock); @@ -65,14 +68,18 @@ virNWFilterIPAddrMapAddIPAddr(const char *ifname, char *addr) virNWFilterVarValueFree(val); goto cleanup; } else { - if (virNWFilterVarValueAddValue(val, addr) < 0) + if (VIR_STRDUP(tmp, addr) < 0) goto cleanup; + if (virNWFilterVarValueAddValue(val, tmp) < 0) + goto cleanup; + tmp = NULL; } ret = 0; cleanup: virMutexUnlock(&ipAddressMapLock); + VIR_FREE(tmp); return ret; } -- 2.13.5

2 1

[libvirt] [PATCH] qemu: add the print of page size in cmd domjobinfo
by Chao Fan 28 Sep '17

28 Sep '17

The command "info migrate" of qemu outputs the dirty-pages-rate during migration, but page size is different in different architectures. So page size should be output to calculate dirty pages in bytes. Page size is already implemented with commit 030ce1f8612215fcbe9d353dfeaeb2937f8e3f94 in qemu. Now Implement the counter-part in libvirt. Signed-off-by: Chao Fan <fanc.fnst(a)cn.fujitsu.com> --- include/libvirt/libvirt-domain.h | 10 +++++++++- src/qemu/qemu_domain.c | 5 +++++ src/qemu/qemu_migration_cookie.c | 7 +++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 2 ++ tools/virsh-domain.c | 8 ++++++++ 6 files changed, 32 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 030a62c43..b05c9d762 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3327,7 +3327,8 @@ typedef enum { */ # define VIR_DOMAIN_JOB_MEMORY_BPS "memory_bps" -/** VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE: +/** + * VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE: * * virDomainGetJobStats field: number of memory pages dirtied by the guest * per second, as VIR_TYPED_PARAM_ULLONG. This statistics makes sense only @@ -3336,6 +3337,13 @@ typedef enum { # define VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE "memory_dirty_rate" /** + * VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE: + * + * virDomainGetJobStats field: page size of the memory in this domian + */ +# define VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE "page_size" + +/** * VIR_DOMAIN_JOB_MEMORY_ITERATION: * * virDomainGetJobStats field: current iteration over domain's memory diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cb371f1e8..9194e70f8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -571,6 +571,11 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, goto error; if (virTypedParamsAddULLong(&par, &npar, &maxpar, + VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE, + stats->ram_page_size) < 0) + goto error; + + if (virTypedParamsAddULLong(&par, &npar, &maxpar, VIR_DOMAIN_JOB_DISK_TOTAL, stats->disk_total + mirrorStats->total) < 0 || diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index eef40a6cd..bc6a8dc55 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -654,6 +654,10 @@ qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf, stats->ram_iteration); virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n", + VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE, + stats->ram_page_size); + + virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n", VIR_DOMAIN_JOB_DISK_TOTAL, stats->disk_total); virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n", @@ -1014,6 +1018,9 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) virXPathULongLong("string(./" VIR_DOMAIN_JOB_MEMORY_ITERATION "[1])", ctxt, &stats->ram_iteration); + virXPathULongLong("string(./" VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE "[1])", + ctxt, &stats->ram_page_size); + virXPathULongLong("string(./" VIR_DOMAIN_JOB_DISK_TOTAL "[1])", ctxt, &stats->disk_total); virXPathULongLong("string(./" VIR_DOMAIN_JOB_DISK_PROCESSED "[1])", diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6414d2483..1e3322433 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -677,6 +677,7 @@ struct _qemuMonitorMigrationStats { unsigned long long ram_normal; unsigned long long ram_normal_bytes; unsigned long long ram_dirty_rate; + unsigned long long ram_page_size; unsigned long long ram_iteration; unsigned long long disk_transferred; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 63b855920..625cbc134 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2892,6 +2892,8 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, &stats->ram_normal_bytes)); ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-pages-rate", &stats->ram_dirty_rate)); + ignore_value(virJSONValueObjectGetNumberUlong(ram, "page-size", + &stats->ram_page_size)); ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-sync-count", &stats->ram_iteration)); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a3f3b7c7b..a50713d6e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6021,6 +6021,14 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) } if ((rc = virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE, + &value)) < 0) { + goto save_error; + } else if (rc) { + vshPrint(ctl, "%-17s %-12llu bytes\n", _("Page size:"), value); + } + + if ((rc = virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_JOB_MEMORY_ITERATION, &value)) < 0) { goto save_error; -- 2.13.5

2 2

[libvirt] [PATCH] rpc: for messages with FDs always decode count of FDs from the message
by Pavel Hrdina 27 Sep '17

27 Sep '17

The packet with passed FD has the following format: -------------------------- | len | header | payload | -------------------------- where "payload" has an additional count of FDs before the actual data: ------------------ | nfds | payload | ------------------ When the packet is received we parse the "header", which as a side effect updates msg->bufferOffset to point to the beginning of "payload". If the message call contains FDs, we need to also parse the count of FDs, which also updates the msg->bufferOffset. The issue here is that when we attempt to read the FDs data from the socket and we receive EAGAIN we finish the reading and call poll() to wait for the data the we need. When the data arrives we already have the packet in our buffer so we read the "header" again but this time we don't read the count of FDs because we already have it stored. That means that the msg->bufferOffset is not updated to point to the actual beginning of the payload data, but it points to the count of FDs. After all FDs are processed we dispatch the message to process it and decode the payload. Since the msg->bufferOffset points to wrong data, we decode the wrong payload and the API call fails with error messages: Domain not found: no domain with matching uuid '67656e65-7269-6300-0c87-5003ca6941f2' () Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/rpc/virnetclient.c | 3 +-- src/rpc/virnetmessage.c | 12 +++++++----- src/rpc/virnetserverclient.c | 3 +-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 95cd9a6c7e..eb46e34301 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1428,8 +1428,7 @@ virNetClientIOHandleInput(virNetClientPtr client) if (client->msg.header.type == VIR_NET_REPLY_WITH_FDS) { size_t i; - if (client->msg.nfds == 0 && - virNetMessageDecodeNumFDs(&client->msg) < 0) + if (virNetMessageDecodeNumFDs(&client->msg) < 0) return -1; for (i = client->msg.donefds; i < client->msg.nfds; i++) { diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index 5908b074a8..94c4c89e4f 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -327,11 +327,13 @@ int virNetMessageDecodeNumFDs(virNetMessagePtr msg) goto cleanup; } - msg->nfds = numFDs; - if (VIR_ALLOC_N(msg->fds, msg->nfds) < 0) - goto cleanup; - for (i = 0; i < msg->nfds; i++) - msg->fds[i] = -1; + if (msg->nfds == 0) { + msg->nfds = numFDs; + if (VIR_ALLOC_N(msg->fds, msg->nfds) < 0) + goto cleanup; + for (i = 0; i < msg->nfds; i++) + msg->fds[i] = -1; + } VIR_DEBUG("Got %zu FDs from peer", msg->nfds); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index fa4e5daabb..6e086b7b4e 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1189,8 +1189,7 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client) /* Now figure out if we need to read more data to get some * file descriptors */ if (msg->header.type == VIR_NET_CALL_WITH_FDS) { - if (msg->nfds == 0 && - virNetMessageDecodeNumFDs(msg) < 0) { + if (virNetMessageDecodeNumFDs(msg) < 0) { virNetMessageQueueServe(&client->rx); virNetMessageFree(msg); client->wantClose = true; -- 2.13.5

2 2

[libvirt] [PATCH] virsh: Add/allow secret-uuid for pool-{define|create}-as
by John Ferlan 27 Sep '17

27 Sep '17

https://bugzilla.redhat.com/show_bug.cgi?id=1476775 For the virsh pool-{define|create}-as command, let's allow using --secret-uuid on the command line as an alternative to --secret-usage, but ensure that they are mutually exclusive. Not sure why I neglected to add it for commit id '8932580' Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- tools/virsh-pool.c | 16 +++++++++++++--- tools/virsh.pod | 9 ++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index ba9281f..03eae94 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -109,6 +109,10 @@ .type = VSH_OT_STRING, \ .help = N_("auth secret usage to be used for underlying storage") \ }, \ + {.name = "secret-uuid", \ + .type = VSH_OT_STRING, \ + .help = N_("auth secret UUID to be used for underlying storage") \ + }, \ {.name = "adapter-name", \ .type = VSH_OT_STRING, \ .help = N_("adapter name to be used for underlying storage") \ @@ -302,9 +306,11 @@ virshBuildPoolXML(vshControl *ctl, *srcDev = NULL, *srcName = NULL, *srcFormat = NULL, *target = NULL, *authType = NULL, *authUsername = NULL, *secretUsage = NULL, *adapterName = NULL, *adapterParent = NULL, - *adapterWwnn = NULL, *adapterWwpn = NULL; + *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid"); + if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) goto cleanup; if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) @@ -319,6 +325,7 @@ virshBuildPoolXML(vshControl *ctl, vshCommandOptStringReq(ctl, cmd, "auth-type", &authType) < 0 || vshCommandOptStringReq(ctl, cmd, "auth-username", &authUsername) < 0 || vshCommandOptStringReq(ctl, cmd, "secret-usage", &secretUsage) < 0 || + vshCommandOptStringReq(ctl, cmd, "secret-uuid", &secretUUID) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-name", &adapterName) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-wwnn", &adapterWwnn) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-wwpn", &adapterWwpn) < 0 || @@ -349,11 +356,14 @@ virshBuildPoolXML(vshControl *ctl, virBufferAsprintf(&buf, "<adapter type='scsi_host' name='%s'/>\n", adapterName); } - if (authType && authUsername && secretUsage) { + if (authType && authUsername && (secretUsage || secretUUID)) { virBufferAsprintf(&buf, "<auth type='%s' username='%s'>\n", authType, authUsername); virBufferAdjustIndent(&buf, 2); - virBufferAsprintf(&buf, "<secret usage='%s'/>\n", secretUsage); + if (secretUsage) + virBufferAsprintf(&buf, "<secret usage='%s'/>\n", secretUsage); + else + virBufferAsprintf(&buf, "<secret uuid='%s'/>\n", secretUUID); virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</auth>\n"); } diff --git a/tools/virsh.pod b/tools/virsh.pod index c13f96f..bf5a124 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3673,7 +3673,8 @@ just I<--build> is provided, then B<pool-build> is called with no flags. =item B<pool-create-as> I<name> I<type> [I<--source-host hostname>] [I<--source-path path>] [I<--source-dev path>] [I<--source-name name>] [I<--target path>] [I<--source-format format>] -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] +[I<--auth-type authtype> I<--auth-username username> +[I<--secret-usage usage> | I<--secret-uuid uuid>]] [[I<--adapter-name name>] | [I<--adapter-wwnn> I<--adapter-wwpn>] [I<--adapter-parent parent>]] [I<--build>] [[I<--overwrite>] | [I<--no-overwrite>]] [I<--print-xml>] @@ -3707,10 +3708,12 @@ the host file system. [I<--source-format format>] provides information about the format of the pool (pool types fs, netfs, disk, logical). -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] +[I<--auth-type authtype> I<--auth-username username> +[I<--secret-usage usage> | I<--secret-uuid uuid>]] provides the elements required to generate authentication credentials for the storage pool. The I<authtype> is either chap for iscsi I<type> pools or -ceph for rbd I<type> pools. +ceph for rbd I<type> pools. Either the secret I<usage> or I<uuid> value may +be provided, but not both. [I<--adapter-name name>] defines the scsi_hostN adapter name to be used for the scsi_host adapter type pool. -- 2.9.5

2 3

[libvirt] [PATCH v9 0/4] Add support for Veritas HyperScale (VxHS) block device protocol
by John Ferlan 27 Sep '17

27 Sep '17

v8: https://www.redhat.com/archives/libvir-list/2017-September/msg00375.html Changes since v8 * Patches 1-6 patches, everything else deals with TLS. * Patches 7-8 - no change, now patch 1-2. Didn't push since they're related to these changes * Patch 9 - Old patch Was ACK'd, new patch is 3. Old patch11 review caused me to add more fields to _virStorageSource to pepare "tlsCertdir", "tlsListen", and "tlsVerify" using the cfg field for certdir, but direct sets for listen and verify. * Patch 10 - dropped * Patch 11 - now patch 4. Alter the qemu_command.c and qemu_hotplug.c code to use the virStorageSource fields rather than from disk or cfg. Code makes more use of whether the "src->haveTLS == VIR_TRISTATE_BOOL_YES" is true rather then src->tlsAlias. Added qemuDomainDelDiskSrcTLSObject. Renamed/reworked the qemuDomainAddDiskSrcTLSObject. Looks cleaner to me... Ashish will post a patch separately to fix an issue found in qemuDomainGetTLSObjects when secAlias == NULL and the deref of *secAlias. Ashish Mittal (3): conf: Introduce TLS options for VxHS block device clients util: Add TLS attributes to virStorageSource qemu: Add TLS support for Veritas HyperScale (VxHS) John Ferlan (1): qemu: Introduce qemuDomainPrepareDiskSource docs/formatdomain.html.in | 40 ++++++++--- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 29 +++++++- src/qemu/libvirtd_qemu.aug | 4 ++ src/qemu/qemu.conf | 34 ++++++++++ src/qemu/qemu_block.c | 8 +++ src/qemu/qemu_command.c | 33 +++++++++ src/qemu/qemu_conf.c | 16 +++++ src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 73 ++++++++++++++++++++ src/qemu/qemu_domain.h | 11 +++ src/qemu/qemu_hotplug.c | 79 ++++++++++++++++++++++ src/qemu/qemu_process.c | 4 ++ src/qemu/test_libvirtd_qemu.aug.in | 2 + src/util/virstoragefile.c | 11 ++- src/util/virstoragefile.h | 15 ++++ ...-disk-drive-network-tlsx509-multidisk-vxhs.args | 43 ++++++++++++ ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 50 ++++++++++++++ ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 30 ++++++++ ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +++++++++ tests/qemuxml2argvtest.c | 7 ++ ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++++++++++ tests/qemuxml2xmltest.c | 1 + 23 files changed, 551 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml -- 2.13.5

3 18

[libvirt] [PATCH] qemu: domain: Extract common clearing of VM private data
by Peter Krempa 27 Sep '17

27 Sep '17

VM private data is cleared when the VM is turned off and also when the VM object is being freed. Some of the clearing code was duplicated. Extract it to a separate function. This also removes the now unnecessary function qemuDomainClearPrivatePaths. --- src/qemu/qemu_domain.c | 76 +++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_domain.h | 4 +-- src/qemu/qemu_process.c | 36 ++--------------------- 3 files changed, 55 insertions(+), 61 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cb371f1e8..7cb2bfce1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1684,16 +1684,6 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, } -void -qemuDomainClearPrivatePaths(virDomainObjPtr vm) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - - VIR_FREE(priv->libDir); - VIR_FREE(priv->channelTargetDir); -} - - static void * qemuDomainObjPrivateAlloc(void *opaque) { @@ -1721,24 +1711,69 @@ qemuDomainObjPrivateAlloc(void *opaque) return NULL; } -static void -qemuDomainObjPrivateFree(void *data) +/** + * qemuDomainObjPrivateDataClear: + * @priv: domain private data + * + * Clears private data entries, which are not necessary or stale if the VM is + * not running. + */ +void +qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) { - qemuDomainObjPrivatePtr priv = data; + virStringListFree(priv->qemuDevices); + priv->qemuDevices = NULL; + + virCgroupFree(&priv->cgroup); + + virPerfFree(priv->perf); + priv->perf = NULL; + + VIR_FREE(priv->machineName); virObjectUnref(priv->qemuCaps); + priv->qemuCaps = NULL; - virBitmapFree(priv->namespaces); + VIR_FREE(priv->pidfile); - virCgroupFree(&priv->cgroup); + VIR_FREE(priv->libDir); + VIR_FREE(priv->channelTargetDir); + + /* remove automatic pinning data */ + virBitmapFree(priv->autoNodeset); + priv->autoNodeset = NULL; + virBitmapFree(priv->autoCpuset); + priv->autoCpuset = NULL; + + /* remove address data */ virDomainPCIAddressSetFree(priv->pciaddrs); + priv->pciaddrs = NULL; virDomainUSBAddressSetFree(priv->usbaddrs); + priv->usbaddrs = NULL; + + /* clean up migration data */ + VIR_FREE(priv->migTLSAlias); + virCPUDefFree(priv->origCPU); + priv->origCPU = NULL; + + /* clear previously used namespaces */ + virBitmapFree(priv->namespaces); + priv->namespaces = NULL; +} + + +static void +qemuDomainObjPrivateFree(void *data) +{ + qemuDomainObjPrivatePtr priv = data; + + qemuDomainObjPrivateDataClear(priv); + virDomainChrSourceDefFree(priv->monConfig); qemuDomainObjFreeJob(priv); VIR_FREE(priv->lockState); VIR_FREE(priv->origname); - virStringListFree(priv->qemuDevices); virChrdevFree(priv->devs); /* This should never be non-NULL if we get here, but just in case... */ @@ -1751,19 +1786,10 @@ qemuDomainObjPrivateFree(void *data) qemuAgentClose(priv->agent); } VIR_FREE(priv->cleanupCallbacks); - virBitmapFree(priv->autoNodeset); - virBitmapFree(priv->autoCpuset); - - VIR_FREE(priv->machineName); - VIR_FREE(priv->libDir); - VIR_FREE(priv->channelTargetDir); qemuDomainSecretInfoFree(&priv->migSecinfo); - VIR_FREE(priv->migTLSAlias); qemuDomainMasterKeyFree(priv); - virCPUDefFree(priv->origCPU); - VIR_FREE(priv); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 09201b1a4..cdf94b2f6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -676,6 +676,8 @@ void qemuDomainCleanupRemove(virDomainObjPtr vm, void qemuDomainCleanupRun(virQEMUDriverPtr driver, virDomainObjPtr vm); +void qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv); + extern virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks; extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace; extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig; @@ -783,8 +785,6 @@ int qemuDomainNetVLAN(virDomainNetDefPtr def); int qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, virDomainObjPtr vm); -void qemuDomainClearPrivatePaths(virDomainObjPtr vm); - virDomainDiskDefPtr qemuDomainDiskByName(virDomainDefPtr def, const char *name); char *qemuDomainGetMasterKeyFilePath(const char *libDir); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c104985aa..0a2493770 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6196,8 +6196,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); - qemuDomainClearPrivatePaths(vm); - ignore_value(virDomainChrDefForeach(vm->def, false, qemuProcessCleanupChardevDevice, @@ -6248,9 +6246,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, VIR_FREE(vm->def->seclabels[i]->imagelabel); } - virStringListFree(priv->qemuDevices); - priv->qemuDevices = NULL; - qemuHostdevReAttachDomainDevices(driver, vm->def); def = vm->def; @@ -6319,10 +6314,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, VIR_WARN("Failed to remove cgroup for %s", vm->def->name); } - virCgroupFree(&priv->cgroup); - - virPerfFree(priv->perf); - priv->perf = NULL; qemuProcessRemoveDomainStatus(driver, vm); @@ -6376,37 +6367,14 @@ void qemuProcessStop(virQEMUDriverPtr driver, } } - VIR_FREE(priv->machineName); - vm->taint = 0; vm->pid = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); for (i = 0; i < vm->def->niothreadids; i++) vm->def->iothreadids[i]->thread_id = 0; - virObjectUnref(priv->qemuCaps); - priv->qemuCaps = NULL; - VIR_FREE(priv->pidfile); - /* remove automatic pinning data */ - virBitmapFree(priv->autoNodeset); - priv->autoNodeset = NULL; - virBitmapFree(priv->autoCpuset); - priv->autoCpuset = NULL; - - /* remove address data */ - virDomainPCIAddressSetFree(priv->pciaddrs); - priv->pciaddrs = NULL; - virDomainUSBAddressSetFree(priv->usbaddrs); - priv->usbaddrs = NULL; - - /* clean up migration data */ - VIR_FREE(priv->migTLSAlias); - virCPUDefFree(priv->origCPU); - priv->origCPU = NULL; - - /* clear previously used namespaces */ - virBitmapFree(priv->namespaces); - priv->namespaces = NULL; + /* clear all private data entries which are no longer needed */ + qemuDomainObjPrivateDataClear(priv); /* The "release" hook cleans up additional resources */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { -- 2.14.1

2 1

[libvirt] [PATCH python] Release the GIL during virDomainGetMemoryStats
by Nir Soffer 27 Sep '17

27 Sep '17

We discovered that the entire python process get stuck for about 30 seconds when calling virDomain.getMemoryStats() if libvirt is stuck in virConnect.getAllDomainStats() on inaccessible storage. This blocking cause a horrible mess in oVirt. This patches adds the standard *_ALLOW_THREADS around the call to avoid this unwanted blocking. Signed-off-by: Nir Soffer <nirsof(a)gmail.com> --- libvirt-override.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libvirt-override.c b/libvirt-override.c index 9eba4ed..0e33afb 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -363,8 +363,11 @@ libvirt_virDomainMemoryStats(PyObject *self ATTRIBUTE_UNUSED, return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + LIBVIRT_BEGIN_ALLOW_THREADS; nr_stats = virDomainMemoryStats(domain, stats, VIR_DOMAIN_MEMORY_STAT_NR, 0); + LIBVIRT_END_ALLOW_THREADS; + if (nr_stats == -1) return VIR_PY_NONE; -- 2.9.5

2 2

[libvirt] [PATCH v3] nwfilter: Don't leak @inetaddr
by ZhiPeng Lu 27 Sep '17

27 Sep '17

In learnIPAddressThread()the @inetaddr may be leaked. Signed-off-by: ZhiPeng Lu <lu.zhipeng(a)zte.com.cn> --- src/nwfilter/nwfilter_learnipaddr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index cfd92d9..0cadf73 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -605,6 +605,7 @@ learnIPAddressThread(void *arg) if (req->status == 0) { int ret; + int mapipret = -1; virSocketAddr sa; sa.len = sizeof(sa.data.inet4); sa.data.inet4.sin_family = AF_INET; @@ -622,7 +623,7 @@ learnIPAddressThread(void *arg) virNWFilterUnlockIface(req->ifname); if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) { - if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) { + if ((mapipret = virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr)) < 0) { VIR_ERROR(_("Failed to add IP address %s to IP address " "cache for interface %s"), inetaddr, req->ifname); } @@ -637,6 +638,9 @@ learnIPAddressThread(void *arg) req->filterparams); VIR_DEBUG("Result from applying firewall rules on " "%s with IP addr %s : %d", req->ifname, inetaddr, ret); + if (mapipret < 0) + VIR_FREE(inetaddr); + } } else { if (showError) -- 1.8.3.1

3 3

[libvirt] [PATCH v2] nwfilter: Don't leak @inetaddr
by ZhiPeng Lu 27 Sep '17

27 Sep '17

In learnIPAddressThread()the @inetaddr may be leaked. Signed-off-by: ZhiPeng Lu <lu.zhipeng(a)zte.com.cn> --- src/nwfilter/nwfilter_learnipaddr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index cfd92d9..5dc212e 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -625,6 +625,7 @@ learnIPAddressThread(void *arg) if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) { VIR_ERROR(_("Failed to add IP address %s to IP address " "cache for interface %s"), inetaddr, req->ifname); + VIR_FREE(inetaddr); } ret = virNWFilterInstantiateFilterLate(req->driver, @@ -636,7 +637,8 @@ learnIPAddressThread(void *arg) req->filtername, req->filterparams); VIR_DEBUG("Result from applying firewall rules on " - "%s with IP addr %s : %d", req->ifname, inetaddr, ret); + "%s with IP addr %s : %d", req->ifname, NULLSTR(inetaddr), ret); + } } else { if (showError) -- 1.8.3.1

3 2