July 2014 - Devel - libvirt List Archives

[libvirt] [PATCH v2] util: forbid freeing const pointers
by Eric Blake 16 Jul '14

16 Jul '14

Now that we've finally fixed all the violators, it's time to enforce that any pointer to a const object is never freed (it is aliasing some other memory, where the non-const original should be freed instead). Alas, the code still needs a normal vs. Coverity version, but at least we are still guaranteeing that the macro call evaluates its argument exactly once. I verified that we still get the following compiler warnings, which in turn halts the build thanks to -Werror on gcc (hmm, gcc 4.8.3's placement of the ^ for ?: type mismatch is a bit off, but that's not our problem): int oops1 = 0; VIR_FREE(oops1); const char *oops2 = NULL; VIR_FREE(oops2); struct blah { int dummy; } oops3; VIR_FREE(oops3); util/virauthconfig.c:159:35: error: pointer/integer type mismatch in conditional expression [-Werror] VIR_FREE(oops1); ^ util/virauthconfig.c:161:5: error: passing argument 1 of 'virFree' discards 'const' qualifier from pointer target type [-Werror] VIR_FREE(oops2); ^ In file included from util/virauthconfig.c:28:0: util/viralloc.h:79:6: note: expected 'void *' but argument is of type 'const void *' void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); ^ util/virauthconfig.c:163:35: error: type mismatch in conditional expression VIR_FREE(oops3); ^ * src/util/viralloc.h (VIR_FREE): No longer cast away const. * src/xenapi/xenapi_utils.c (xenSessionFree): Work around bogus header. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- v2: this depends on the existing 1/4, while being a replacement to all of 2-4/4 at once. https://www.redhat.com/archives/libvir-list/2014-July/msg00716.html src/util/viralloc.h | 11 +++++------ src/xenapi/xenapi_utils.c | 4 +++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 7125e67..bf85c16 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -548,18 +548,17 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * This macro is safe to use on arguments with side effects. */ # if !STATIC_ANALYSIS -/* The ternary ensures that ptr is a pointer and not an integer type, - * while evaluating ptr only once. This gives us extra compiler - * safety when compiling under gcc. For now, we intentionally cast - * away const, since a number of callers safely pass const char *. +/* The ternary ensures that ptr is a non-const pointer and not an + * integer type, all while evaluating ptr only once. This gives us + * extra compiler safety when compiling under gcc. */ -# define VIR_FREE(ptr) virFree((void *) (1 ? (const void *) &(ptr) : (ptr))) +# define VIR_FREE(ptr) virFree(1 ? (void *) &(ptr) : (ptr)) # else /* The Coverity static analyzer considers the else path of the "?:" and * flags the VIR_FREE() of the address of the address of memory as a * RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(&ptr)) */ -# define VIR_FREE(ptr) virFree((void *) &(ptr)) +# define VIR_FREE(ptr) virFree(&(ptr)) # endif void virAllocTestInit(void); diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c index a80d136..ef89f42 100644 --- a/src/xenapi/xenapi_utils.c +++ b/src/xenapi/xenapi_utils.c @@ -44,13 +44,15 @@ void xenSessionFree(xen_session *session) { size_t i; + char *tmp; if (session->error_description != NULL) { for (i = 0; i < session->error_description_count; i++) VIR_FREE(session->error_description[i]); VIR_FREE(session->error_description); } /* The session_id member is type of 'const char *'. Sigh. */ - VIR_FREE(session->session_id); + tmp = (char *)session->session_id; + VIR_FREE(tmp); VIR_FREE(session); } -- 1.9.3

2 2

[libvirt] [PATCH 0/6] Fix regression with relative backing names in storage pools
by Peter Krempa 16 Jul '14

16 Jul '14

Our recent refactors broke relative backing names in storage pools, fix it partially. Peter Krempa (6): storage: backend: Fix formatting of function arguments storage: Track backing store of a volume in the target struct storage: backend: fs: Touch up coding style storage: fs: Process backing store data in virStorageBackendProbeTarget storage: fs: Properly parse backing store info storage: fs: Don't fail volume update if backing store isn't accessible src/conf/storage_conf.c | 53 +++++++++-------- src/conf/storage_conf.h | 1 - src/storage/storage_backend.c | 40 ++++++------- src/storage/storage_backend_fs.c | 103 ++++++++++++++++++---------------- src/storage/storage_backend_gluster.c | 20 +++++-- src/storage/storage_backend_logical.c | 11 ++-- 6 files changed, 129 insertions(+), 99 deletions(-) -- 2.0.0

