August 2017 - Devel - libvirt List Archives

[libvirt] [PATCH] qemu: don't check whether offline migration is safe
by Pavel Hrdina 18 Aug '17

18 Aug '17

Offline migration transfers only the domain definition. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449715 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/qemu/qemu_migration.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 056c051b3e..ca1f67146b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1946,7 +1946,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, if (!qemuMigrationIsAllowed(driver, vm, true, flags)) goto cleanup; - if (!(flags & VIR_MIGRATE_UNSAFE) && + if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) && !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags)) goto cleanup; @@ -4809,7 +4809,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, if (!qemuMigrationIsAllowed(driver, vm, true, flags)) goto endjob; - if (!(flags & VIR_MIGRATE_UNSAFE) && + if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) && !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags)) goto endjob; -- 2.13.5

3 3

[libvirt] [PATCH v2 0/3] conf: Use the correct limit for the number of PHBs
by Andrea Bolognani 18 Aug '17

18 Aug '17

Andrea Bolognani (3): tests: Improve target index validation coverage conf: Move target index validation conf: Use the correct limit for the number of PHBs src/conf/domain_conf.c | 40 +++++++++++++--------- ...ml2argv-pseries-phb-invalid-target-index-1.xml} | 4 --- ...ml2argv-pseries-phb-invalid-target-index-2.xml} | 6 +--- ...ml2argv-pseries-phb-invalid-target-index-3.xml} | 11 +++--- tests/qemuxml2argvtest.c | 4 ++- 5 files changed, 32 insertions(+), 33 deletions(-) copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml => qemuxml2argv-pseries-phb-invalid-target-index-1.xml} (79%) copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml => qemuxml2argv-pseries-phb-invalid-target-index-2.xml} (68%) rename tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml => qemuxml2argv-pseries-phb-invalid-target-index-3.xml} (57%) -- 2.13.5

2 4

[libvirt] [PATCH] util: string: Introduce virStringHasChars
by Peter Krempa 18 Aug '17

18 Aug '17

The helper returns true if a string contains any of the given chars. virStringHasControlChars can be reimplemented using that helper. --- Pavel was loudly thinking about such function. It turns out that I have it ready on one of my branches. src/util/virstring.c | 23 +++++++++++++++++++---- src/util/virstring.h | 2 ++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index 58abf9dd6..0288d1e67 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1195,6 +1195,24 @@ virStringStripIPv6Brackets(char *str) } +/** + * virStringHasChars: + * @str: string to look for chars in + * @chars: chars to find in string @str + * + * Returns true if @str contains any of the chars in @chars. + */ +bool +virStringHasChars(const char *str, + const char *chars) +{ + if (!str) + return false; + + return str[strcspn(str, chars)] != '\0'; +} + + static const char control_chars[] = "\x01\x02\x03\x04\x05\x06\x07" "\x08" /* \t \n */ "\x0B\x0C" /* \r */ "\x0E\x0F" @@ -1204,10 +1222,7 @@ static const char control_chars[] = bool virStringHasControlChars(const char *str) { - if (!str) - return false; - - return str[strcspn(str, control_chars)] != '\0'; + return virStringHasChars(str, control_chars); } diff --git a/src/util/virstring.h b/src/util/virstring.h index ff5f0148d..1290fcce1 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -286,6 +286,8 @@ char *virStringReplace(const char *haystack, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); void virStringStripIPv6Brackets(char *str); +bool virStringHasChars(const char *str, + const char *chars); bool virStringHasControlChars(const char *str); void virStringStripControlChars(char *str); -- 2.14.0

2 1

[libvirt] [PATCH 00/12] qemu: don't lose VMs if emulator is not installed
by Peter Krempa 18 Aug '17

18 Aug '17

The post-parse callback grew into an abomination which requires qemuCaps to succeed. That won't work out well if for some reasons qemu is uninstalled. Restarting of libvirtd would result in all VMs being lost untill qemu is reinstalled. Fix this by allowing qemuCaps to be missing and re-running the postparse callbacks when attempting a VM start. Peter Krempa (12): conf: domainlist: Explicitly report failure to load domain config conf: Add 'basic' post parse callback qemu: Move assignment of default emulator to the basic post parse callback conf: Add callbacks that allocate per-def private data qemu: domain: Don't re-allocate qemuCaps in post parse callbacks conf: Return any non-zero value from virDomainDeviceInfoIterateInternal callback conf: add infrastructure for tolerating certain post parse callback failures qemu: capabilities: Tolerate missing @qemuCaps in virQEMUCapsGetCanonicalMachine qemu: capabilities: Tolerate missing @qemuCaps in virQEMUCapsSupportsGICVersion qemu: domain: Don't return default NIC model if @qemuCaps are missing qemu: domain: Don't set default USB model if qemuCaps is missing qemu: Implement postParse callback skipping on config reload src/conf/domain_conf.c | 202 +++++++++++++++++++++++++++++-------------- src/conf/domain_conf.h | 35 +++++++- src/conf/virdomainobjlist.c | 8 +- src/qemu/qemu_capabilities.c | 20 +++-- src/qemu/qemu_domain.c | 111 ++++++++++++++++-------- src/qemu/qemu_process.c | 9 ++ 6 files changed, 277 insertions(+), 108 deletions(-) -- 2.14.0

