- Devel - libvirt List Archives

[libvirt] [PATCH] virNWFilterObjListFree: Don't leak nwfilters->objs
by Michal Privoznik 04 May '17

04 May '17

When adding a nwfilter onto the list in virNWFilterObjListAssignDef() this array is re-allocated to match demand for new size. However, it is never freed leading to a leak: ==26535== 136 bytes in 1 blocks are definitely lost in loss record 1,079 of 1,250 ==26535== at 0x4C2E2BE: realloc (vg_replace_malloc.c:785) ==26535== by 0x54BA28E: virReallocN (viralloc.c:245) ==26535== by 0x54BA384: virExpandN (viralloc.c:294) ==26535== by 0x54BA657: virInsertElementsN (viralloc.c:436) ==26535== by 0x55DB011: virNWFilterObjListAssignDef (virnwfilterobj.c:362) ==26535== by 0x55DB530: virNWFilterObjListLoadConfig (virnwfilterobj.c:503) ==26535== by 0x55DB635: virNWFilterObjListLoadAllConfigs (virnwfilterobj.c:539) ==26535== by 0x2AC5A28B: nwfilterStateInitialize (nwfilter_driver.c:250) ==26535== by 0x5621C64: virStateInitialize (libvirt.c:770) ==26535== by 0x124379: daemonRunStateInit (libvirtd.c:881) ==26535== by 0x554AC78: virThreadHelper (virthread.c:206) ==26535== by 0x8F5F493: start_thread (in /lib64/libpthread-2.23.so) Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/virnwfilterobj.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index c69407a..b5aaa6b 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -110,6 +110,7 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) size_t i; for (i = 0; i < nwfilters->count; i++) virNWFilterObjFree(nwfilters->objs[i]); + VIR_FREE(nwfilters->objs); VIR_FREE(nwfilters); } -- 2.10.2

2 1

[libvirt] [PATCH] virPerfEventIsEnabled: Accept NULL @perf
by Michal Privoznik 04 May '17

04 May '17

