[libvirt] [PATCH] util: string: Introduce virStringHasChars
by Peter Krempa
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
7 years, 4 months
[libvirt] [PATCH 00/12] qemu: don't lose VMs if emulator is not installed
by Peter Krempa
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
7 years, 4 months
Re: [libvirt] [PATCH v2] Add support for virtio-net.tx_queue_size
by Jan Scheurich
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
7 years, 4 months
[libvirt] [PATCH 0/5] A few fixes and improvements to the nodedev driver
by Erik Skultety
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
7 years, 4 months
[libvirt] [PATCH] virt-aa-helper: locking disk files for qemu 2.10
by Christian Ehrhardt
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
7 years, 4 months
[libvirt] [PATCH] virt-aa-helper: locking loader/nvram for qemu 2.10
by Christian Ehrhardt
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
7 years, 4 months
[libvirt] [PATCH] network: Use self inflating bitmap for class IDs
by Michal Privoznik
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
7 years, 4 months
[libvirt] [PATCH] virt-host-validate: Fix warning for IOMMU detection on PPC
by Nitesh Konkar
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
7 years, 4 months
[libvirt] [PATCH 00/10] another set of parsing improvements
by Pavel Hrdina
Pavel Hrdina (10):
util: introduce virXMLPropStringLimit
util: introduce virXMLNodeContentString
conf: use virXMLPropString for virDomainVirtioOptionsParseXML
conf: use virXMLPropString for IOMMU def parsing
conf: use virXMLPropString for network parsing
conf: use virXMLPropString for boot parsing
conf: use virXMLPropString for actual network parsing
conf: use virXMLPropStringLimit where it makes sense
conf: use virXMLNodeContentString for boot options parsing
conf: use virXMLPropString and virXMLNodeContentString for vcpu
parsing
src/conf/domain_conf.c | 601 +++++++++++++++++++++++++----------------------
src/libvirt_private.syms | 2 +
src/util/virxml.c | 68 +++++-
src/util/virxml.h | 4 +
4 files changed, 381 insertions(+), 294 deletions(-)
--
2.13.5
7 years, 4 months
[libvirt] [PATCH] network: Use @nnames instead of @got
by John Ferlan
To make it clearer, let's use @nnames instead of @got for counting the
names in the @names array. Keeps things consistent and clear.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
NB: Be sure you're tree is up to date before attempting to apply ;-)
src/conf/virnetworkobj.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 918ef44..b16844d 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1444,9 +1444,9 @@ struct virNetworkObjListGetHelperData {
virConnectPtr conn;
virNetworkObjListFilter filter;
char **names;
+ int nnames;
int maxnames;
bool active;
- int got;
bool error;
};
@@ -1462,7 +1462,7 @@ virNetworkObjListGetHelper(void *payload,
return 0;
if (data->maxnames >= 0 &&
- data->got == data->maxnames)
+ data->nnames == data->maxnames)
return 0;
virObjectLock(obj);
@@ -1474,11 +1474,11 @@ virNetworkObjListGetHelper(void *payload,
if ((data->active && virNetworkObjIsActive(obj)) ||
(!data->active && !virNetworkObjIsActive(obj))) {
if (data->names &&
- VIR_STRDUP(data->names[data->got], obj->def->name) < 0) {
+ VIR_STRDUP(data->names[data->nnames], obj->def->name) < 0) {
data->error = true;
goto cleanup;
}
- data->got++;
+ data->nnames++;
}
cleanup:
@@ -1498,8 +1498,8 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
int ret = -1;
struct virNetworkObjListGetHelperData data = {
- .conn = conn, .filter = filter, .names = names,
- .maxnames = maxnames, .active = active, .got = 0, .error = false};
+ .conn = conn, .filter = filter, .names = names, .nnames = 0,
+ .maxnames = maxnames, .active = active, .error = false};
virObjectLock(nets);
virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
@@ -1508,11 +1508,11 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
if (data.error)
goto cleanup;
- ret = data.got;
+ ret = data.nnames;
cleanup:
if (ret < 0) {
- while (data.got)
- VIR_FREE(data.names[--data.got]);
+ while (data.nnames)
+ VIR_FREE(data.names[--data.nnames]);
}
return ret;
}
@@ -1525,14 +1525,14 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
virConnectPtr conn)
{
struct virNetworkObjListGetHelperData data = {
- .conn = conn, .filter = filter, .names = NULL,
- .maxnames = -1, .active = active, .got = 0, .error = false};
+ .conn = conn, .filter = filter, .names = NULL, .nnames = 0,
+ .maxnames = -1, .active = active, .error = false};
virObjectLock(nets);
virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
virObjectUnlock(nets);
- return data.got;
+ return data.nnames;
}
--
2.9.4
7 years, 4 months