2 16

Re: [libvirt] [PATCH v2] Add support for virtio-net.tx_queue_size
by Jan Scheurich 18 Aug '17

18 Aug '17

On Thu, Jul 27 at 14:05:09 +0200 Peter Krempa wrote: > On Thu, Jul 20, 2017 at 21:36:28 -0400, Laine Stump wrote: > > On 07/18/2017 07:12 AM, Michal Privoznik wrote: [...] > > >>> So only users that know how virtio works under the hood are > > >>> expected to also know what rx/tx queue size is and how to set it. > > >>> But frankly, I > > >> This statement is ridiculous by itself. > > > Why? There are more experienced users (for whom libvirt's just a > > > stable > > > API) and less experienced ones (for whom libvirt's and tools on the > > > top of it are great for their automatic chose of parameters, e.g. gnome > boxes). > > > > Beyond that, that same statement (or something nearly identical) is > > repeated in multiple places throughout the XML documentation. There > > are at least two classes of these attributes that I can think of: > > > > 1) attributes that are automatically set to a sane value by libvirt > > when not specified (and they usually *aren't* specified), and saved in > > the config XML in order to assure they are set the same every time the > > domain is started (to preserve guest ABI). These are intended to > > record whatever was the default setting for the attribute at the time > > the domain was created. For example, a pcie-root-port controller might > > have <model name='ioh3420'/> set, if that was the only type of > > pcie-root-port available at the time a domain was created; this comes > > in handy now that there is a generic pcie-root-port (which also has > > <model > > name='pcie-root-port'/>) - existing domains don't get their ABI > > screwed up when they're migrated to a newer host. > > > > 2) knobs that have been added in over the years at the request/demand > > from below (qemu) and above (ovirt / openstack), many of them obscure, > > difficult to explain with any amount of useful detail, and almost > > never worthy of being manually set (and if you "don't know what you're > > doing", you're just as likely to make things worse as to make them > better). > > > > tx_queue_size is one of the latter. > > > > For either of these types of attributes, they need to be documented so > > that someone encountering them (or actively searching them out) will > > at least have a basic idea what the attribute is named / used for, but > > we also need to warn the casual user to not mess with them. I don't > > see anything ridiculous about that. > We, Ericsson, have been driving the possibility to increase the Rx and Tx queue sizes for virtio interfaces in Qemu because our throughput-intensive and packet-drop sensitive virtual network functions (VNFs) were suffering frequent early packet drops. In the age of DPDK and with packet rates in the Mpps range the hard-coded default queue size of 256 in Qemu was no longer adequate to compensate short interrupts on the receiving side or microbursts on the sending size. Many measurements across the industry had proven that increasing the queue size to 1024 very effectively reduced the packet drop probability and consequently improved throughput significantly. The Qemu community could not change the virtio-net default queue size to 1024 to avoid compatibility issues and limitations in legacy virtio drivers and virtio backends. But many VNFs and the DPDK vhostuser backend in vSwitches like OVS and fd.io/VPP can handle 1K queue size and benefit from it. So they made the queue sizes configurable in Qemu to give NFVI operators the chance to tune the performance of their infrastructure. This change in Libvirt is about making this tuning option available for Qemu users like OpenStack. There is no other way than Libvirt to make this accessible as Qemu does not have the facility of global default parameters. > > >>> think users setting this are always gonna go with the highest > > >>> value avaliable (1024). Such detailed description is a copy of > > >>> rx_virtio_queue size description which is result of review. Reasons not to set to 1024 could include compatibility constraints (see above) or a trade-off between packet drop probability and maximum latency/jitter. > > >>> > > >>>> Is it really needed? How should it be configured? Can't we or > > >>>> qemu pick a sane value? > > >>>> > > >>> No. Some users need bigger virtio rings otherwise they see a > > >>> packet drop. So this is a fine tuning that heavily depends on the use > case. > > >>> Thus libvirt should not try to come up with some value. > > >> That's why I think it's wrong. What's the drawback of setting it > > >> to maximum? Which workloads will hit it? Why is the default not > sufficient? > > >> > > >> And most notably, how do the users figure out that they need it? In our case the "user" is the NFVI deployment tool chain with tuning input from NFV solution architects, guided by Installation/Dimensioning guides. > > > > > > I think it's a bit too much burden on libvirt to expect that much > > detail to be included in formatdomain.html. Heck, I don't know if > > anyone even > > *knows* that much detail right now. > > That proves my point kind of. If there isn't knowledge how to set up this, > then why even add the option. Is there really a point? I think we could improve the documentation a bit with reference to vhostuser interfaces as main target interface type and mentioning potential compatibility constraints for applications. We would actually like to see an additional global config knob in Libvirt to (optionally) set default Rx and Tx queue sizes for vhostuser interfaces, so that Libvirt consumers like OpenStack would not have to be modified to use 1K queues. BR, Jan