After bdcf6e481 there is a crasher in libvirt. The commit assumes that priv->perf is always set. That is not true. For inactive domains, the priv->perf is not allocated as it is set in qemuProcessLaunch(). Now, usually we differentiate between accesses to inactive and active definition and it works just fine. Except for 'domstats'. There priv->perf is accessed without prior check for domain inactivity. While we could check for that, more robust solution is to make virPerfEventIsEnabled() accept NULL. How to reproduce: 1) ensure you have at least one inactive domain 2) virsh domstats Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virperf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virperf.c b/src/util/virperf.c index fa5b6cc..2c832b3 100644 --- a/src/util/virperf.c +++ b/src/util/virperf.c @@ -297,7 +297,7 @@ virPerfEventDisable(virPerfPtr perf, bool virPerfEventIsEnabled(virPerfPtr perf, virPerfEventType type) { - return perf->events[type].enabled; + return perf && perf->events[type].enabled; } int -- 2.10.2

2 2

[libvirt] Entering freeze for libvirt-3.3.0
by Daniel Veillard 04 May '17

04 May '17

As planned, I tagged RC1 in git and pushed signed tarball and rpms to the usual place: ftp://libvirt.org/libvirt/ Seems to work in my limited testing, https://ci.centos.org/view/libvirt/ seems mostly clean and happy, but please give it some testing ! Assuming things go well, I think I will make RC2 on Tuesday (Monday being closed in most of Europe we should have one day of worktime to test) and that would imply a release on Thursday 3rd, assuming nothing critical is found, thanks in advance for your testing and reports :-) Daniel -- Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/ veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

2 3

[libvirt] [PATCH] mdev: Fix mingw build by adding a check for non-NULL pointer
by Erik Skultety 04 May '17

04 May '17

This patch fixes the following MinGW error (although actually being a false positive): ../../src/util/virmdev.c: In function 'virMediatedDeviceListMarkDevices': ../../src/util/virmdev.c:453:21: error: potential null pointer dereference [-Werror=null-dereference] const char *mdev_path = mdev->path; ^~~~~~~~~ Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- Pushed under the build breaker rule. Erik src/util/virmdev.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virmdev.c b/src/util/virmdev.c index c861d21..174f48c 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -449,9 +449,13 @@ virMediatedDeviceListMarkDevices(virMediatedDeviceListPtr dst, virObjectLock(dst); for (i = 0; i < count; i++) { + const char *mdev_path = NULL; virMediatedDevicePtr mdev = virMediatedDeviceListGet(src, i); - const char *mdev_path = mdev->path; + if (!mdev) + goto cleanup; + + mdev_path = mdev->path; if (virMediatedDeviceIsUsed(mdev, dst) || virMediatedDeviceSetUsedBy(mdev, drvname, domname) < 0) goto cleanup; -- 2.9.3

2 2

[libvirt] Raising limits for our RPC messages
by Michal Privoznik 04 May '17

04 May '17

Dear list, maybe you've seen us raising limit for various parts of RPC messages (for eaxmple: d15b29be, 66bfc7cc61ca0, e914dcfd, etc.). It usually happens when we receive a report that current limits are not enough. Well, we just did: https://bugzilla.redhat.com/show_bug.cgi?id=1440683 The thing is, virConnectGetAllDomainStats() is unable to encode RPC message because there's so much domains & stats to send that the limit is reached. Now, I was thinking how to approach this problem. Yes, we can raise the limits again. But we also have another approach: split data into multiple messages. The current limit for an RPC message is 4MB. Say you have 9MB of data you want to send to the other side. We could construct 3 messages (4 + 4 + 1) and send them to the other side where the data would be reconstructed. At RPC level, we could use @status header file to represent intermediate messages. That is, for the example above messages with the following headers would be constructed: {.len = 4MB, prog, vers, proc, .type = VIR_NET_REPLY, .serial, .status = VIR_NET_CONTINUE} {.len = 4MB, prog, vers, proc, .type = VIR_NET_REPLY, .serial, .status = VIR_NET_CONTINUE} {.len = 1MB, prog, vers, proc, .type = VIR_NET_REPLY, .serial, .status = VIR_NET_OK} (the actual @len accounts for message header too, so the message would be split into <nearly 4MB + nearly 4MB + slightly over 1MB> chunks, but that is not important right now) Just like we have in streams, VIR_NET_CONTINUE tells that there is more data yet to come, VIR_NET_OK could then mark the last packet in the stream. Cool you say, this could work. But not really. The problem with this approach is: a) we just transform problem to a different one (the limit of number of messages data can be split into) b) increased overhead (message header is sent multiple times) c) I don't expect we will ever support cherry-picking of messages (caller picks just the message that contains data they are interested in and discards the rest). So after all, we will effectively have the limit of message size increased to $limit_of_split_messages * $current_limit_of_message_size. Instead of introducing complex code that splits data into messages and reconstructs back, we might as well increase the current limit of message size. Michal

3 6

[libvirt] [PATCH 0/2] news: Update for v3.3.0
by Andrea Bolognani 04 May '17

04 May '17

The release notes have been almost completely neglected this cycle, so I've spent some time going through the git log and trying to include what to me looked like relevant changes. Summaries and descriptions are going to be worse that they would be if they were written by the people implementing the changes, but I reckon it's still better than nothing :) Reviews are much appreciated, but if I won't get any feedback I will push this anyway before release just so that v3.3.0 doesn't end up including such sorry excuse for release notes in the final tarball. Andrea Bolognani (2): news: Tweak existing v3.3.0 entries news: Add more v3.3.0 entries docs/news.xml | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 119 insertions(+), 4 deletions(-) -- 2.7.4

4 11

[libvirt] [PATCH 0/2] Fix daemon crash caused by virHostdevUpdateActiveMediatedDevices
by Erik Skultety 04 May '17

04 May '17