2 13

[libvirt] [PATCH 0/4] fix virReportSystemError misuse
by Jincheng Miao 16 Jul '14

16 Jul '14

virReportSystemError() reports some OS errors, and the first argument of it should be the error number defined in errno.h. virReportError() reports libvirt errors, and the first argument should be the error number defined in virerrno.h. This patch set fix some misuse of virReportSystemError: passing virerrno instead of errno. Jincheng Miao (4): qemu: fix wrong errno report in qemuMonitorOpenUnix lxc: print ENOTSUP when usernamespace is not supported openvz: print EOVERFLOW when barrier:limit are too long util: print errno in virObjectLockableNew src/lxc/lxc_container.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/qemu/qemu_monitor.c | 7 +++++-- src/util/virobject.c | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) -- 1.8.3.1

3 12

[libvirt] [PATCH] spec: Update polkit dependencies for CVE-2013-4311
by Jiri Denemark 16 Jul '14

16 Jul '14

Use secured polkit on distros which provide it. However, RHEL-6 will still allow for older polkit-0.93 rather than forcing polkit-0.96-5 which is not available in all RHEL-6 releases. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- libvirt.spec.in | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 8d1acfa..f32ab00 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -535,7 +535,9 @@ BuildRequires: module-init-tools BuildRequires: cyrus-sasl-devel %endif %if %{with_polkit} - %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 + %if 0%{?fedora} >= 21 || 0%{?rhel} >= 7 +BuildRequires: polkit-devel >= 0.112 + %elif 0%{?fedora} >= 12 || 0%{?rhel} >= 6 BuildRequires: polkit-devel >= 0.93 %else BuildRequires: PolicyKit-devel >= 0.6 @@ -698,7 +700,9 @@ Requires: avahi-libs %endif %endif %if %{with_polkit} - %if 0%{?fedora} >= 12 || 0%{?rhel} >=6 + %if 0%{?fedora} >= 21 || 0%{?rhel} >= 7 +Requires: polkit >= 0.112 + %elif 0%{?fedora} >= 12 || 0%{?rhel} >=6 Requires: polkit >= 0.93 %else Requires: PolicyKit >= 0.6 -- 2.0.0

2 4

[libvirt] [PATCH] spec: fix invalid syntax
by Eric Blake 16 Jul '14

16 Jul '14

Commit 20e01504 broke 'make rpm': error: line 540: Unknown tag: %elif 020 >= 12 || 0 >= 6 Apparently, even though shell has elif so that you can do a chain of conditionals, the rpm spec file does not, and you have to nest things instead. * libvirt.spec.in: Convert %elif to proper nested %if. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule. libvirt.spec.in | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 47bfec5..9c7b241 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -537,10 +537,12 @@ BuildRequires: cyrus-sasl-devel %if %{with_polkit} %if 0%{?fedora} >= 20 || 0%{?rhel} >= 7 BuildRequires: polkit-devel >= 0.112 - %elif 0%{?fedora} >= 12 || 0%{?rhel} >= 6 -BuildRequires: polkit-devel >= 0.93 %else + %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 +BuildRequires: polkit-devel >= 0.93 + %else BuildRequires: PolicyKit-devel >= 0.6 + %endif %endif %endif %if %{with_storage_fs} @@ -702,10 +704,12 @@ Requires: avahi-libs %if %{with_polkit} %if 0%{?fedora} >= 20 || 0%{?rhel} >= 7 Requires: polkit >= 0.112 - %elif 0%{?fedora} >= 12 || 0%{?rhel} >=6 -Requires: polkit >= 0.93 %else + %if 0%{?fedora} >= 12 || 0%{?rhel} >=6 +Requires: polkit >= 0.93 + %else Requires: PolicyKit >= 0.6 + %endif %endif %endif %if %{with_cgconfig} -- 1.9.3

1 0

Re: [libvirt] Enable kvm on aarch64, Cleanup F-16/18 conditionals
by Eric Blake 16 Jul '14

16 Jul '14