1 0

[libvirt] [PATCH 0/5] A few fixes and improvements to the nodedev driver
by Erik Skultety 17 Aug '17

17 Aug '17

These are just the few I came across when working on another nodedev-related series. Erik Skultety (5): nodedev: mdev: Report an error when virFileResolveLink fails nodedev: udev: Remove the udevEventHandleCallback on fatal error nodedev: Clear the udev_monitor reference once unref'd nodedev: Introduce udevHandleOneDevice nodedev: Protect every access to udev_monitor by locking the driver src/node_device/node_device_udev.c | 59 +++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 17 deletions(-) -- 2.13.3

2 14

[libvirt] [PATCH] virt-aa-helper: locking disk files for qemu 2.10
by Christian Ehrhardt 17 Aug '17

17 Aug '17

Testing qemu-2.10-rc2 shows issues like: qemu-system-x86_64: -drive file=/var/lib/uvtool/libvirt/images/kvmguest- \ artful-normal.qcow,format=qcow2,if=none,id=drive-virtio-disk0: Failed to lock byte 100 It seems the following qemu commit changed the needs for the backing image rules: (qemu) commit 244a5668106297378391b768e7288eb157616f64 Author: Fam Zheng <famz(a)redhat.com> file-posix: Add image locking to perm operations The block appears as: apparmor="DENIED" operation="file_lock" [...] name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow" [...] comm="qemu-system-x86" requested_mask="k" denied_mask="k" With that qemu change in place the rules generated for the image and backing files need the allowance to also lock (k) the files. Disks are added via add_file_path and with this fix rules now get that permission, but no other rules are changed, example: - "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rw, + "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rwk Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> --- src/security/virt-aa-helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35dcb35..ab82f12 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -892,11 +892,11 @@ add_file_path(virDomainDiskDefPtr disk, if (depth == 0) { if (disk->src->readonly) - ret = vah_add_file(buf, path, "r"); + ret = vah_add_file(buf, path, "rk"); else - ret = vah_add_file(buf, path, "rw"); + ret = vah_add_file(buf, path, "rwk"); } else { - ret = vah_add_file(buf, path, "r"); + ret = vah_add_file(buf, path, "rk"); } if (ret != 0) -- 2.7.4

3 4

[libvirt] [PATCH] virt-aa-helper: locking loader/nvram for qemu 2.10
by Christian Ehrhardt 17 Aug '17

17 Aug '17