Erik Skultety (2): util: mdev: Use a local variable instead of an invalid pointer reference mdev: Fix daemon crash on domain shutdown after reconnect src/util/virhostdev.c | 4 ++-- src/util/virmdev.c | 18 +++++++++++------- src/util/virmdev.h | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) -- 2.12.2

3 7

Re: [libvirt] 答复: Re: 答复: Re: [PATCH] qemu: change the name of tap device for a tapand bridge network
by Laine Stump 04 May '17

04 May '17

(Please don't top-post replies. ALso, I added libvir-list back to the To:) On 05/03/2017 08:46 PM, lu.zhipeng(a)zte.com.cn wrote: > > Some other scenarios such as migration. > > Can we guarantee that the network card driver has been > closed before killing QEMU ? During a migration, before we ever get to the point of killing the source qemu process, we have already stopped the guest CPUs and made sure that the entire state of the guest has been transferred to the qemu process on the destination host. It doesn't matter if we detach the tap device from the bridge or not - any traffic received on the source host during that time (after stopping the guest CPUs) will be dropped (and re-transmitted + received by the guest after it starts running on the destination). Since the guest CPU is already stopped on the source, and the guest on the destination already has the full state of the guest ready to start up (and will not receive any new updates), keeping the tap device connected to the bridge for a small fraction of a second is not going to make any difference at all. Is that convincing enough? :-) > > if we are sure,that's ok > > > > > > *为了让您的VPlat虚拟化故障得到高效的处理，请上报故障到: $VPlat技术支持。* > > 芦志朋 luzhipeng > > > IT开发工程师 IT Development Engineer > 操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R＆D > Institute/System Product > > > > 深圳市南山区科技南路55号中兴通讯研发大楼33楼 > 33/F, R&D Building, ZTE Corporation Hi-tech Road South, > Hi-tech Industrial Park Nanshan District, Shenzhen, P.R.China, 518057 > T: +86 755 xxxxxxxx F:+86 755 xxxxxxxx > M: +86 xxxxxxxxxxx > E: lu.zhipeng(a)zte.com.cn > www.zte.com.cn <http://www.zte.com.cn/> > > 原始邮件 > *发件人：*＜laine(a)laine.org>; > *收件人：*芦志朋10108272; > *日期：*2017年05月04日 08:31 > *主题：**Re: 答复: Re: [libvirt] [PATCH] qemu: change the name of tap > device for a tapand bridge network* > > > > > On May 3, 2017 8:28 PM, ＜lu.zhipeng(a)zte.com.cn > <mailto:lu.zhipeng@zte.com..cn>＞ wrote: > > > I'm worried that the network packet loss rate will increase after > reversing the order. > > > Why? The guest is already shut down (or being shut down). By this time > there is no traffic that would make any difference. And anyway, we've > already determined that the current order causes a race so it is wrong. > > > > > > *为了让您的VPlat虚拟化故障得到高效的处理，请上报故障到: $VPlat技术支 > 持。* > > 芦志朋 luzhipeng > > > IT开发工程师 IT Development Engineer > 操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R＆D > Institute/System Product > > > > 深圳市南山区科技南路55号中兴通讯研发大楼33楼 > 33/F, R&D Building, ZTE Corporation Hi-tech Road South, > Hi-tech Industrial Park Nanshan District, Shenzhen, P.R.China, 518057 > T: +86 755 xxxxxxxx F:+86 755 xxxxxxxx > M: +86 xxxxxxxxxxx > E: lu.zhipeng(a)zte.com.cn <mailto:lu.zhipeng@zte.com.cn> > www.zte.com.cn <http://www.zte.com.cn/> > > 原始邮件 > *发件人：*＜berrange(a)redhat.com <mailto:berrange@redhat.com>＞; > *收件人：*＜laine(a)laine.org <mailto:laine@laine.org>＞; > *抄送人：*＜libvir-list(a)redhat.com <mailto:libvir- > list(a)redhat.com>＞;芦志朋10108272; > *日期：*2017年05月02日 16:30 > *主题：**Re: [libvirt] [PATCH] qemu: change the name of tap device > for a tapand bridge network* > > > On Fri, Apr 28, 2017 at 01:08:45PM -0400, Laine Stump wrote: > ＞ On 04/28/2017 05:33 AM, Daniel P. Berrange wrote: > ＞＞ On Fri, Apr 28, 2017 at 05:23:19PM +0800, ZhiPeng Lu wrote: > ＞＞＞ Creating tap device and adding the device to bridge are not atomic operation. > ＞＞＞ Similarly deleting tap device and removing it from bridge are not atomic operation. > ＞＞＞ The Problem occurs when two vms start and shutdown. When one vm with the nic > ＞＞＞ named "vnet0" stopping, it deleted tap device but not removing port from bridge. > ＞＞＞ At this time, another vm created the tap device named "vnet0" and added port to the > ＞＞＞ same bridge. Then, the first vm deleted the tap device from the same bridge. > ＞＞＞ Finally, the tap device of the second vm don't attached to the bridge. > ＞＞＞ So, we can add domid to vm's nic name. For example, the vm's domid is 1 and vnet0 > ＞＞＞ is renamed to vnet1.0. > ＞＞ > ＞＞ Surely deleting the NIC automatically removes it from the bridge so we > ＞＞ can just remove the code that delets the bridge port. > ＞ > ＞ That is true when using a Linux host bridge, but I recall that for > ＞ openvswitch (which I think is what ZhiPeng is using, based on an earlier > ＞ patch), you must explicitly remove the port from the bridge - apparently > ＞ the port is still there in openvswitch's table as some sort of "zombie" > ＞ connection even after the tap device itself no longer exists. > ＞ > ＞ > ＞ But instead of changing the naming scheme, maybe we should just delete > ＞ the bridge port *before* deleting the tap device instead of after. (Am I > ＞ recalling correctly that the tap device is deleted automatically when > ＞ the qemu process is killed? If so, then what's needed is to move the > ＞ loop in qemuProcessStop() that cleans up network interfaces so that it > ＞ happens before qemuProcessKill() is called. > > Agreed, we should just reverse the order. > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange > <https://www.flickr.com/photos/dberrange> :| > |: https://libvirt.org -o- https://fstop138.berrange.com > <https://fstop138..berrange.com> :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange > <https://www.instagram.com/dberrange> :| > > -- > libvir-list mailing list > libvir-list(a)redhat.com <mailto:libvir-list@redhat.com> > https://www.redhat.com/mailman/listinfo/libvir-list > <https://www.redhat.com/mailman/listinfo/libvir-list> > > > > > >