On 07/15/2014 04:00 PM, Peter Robinson wrote: [reformatting to avoid top-posting] > On Tue, Jul 15, 2014 at 6:52 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote: >> Historically we've kept support for building libvirt against >> Fedora versions even if unsupported by Fedora itself, because >> we've found people often stick on old Fedora versions. At some >> point though it does get a bit insane. eg we don't really need >> Fedora 8 support at this point. If we are going todo a cleanup >> though, we should do it throughout the spec, not just in these >> few places > Sorry, I missed the Fedora 8 stuff. Would it even work on Fedora 8 > with all the various other changes? Not sure it's worth the effort for > the few people that want that release as a hypervisor, I can > understand for the supported elX releases but not sure the Fedora > releases that are systemd based give value to elX releases and are > generally relatively ancient history. Dan do you really have know > users of the latest release running it on F-8? > > In terms of pushing upstream Cole has said he's happy to push the > commits to ensure it fits your work flows. Doing an out-of-the-box build on RHEL 5 is the oldest configuration still actively (if marginally) supported, ideally for as long as RHEL 5 remains a live platform (several more years to go). We have build-bots that ensure that we can build on RHEL 5, although I'm not sure if those buildbots are exercising 'make rpm' to test the older parts of the spec file. Historically, RHEL 5.10 is based off of libvirt-0.8.2, and that was the release in use during Fedora 13. So it's _definitely_ worth culling any conditionals older than F13; but stuff between F13 and F18 might be shared with RHEL 5, and therefore more effort to cull the Fedora side while still leaving the RHEL side intact. I could go either way if we were to set some sort of policy (maybe: any fedora release that is unsupported for more than a year is no longer required to be supported in the spec file), and use that to justify cleanups of older parts of the spec file as long as it doesn't break RHEL 5. F16 is definitely more than a year out-of-date, F18 is a bit more borderline on whether we are ready to call it unsupported. Anyone else on the libvirt list have an opinion on how far back we can clean without annoying people that are slow on the upgrade to modern Fedora? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

1 0

[libvirt] [PATCH 0/4] Resolve const correctness isues
by Michal Privoznik 15 Jul '14

15 Jul '14

Okay, okay. The approach in 4/4 can be considered hackish, but hey - it works! Michal Privoznik (4): Fix const correctness viralloc: Honor const correctness in VIR_FREE VIR_FREE: Avoid doing side work in callees virFree: Check const correctness src/conf/network_conf.c | 12 ++++++++---- src/locking/lock_driver_lockd.c | 2 +- src/qemu/qemu_capabilities.c | 2 +- src/remote/remote_driver.c | 2 +- src/util/viralloc.c | 6 ++++-- src/util/viralloc.h | 33 ++++++++++++++++----------------- src/xenapi/xenapi_utils.c | 3 ++- tools/virsh-domain.c | 4 ++-- tools/wireshark/src/packet-libvirt.c | 6 +++--- tools/wireshark/src/packet-libvirt.h | 4 ++-- 10 files changed, 40 insertions(+), 34 deletions(-) -- 1.8.5.5

5 15

[libvirt] [PATCH 0/2] Implement interface stats for BSD
by Roman Bogorodskiy 15 Jul '14

15 Jul '14

This series implements support for querying network interface stats on (Free)BSD. It's more of an RFC, because I'm uncertain about few things: - It feels a little strange to have a source file that implements only a single function like this. I am wondering if it would be better to just move it to something like util/virnetdev.c? - FreeBSD stores interface data in the if_data struct and a number of outgoing packet drops is stored in a field 'ifi_oqdrops'. This field was added in -CURRENT and later merged back to 10-STABLE. In order not to break the ABI, it's available only if _IFI_OQDROPS is defined. I've added a configure.ac check which adds -D_IFI_OQDROPS before checking this field and resetting it back if it is not present. This way, this flag will present when the field is available even if the flag is not needed (e.g. on -CURRENT). Is there a better way of doing it? I was thinking about trying to check this field without the flag and if it fails check one more time with the flag, but it looks a little messy. - Did I get it right that the stats reported are from the guest POV, e.g. when downloading a large file from guest, it should look like: vnet0 rx_bytes 731603341 vnet0 rx_packets 518354 vnet0 rx_errs 0 vnet0 rx_drop 0 vnet0 tx_bytes 17577834 vnet0 tx_packets 264226 vnet0 tx_errs 0 vnet0 tx_drop 0 Roman Bogorodskiy (2): util: virstatslinux: make more generic Implement interface stats for BSD configure.ac | 13 ++++- po/POTFILES.in | 2 +- src/Makefile.am | 2 +- src/libvirt_linux.syms | 3 -- src/libvirt_private.syms | 2 + src/lxc/lxc_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/qemu/qemu_driver.c | 16 +----- src/uml/uml_driver.c | 2 +- src/util/{virstatslinux.c => virstats.c} | 93 +++++++++++++++++++++++++------- src/util/{virstatslinux.h => virstats.h} | 12 ++--- src/xen/xen_hypervisor.c | 2 +- tests/statstest.c | 2 +- 13 files changed, 102 insertions(+), 51 deletions(-) rename src/util/{virstatslinux.c => virstats.c} (61%) rename src/util/{virstatslinux.h => virstats.h} (77%) -- 1.9.0

