- Devel - libvirt List Archives

[libvirt] [PREPOST 01/17] src/xenxs:Introduce function
by David Kiarie 16 Jul '14

16 Jul '14

From: Kiarie Kahurani <davidkiarie4(a)gmail.com> Introduce function xenParseXMGeneral(virConfPtr conf, ...); This function parses the xm general options such as uuid and name signed-off-by: David Kiarie <davidkiarie4(a)gmail.com> --- src/xenxs/xen_xm.c | 65 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 25a042d..1953a85 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char *uuid) * Turn a config record into a lump of XML describing the * domain, suitable for later feeding for virDomainCreateXML */ -virDomainDefPtr -xenParseXM(virConfPtr conf, int xendConfigVersion, - virCapsPtr caps) +static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def, + virCapsPtr caps) { + + const char *defaultMachine; const char *str; int hvm = 0; - int val; - virConfValuePtr list; - virDomainDefPtr def = NULL; - virDomainDiskDefPtr disk = NULL; - virDomainNetDefPtr net = NULL; - virDomainGraphicsDefPtr graphics = NULL; - virDomainHostdevDefPtr hostdev = NULL; - size_t i; - const char *defaultMachine; - int vmlocaltime = 0; - unsigned long count; - char *script = NULL; - char *listenAddr = NULL; - - if (VIR_ALLOC(def) < 0) - return NULL; - - def->virtType = VIR_DOMAIN_VIRT_XEN; - def->id = -1; if (xenXMConfigCopyString(conf, "name", &def->name) < 0) - goto cleanup; + return -1; if (xenXMConfigGetUUID(conf, "uuid", def->uuid) < 0) - goto cleanup; - + return -1; if ((xenXMConfigGetString(conf, "builder", &str, "linux") == 0) && STREQ(str, "hvm")) hvm = 1; if (VIR_STRDUP(def->os.type, hvm ? "hvm" : "xen") < 0) - goto cleanup; + return -1; def->os.arch = virCapabilitiesDefaultGuestArch(caps, @@ -300,7 +281,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, virReportError(VIR_ERR_INTERNAL_ERROR, _("no supported architecture for os type '%s'"), def->os.type); - goto cleanup; + return -1; } defaultMachine = virCapabilitiesDefaultGuestMachine(caps, @@ -309,9 +290,37 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, virDomainVirtTypeToString(def->virtType)); if (defaultMachine != NULL) { if (VIR_STRDUP(def->os.machine, defaultMachine) < 0) - goto cleanup; + return -1; } + return 0; +} +virDomainDefPtr +xenParseXM(virConfPtr conf, int xendConfigVersion, + virCapsPtr caps) +{ + const char *str; + int hvm = 0; + int val; + virConfValuePtr list; + virDomainDefPtr def = NULL; + virDomainDiskDefPtr disk = NULL; + virDomainNetDefPtr net = NULL; + virDomainGraphicsDefPtr graphics = NULL; + virDomainHostdevDefPtr hostdev = NULL; + size_t i; + int vmlocaltime = 0; + unsigned long count; + char *script = NULL; + char *listenAddr = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + def->virtType = VIR_DOMAIN_VIRT_XEN; + def->id = -1; + if (xenParseXMGeneral(conf, def, caps) < 0) + goto cleanup; + hvm = (STREQ(def->os.type, "hvm")); if (hvm) { const char *boot; if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) < 0) -- 1.8.4.5

4 42

[libvirt] [PATCH] blockjob: wait for pivot to complete
by Eric Blake 16 Jul '14

16 Jul '14

https://bugzilla.redhat.com/show_bug.cgi?id=1119173 documents that commit eaba79d was flawed in the implementation of the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag when it comes to completing a blockcopy. Basically, the qemu pivot action is async (the QMP command returns immediately, but the user must wait for the BLOCK_JOB_COMPLETE event to know that all I/O related to the job has finally been flushed), but the libvirt command was documented as synchronous by default. As active block commit will also be using this code, it is worth fixing now. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Don't skip wait loop after pivot. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Shown here with extra context, to hopefully aid the review. src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..300e49e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14973,45 +14973,45 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, if (!device) goto endjob; disk = vm->def->disks[idx]; if (mode == BLOCK_JOB_PULL && disk->mirror) { virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, _("disk '%s' already in active block job"), disk->dst); goto endjob; } if (mode == BLOCK_JOB_ABORT && (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) && !(async && disk->mirror)) { virReportError(VIR_ERR_OPERATION_INVALID, _("pivot of disk '%s' requires an active copy job"), disk->dst); goto endjob; } if (disk->mirror && mode == BLOCK_JOB_ABORT && (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) { ret = qemuDomainBlockPivot(conn, driver, vm, device, disk); - goto endjob; + goto waitjob; } if (base && (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 || !(baseSource = virStorageFileChainLookup(disk->src, disk->src, base, baseIndex, NULL)))) goto endjob; if (baseSource) { if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0) goto endjob; if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this QEMU binary doesn't support relative " "block pull/rebase")); goto endjob; } if (virStorageFileGetRelativeBackingPath(disk->src->backingStore, baseSource, @@ -15030,44 +15030,45 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, bandwidth, info, mode, async); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; /* Snoop block copy operations, so future cancel operations can * avoid checking if pivot is safe. */ if (mode == BLOCK_JOB_INFO && ret == 1 && disk->mirror && info->cur == info->end && info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) disk->mirroring = true; /* A successful block job cancelation stops any mirroring. */ if (mode == BLOCK_JOB_ABORT && disk->mirror) { /* XXX We should also revoke security labels and disk lease on * the mirror, and audit that fact, before dropping things. */ virStorageSourceFree(disk->mirror); disk->mirror = NULL; disk->mirroring = false; } + waitjob: /* With synchronous block cancel, we must synthesize an event, and * we silently ignore the ABORT_ASYNC flag. With asynchronous * block cancel, the event will come from qemu, but without the * ABORT_ASYNC flag, we must block to guarantee synchronous * operation. We do the waiting while still holding the VM job, * to prevent newly scheduled block jobs from confusing us. */ if (mode == BLOCK_JOB_ABORT) { if (!async) { /* Older qemu that lacked async reporting also lacked * active commit, so we can hardcode the event to pull. * We have to generate two variants of the event. */ int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; int status = VIR_DOMAIN_BLOCK_JOB_CANCELED; event = virDomainEventBlockJobNewFromObj(vm, disk->src->path, type, status); event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, status); } else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) { while (1) { /* Poll every 50ms */ static struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; -- 1.9.3

2 2

[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