1 0

[libvirt] [PATCH v1] libxl: report numa sibling distances on host capabilities
by Wim Ten Have 03 May '17

03 May '17

From: Wim ten Have <wim.ten.have(a)oracle.com> When running on a NUMA machine, populate the sibling node and distance information using data supplied by Xen. Wim ten Have (1): libxl: report numa sibling distances on host capabilities src/libxl/libxl_capabilities.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) -- 2.9.3

2 2

[libvirt] [PATCH 0/5] qemu: Couple of namespace fixes
by Michal Privoznik 03 May '17

03 May '17

It was brought to my attention on IRC by Cedric that there's a bug in our namespace code. Namely, if a disk has say /dev/shm/blah source. For more detailed description see the first patch. The rest is just subsequent fixes. Michal Privoznik (5): qemuDomainBuildNamespace: Move /dev/* mountpoints later qemuDomainCreateDeviceRecursive: pass a structure instead of bare path qemuDomainCreateDeviceRecursive: Don't try to create devices under preserved mount points qemuDomainAttachDeviceMknodRecursive: Don't try to create devices under preserved mount points qemuDomainDetachDeviceUnlink: Don't unlink files we haven't created src/qemu/qemu_domain.c | 373 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 275 insertions(+), 98 deletions(-) -- 2.10.2

2 10