2 4

Re: [libvirt] Enable kvm on aarch64, Cleanup F-16/18 conditionals
by Eric Blake 15 Jul '14

15 Jul '14

reposting to the upstream list - we'd like to keep the downstream .spec file in sync with upstream, rather than needlessly diverging. On 07/15/2014 10:52 AM, Peter Robinson wrote: > commit ae37ed3500672f12383825c84dd5ae940fb90ff8 > Author: Peter Robinson <pbrobinson(a)gmail.com> > Date: Tue Jul 15 17:52:18 2014 +0100 > > Enable kvm on aarch64, Cleanup F-16/18 conditionals > > libvirt.spec | 28 ++++++++-------------------- > 1 files changed, 8 insertions(+), 20 deletions(-) > --- > diff --git a/libvirt.spec b/libvirt.spec > index fad21d9..213760d 100644 > --- a/libvirt.spec > +++ b/libvirt.spec > @@ -55,14 +55,10 @@ > > %define with_qemu_tcg %{with_qemu} > # Change if we ever provide qemu-kvm binaries on non-x86 hosts > -%if 0%{?fedora} >= 18 > - %if 0%{?fedora} >= 20 > - %define qemu_kvm_arches %{ix86} x86_64 ppc64 s390x %{arm} > - %else > - %define qemu_kvm_arches %{ix86} x86_64 ppc64 s390x > - %endif > +%if 0%{?fedora} >= 20 > + %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64 > %else > - %define qemu_kvm_arches %{ix86} x86_64 > + %define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x > %endif > > %ifarch %{qemu_kvm_arches} > @@ -212,18 +208,6 @@ > %define with_xen 0 > %endif > > -# Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc > -%if 0%{?fedora} && 0%{?fedora} < 16 > - %ifarch ppc64 > - %define with_qemu 0 > - %endif > -%endif > - > -# Fedora doesn't have new enough Xen for libxl until F18 > -%if 0%{?fedora} && 0%{?fedora} < 18 > - %define with_libxl 0 > -%endif > - > # PolicyKit was introduced in Fedora 8 / RHEL-6 or newer > %if 0%{?fedora} >= 8 || 0%{?rhel} >= 6 > %define with_polkit 0%{!?_without_polkit:1} > @@ -385,7 +369,7 @@ > Summary: Library providing a simple virtualization API > Name: libvirt > Version: 1.2.6 > -Release: 1%{?dist}%{?extra_release} > +Release: 2%{?dist}%{?extra_release} > License: LGPLv2+ > Group: Development/Libraries > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root > @@ -2236,6 +2220,10 @@ exit 0 > %doc examples/systemtap > > %changelog > +* Tue Jul 15 2014 Peter Robinson <pbrobinson(a)fedoraproject.org> 1.2.6-2 > +- Enable kvm on aarch64 > +- Cleanup F-16/18 conditionals > + > * Wed Jul 2 2014 Daniel P. Berrange <berrange(a)redhat.com> - 1.2.6-1 > - Update to 1.2.6 release > > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

2 1

[libvirt] [PATCH 0/3] libxl: support hotplug of <interface> device
by Chunyan Liu 15 Jul '14

15 Jul '14

This patch series is to add support for attach/detaching an <interface> device. At the same time, add two fixes (1/3 and 3/3) Chunyan Liu (3): libxl: add HOSTDEV type in libxlDomainDetachDeviceConfig libxl: support hotplug of <interface> libxl: fix return value error Attach|DetachDeviceFlags .gnulib | 2 +- src/libxl/libxl_domain.c | 12 ++- src/libxl/libxl_driver.c | 193 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 180 insertions(+), 27 deletions(-) -- 1.8.4.5

2 4