Testing qemu-2.10-rc3 shows issues like: qemu-system-aarch64: -drive file=/home/ubuntu/vm-start-stop/vms/ 7936-0_CODE.fd,if=pflash,format=raw,unit=1: Failed to unlock byte 100 There is an apparmor deny due to qemu now locking those files: apparmor="DENIED" operation="file_lock" [...] name="/home/ubuntu/vm-start-stop/vms/7936-0_CODE.fd" name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow" [...] comm="qemu-system-aarch64" requested_mask="k" denied_mask="k" The profile needs to allow locking for loader and nvram files via the locking (k) rule. Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> --- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index ab82f12..2308323 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1029,11 +1029,11 @@ get_files(vahControl * ctl) goto cleanup; if (ctl->def->os.loader && ctl->def->os.loader->path) - if (vah_add_file(&buf, ctl->def->os.loader->path, "r") != 0) + if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0) goto cleanup; if (ctl->def->os.loader && ctl->def->os.loader->nvram) - if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rw") != 0) + if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0) goto cleanup; for (i = 0; i < ctl->def->ngraphics; i++) { -- 2.7.4

3 4

[libvirt] [PATCH] network: Use self inflating bitmap for class IDs
by Michal Privoznik 17 Aug '17

17 Aug '17

Back in the day when I was implementing QoS for networks there were no self inflating virBitmaps. Only the static ones. Therefore, I had to allocate the whole 8KB of memory in order to keep track of used/unused class IDs. This is rather wasteful because nobody is ever gonna use that much classes (kernel overhead would drastically lower the bandwidth). Anyway, now that we have self inflating bitmaps we can start small and allocate more if there's need for it. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/virnetworkobj.c | 18 ++++++++++-------- src/network/bridge_driver.c | 5 +++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 918ef44ea..455b604bb 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -34,8 +34,10 @@ VIR_LOG_INIT("conf.virnetworkobj"); -/* currently, /sbin/tc implementation allows up to 16 bits for minor class size */ -#define CLASS_ID_BITMAP_SIZE (1<<16) +/* Currently, /sbin/tc implementation allows up to 16 bits for + * minor class size. But the initial bitmap doesn't have to be + * that big. */ +#define INIT_CLASS_ID_BITMAP_SIZE (1<<4) struct _virNetworkObj { virObjectLockable parent; @@ -100,13 +102,14 @@ virNetworkObjNew(void) if (!(obj = virObjectLockableNew(virNetworkObjClass))) return NULL; - if (!(obj->classIdMap = virBitmapNew(CLASS_ID_BITMAP_SIZE))) + if (!(obj->classIdMap = virBitmapNew(INIT_CLASS_ID_BITMAP_SIZE))) goto error; /* The first three class IDs are already taken */ - ignore_value(virBitmapSetBit(obj->classIdMap, 0)); - ignore_value(virBitmapSetBit(obj->classIdMap, 1)); - ignore_value(virBitmapSetBit(obj->classIdMap, 2)); + if (virBitmapSetBitExpand(obj->classIdMap, 0) < 0 || + virBitmapSetBitExpand(obj->classIdMap, 1) < 0 || + virBitmapSetBitExpand(obj->classIdMap, 2) < 0) + goto error; virObjectLock(obj); @@ -909,8 +912,7 @@ virNetworkLoadState(virNetworkObjListPtr nets, ctxt->node = node; if ((classIdStr = virXPathString("string(./class_id[1]/@bitmap)", ctxt))) { - if (virBitmapParse(classIdStr, &classIdMap, - CLASS_ID_BITMAP_SIZE) < 0) { + if (!(classIdMap = virBitmapParseUnlimited(classIdStr))) { VIR_FREE(classIdStr); goto error; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 74ce92e43..e8d093a31 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5395,9 +5395,10 @@ networkNextClassID(virNetworkObjPtr obj) ssize_t ret = 0; virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj); - ret = virBitmapNextClearBit(classIdMap, -1); + if ((ret = virBitmapNextClearBit(classIdMap, -1)) < 0) + ret = virBitmapSize(classIdMap); - if (ret < 0 || virBitmapSetBit(classIdMap, ret) < 0) + if (virBitmapSetBitExpand(classIdMap, ret) < 0) return -1; return ret; -- 2.13.0

2 1

[libvirt] [PATCH] virt-host-validate: Fix warning for IOMMU detection on PPC
by Nitesh Konkar 17 Aug '17

17 Aug '17

Fix the warning generated on PPC by virt-host-validate for IOMMU Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com> --- tools/virt-host-validate-common.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 6faed04..e0ca1dd 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -35,6 +35,7 @@ #include "virfile.h" #include "virt-host-validate-common.h" #include "virstring.h" +#include "virarch.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -443,7 +444,7 @@ int virHostValidateIOMMU(const char *hvname, struct stat sb; const char *bootarg = NULL; bool isAMD = false, isIntel = false; - + virArch hostarch; flags = virHostValidateGetCPUFlags(); if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX)) @@ -454,6 +455,7 @@ int virHostValidateIOMMU(const char *hvname, virBitmapFree(flags); virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support")); + hostarch = virArchFromHost(); if (isIntel) { if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) { @@ -477,7 +479,7 @@ int virHostValidateIOMMU(const char *hvname, "hardware platform"); return -1; } - } else { + } else if (!ARCH_IS_PPC64(hostarch)) { virHostMsgFail(level, "Unknown if this platform has IOMMU support"); return -1; @@ -491,6 +493,9 @@ int virHostValidateIOMMU(const char *hvname, if (!S_ISDIR(sb.st_mode)) return 0; + if (S_ISDIR(sb.st_mode) && ARCH_IS_PPC64(hostarch)) + virHostMsgPass(); + virHostMsgCheck(hvname, "%s", _("if IOMMU is enabled by kernel")); if (sb.st_nlink <= 2) { virHostMsgFail(level, -- 2.7.4

2 1