- Devel - libvirt List Archives

[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

[libvirt] [PATCH 00/10] another set of parsing improvements
by Pavel Hrdina 17 Aug '17

17 Aug '17

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

2 20

[libvirt] [PATCH] network: Use @nnames instead of @got
by John Ferlan 17 Aug '17

17 Aug '17

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

2 1

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

17 Aug '17

I mistakenly thought pSeries guests supported 32 PHBs, but it turns out they only support 31. Validate the target index accordingly. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1479647 Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/qemu/qemu_domain_address.c | 4 ++-- .../qemuxml2argv-pseries-phb-too-many.xml | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 281dc68f0..8257f04ab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9326,10 +9326,10 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; } if (def->opts.pciopts.targetIndex < 0 || - def->opts.pciopts.targetIndex > 31) { + def->opts.pciopts.targetIndex > 30) { virReportError(VIR_ERR_XML_ERROR, _("PCI controller target index '%s' out of " - "range - must be 0-31"), + "range - must be 0-30"), targetIndex); goto error; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 8066ed123..04438fae9 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2157,9 +2157,9 @@ qemuDomainAddressFindNewTargetIndex(virDomainDefPtr def) int targetIndex; int ret = -1; - /* Try all indexes between 1 and 31 - QEMU only supports 32 + /* Try all indexes between 1 and 30 - QEMU only supports 31 * PHBs, and 0 is reserved for the default, implicit one */ - for (targetIndex = 1; targetIndex <= 31; targetIndex++) { + for (targetIndex = 1; targetIndex <= 30; targetIndex++) { bool found = false; size_t i; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml new file mode 100644 index 000000000..3d99da499 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> +  + <controller type='pci' model='pci-root'> + <target index='31'/> + </controller> + <controller type='usb' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 77cfe8d28..47ca90f04 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1761,6 +1761,7 @@ mymain(void) DO_TEST("pseries-phb-simple", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); + DO_TEST_PARSE_ERROR("pseries-phb-too-many", NONE); DO_TEST("pseries-phb-default-missing", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); -- 2.13.5

2 1

[libvirt] [PATCH] Don't autogenerate seclabels of type 'none'
by Jim Fehlig 17 Aug '17

17 Aug '17

When security drivers are active and domain def contains no <seclabel> elements, there is no need to autogenerate seclabels when starting the domain, e.g. <seclabel type='none' model='apparmor'/> In fact, autogenerating the label can result in needless save/restore and migration failures when the security driver is not active on the restore/migration target. The virSecurityManagerGenLabel function in src/security_manager.c even has logic to skip autogenerated labels, but the logic is a bit flawed. Autogeneration should be skipped when the domain has not seclabels, i.e. vm->nseclabels == 0. Resolves: https://bugzilla.opensuse.org/show_bug.cgi?id=1051017 Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/security/security_manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 013bbc37e..441c4d1fd 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -670,7 +670,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Unconfined guests are not allowed on this host")); goto cleanup; - } else if (vm->nseclabels && generated) { + } else if (vm->nseclabels == 0 && generated) { VIR_DEBUG("Skipping auto generated seclabel of type none"); virSecurityLabelDefFree(seclabel); seclabel = NULL; -- 2.13.1

2 2

[libvirt] [PATCH v2 00/20] Make the virNetworkObjPtr private
by John Ferlan 16 Aug '17

16 Aug '17

v1: https://www.redhat.com/archives/libvir-list/2017-May/msg00701.html (but reviewed much more recently) NOTE from v1: Patches 1-3 already pushed Former patch 4: * Patch 1 (NEW) - splits out the formatting change in bridge_driver.h * Patch 2 - Remainder of the change for consistent naming NB: Without the split, there was a R-B, but I didn't push so that the split could be "seen". Former patch 5: * Patch 3 (NEW) - from review create virMacMapFileName in src/util/virmacmap.c * Patch 4 - Remainder of the previous change w/ adjusted name of course * Patch 5 (NEW) - from review, alter virNetworkObjUnrefMacMap Former patch 6: * Patch 6 - No change Former patch 7: * Patch 7 (NEW) - Split out the @class_id to @classIdMap change * Patch 8 - Remainder of previous change Former patch 8: * Patch 9 - No change Former patch 9: * Patch 10 - Make suggested naming adjustments Add/use virNetworkObjSetDef API Former patch 10: * Patch 11 - Move code back to driver, just have accessors for @autostart Former patch 11: * Patch 12 - No change Former patch 12: * Patch 13 - Use virNetworkObjIsPersistent in networkSetAutostart Former patch 13: * Patch 14 - No change Former patch 14: * Patch 15 - Just have the virNetworkObjNew lock the object now and make use of that with using virNetworkObjEndAPI in networkxml2conftest NB: Since we'll have a refcnt=1 and lock=true after New the EndAPI is proper * Patch 16 (NEW) - Just move the virObjectRef - makes it clearer why it's being ref'd Former patch 15: * Patch 17 (NEW) - Split out the rename of @nnames to @maxnames and explain the reason better * Patch 18 (NEW) - Split out the rename of @filter to @aclnames and explain the reason better * Patch 19 - The remainder of the former patch Former patch 16: * Patch 20 - No change (other than merge conflict resolution) John Ferlan (20): network: Perform some formatting cleanup in bridge_driver.h network: Use consistent naming in bridge_driver for virNetwork objects network: Move and rename networkMacMgrFileName network: Move macmap mgmt from bridge_driver to virnetworkobj network: Unconditionally initialize macmap when stopping virtual network network: Add virNetworkObj Get/Set API's for @dnsmasqPid and @radvdPid network: Alter virNetworkObj @class_id to be @classIdMap network: Introduce virNetworkObjGetClassIdMap network: Add virNetworkObj Get/Set API's for @floor_sum network: Add virNetworkObj Get/Set API's for @def and @newDef network: Introduce virNetworkObj{Get|Set}Autostart network: Introduce virNetworkObj{Is|Set}Active network: Introduce virNetworkObjIsPersistent network: Consistent use of @obj for virnetworkobj network: Have virNetworkObjNew lock the returned object network: Move virObjectRef during AssignDef processing network: Use @maxnames instead of @nnames network: Rename @filter to @aclfilter network: Modify naming for virNetworkObjList* fetching APIs network: Privatize virNetworkObj src/conf/virnetworkobj.c | 614 +++++++++++++++------- src/conf/virnetworkobj.h | 105 ++-- src/libvirt_private.syms | 21 + src/network/bridge_driver.c | 1218 ++++++++++++++++++++++--------------------- src/network/bridge_driver.h | 50 +- src/test/test_driver.c | 83 +-- src/util/virmacmap.c | 12 + src/util/virmacmap.h | 4 + tests/networkxml2conftest.c | 11 +- 9 files changed, 1231 insertions(+), 887 deletions(-) -- 2.9.4

3 38

[libvirt] [PATCH 0/4] Alter @aclfilter to just @filter
by John Ferlan 16 Aug '17

16 Aug '17

>From recent review comments in the network series, Michal and Pavel have both stated the preference to use @filter, so to be consistent, rename variables that were changed in prior series. John Ferlan (4): nodedev: Alter @aclfilter to just @filter nwfilter: Alter @aclfilter to just @filter secret: Alter @aclfilter to just @filter storage: Alter @aclfilter to just @filter src/conf/virnodedeviceobj.c | 28 ++++++++++++++-------------- src/conf/virnodedeviceobj.h | 6 +++--- src/conf/virnwfilterobj.c | 18 +++++++++--------- src/conf/virnwfilterobj.h | 6 +++--- src/conf/virsecretobj.c | 24 ++++++++++++------------ src/conf/virsecretobj.h | 6 +++--- src/conf/virstorageobj.c | 20 ++++++++++---------- src/conf/virstorageobj.h | 10 +++++----- 8 files changed, 59 insertions(+), 59 deletions(-) -- 2.9.4

2 5

[libvirt] question about locking in qemuDomainObjBeginJobInternal()
by Chris Friesen 16 Aug '17

16 Aug '17

Hi, I'm hitting a scenario (on libvirt 1.2.12, so yeah it's a bit old) where I'm attempting to create two domains at the same time, and they both end up erroring out with "cannot acquire state change lock": 2017-08-14T12:57:00.000 79674: warning : qemuDomainObjBeginJobInternal:1380 : Cannot start job (modify, none) for domain instance-00000001; current job is (modify, none) owned by (79673, 0) 2017-08-14T12:57:00.000 79674: error : qemuDomainObjBeginJobInternal:1385 : Timed out during operation: cannot acquire state change lock 2017-08-14T12:57:01.000 79675: warning : qemuDomainObjBeginJobInternal:1380 : Cannot start job (modify, none) for domain instance-00000002; current job is (modify, none) owned by (79677, 0) 2017-08-14T12:57:01.000 79675: error : qemuDomainObjBeginJobInternal:1385 : Timed out during operation: cannot acquire state change lock Given that the lock appears to be per-domain, I assume this means that something is trying to issue multiple operations in parallel to each domain? Chris

2 1

[libvirt] IRC support thanks
by Rob Whyte 16 Aug '17

16 Aug '17

Hi from Tasmania Australia, I am writing to express gratitude twoard those on the IRC channel for soem hand-holding support iwth FVirt. In turn I would like to commit some patches to tidy up some docus. Until then. cheers Rob

2 1