Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
On Sun, 5 May 2019 21:49:04 -0400 Yan Zhao wrote: > version attribute is used to check two mdev devices' compatibility. > > The key point of this version attribute is that it's rw. > User space has no need to understand internal of device version and no > need to compare versions by itself. > Compared to reading version strings from both two mdev devices being > checked, user space only reads from one mdev device's version attribute. > After getting its version string, user space writes this string into the > other mdev device's version attribute. Vendor driver of mdev device > whose version attribute being written will check device compatibility of > the two mdev devices for user space and return success for compatibility > or errno for incompatibility. > So two readings of version attributes + checking in user space are now > changed to one reading + one writing of version attributes + checking in > vendor driver. > Format and length of version strings are now private to vendor driver > who can define them freely. > > __ user space > /\ \ > / \write > / read \ > __/__ ___\|/___ > | version | | version |-->check compatibility > --- --- > mdev device A mdev device B > > This version attribute is optional. If a mdev device does not provide > with a version attribute, this mdev device is incompatible to all other > mdev devices. > > Live migration is able to take advantage of this version attribute. > Before user space actually starts live migration, it can first check > whether two mdev devices are compatible. > > v2: > 1. added detailed intent and usage > 2. made definition of version string completely private to vendor driver >(Alex Williamson) > 3. abandoned changes to sample mdev drivers (Alex Williamson) > 4. mandatory --> optional (Cornelia Huck) > 5. added description for errno (Cornelia Huck) > > Cc: Alex Williamson > Cc: Erik Skultety > Cc: "Dr. David Alan Gilbert" > Cc: Cornelia Huck > Cc: "Tian, Kevin" > Cc: Zhenyu Wang > Cc: "Wang, Zhi A" > Cc: Neo Jia > Cc: Kirti Wankhede > Cc: Daniel P. Berrangé > Cc: Christophe de Dinechin > > Signed-off-by: Yan Zhao > --- > Documentation/vfio-mediated-device.txt | 140 + > 1 file changed, 140 insertions(+) > > diff --git a/Documentation/vfio-mediated-device.txt > b/Documentation/vfio-mediated-device.txt > index c3f69bcaf96e..013a764968eb 100644 > --- a/Documentation/vfio-mediated-device.txt > +++ b/Documentation/vfio-mediated-device.txt > @@ -202,6 +202,7 @@ Directories and files under the sysfs for Each Physical > Device >| | |--- available_instances >| | |--- device_api >| | |--- description > + | | |--- version >| | |--- [devices] >| |--- [] >| | |--- create > @@ -209,6 +210,7 @@ Directories and files under the sysfs for Each Physical > Device >| | |--- available_instances >| | |--- device_api >| | |--- description > + | | |--- version >| | |--- [devices] >| |--- [] >| |--- create > @@ -216,6 +218,7 @@ Directories and files under the sysfs for Each Physical > Device >| |--- available_instances >| |--- device_api >| |--- description > + | |--- version >| |--- [devices] I thought there was a request to make this more specific to migration by renaming it to something like migration_version. Also, as an optional attribute, it seems the example should perhaps not add it to all types to illustrate that it is not required. > > * [mdev_supported_types] > @@ -246,6 +249,143 @@ Directories and files under the sysfs for Each Physical > Device >This attribute should show the number of devices of type that > can be >created. > > +* version > + > + This attribute is rw, and is optional. > + It is used to check device compatibility between two mdev devices and is between two mdev devices of the same type. > + accessed in pairs between the two mdev devices being checked. "in pairs"? > + The intent of this attribute is to make an mdev device's version opaque to > + user space, so instead of reading two mdev devices' version strings and perhaps "...instead of reading the version string of two mdev devices and comparing them in userspace..." > + comparing in userspace, user space should only read one mdev device's > version > + attribute, and writes this version string into the other mdev device's > version > + attribute. Then vendor driver of mdev device whose version attribute being > + written would check the incoming version string and tell user space whether > + the two mdev devices are compatible via return value. That's why this > + attribute is writable. > + > + when reading this attribute, it should show device version string of > +
Re: [libvirt] [PATCH 0/2] Avoid issues due to qemu dropping osxsave and ospke
On 4/25/19 9:50 AM, Christian Ehrhardt wrote: Hi, this series tries to address a drop of commandline options by qemu in regard to osxsave [1] and ospke [2]. This was already discussed in [3] late last year but got forgotten afterwards. The Ubuntu bug is at [4] and an older Fedora bug is at [5]. TL;DR: - osxsave/ospke features were never really configurable - KVM never returned the bits on GET_SUPPORTED_CPUID - very rare to be seen in the wild - avoid issues with newer qemu and old/odd XMLs to be sure Details: I checked various use cases from virt-install to openstack and some in between. The only cases I found that would define osxsave/ospke is virt-install pior to version 2.0 and even there only when used with --cpu=host-model or --cpu=host-copy. If you ever really enabled the feature you'd have got: error: the CPU is incompatible with host CPU: Host CPU does not provide required features: ospke The problem lies in domain XMLs that explicitly disable it. That would be But due to almost (or actually none) no host exposing this the following also triggers: So, it happened that this notebook (ThinkPad T480) has the feature you're handling in this patch series: [danielhb@rekt libvirt]$ sudo ./run tools/virsh capabilities [sudo] password for danielhb: 985fc2cc-23a6-11b2-a85c-a1357d626e56 x86_64 Skylake-Client-IBRS Intel (...) (...) With your approach, what happens is that a feature that is declared to be effective in the capabilities is, in fact, ignored. It is an upgrade of what would happen without it, of course. But I am wondering here, fully aware that you mentioned that you wanted this discussion to be avoided, that we should simply exterminate both osxsave and ospke from Libvirt capabilities. I mean, what's the point of even reporting them if they will end up being ignored? You have both QEMU commits [1] and [2] mentioning that these flags were never configurable in the first place, so it's not like we need to keep them for backward compatibility either. Note that this does not discard this patch series - I think we'll need a solution like this to deal with older VMs that define these features in their XMLs. Thanks, DHB This will make libvirt add it to the qemu commandline like: -cpu ...,osxsave=off,ospke=off And that will crash when qemu starts with: error: internal error: process exited while connecting to monitor: 2019-04-25T12:12:01.698646Z qemu-system-x86_64: can't apply global core2duo-x86_64-cpu.osxsave=off: Property '.osxsave' not found There are much more long term discussions about demoting and dropping qemu features and I'd like to avoid those discussions being mixed. The reason to drop it more or less without notice was that it never did anything to begin with. Due to that our solution might in a similar fashion be more trivial - just stop defining those two features to qemu commandline. [1]: https://git.qemu.org/?p=qemu.git;a=commit;h=f1a23522b03a569f13aad49294bb4c4b1a9500c7 [2]: https://git.qemu.org/?p=qemu.git;a=commit;h=9ccb9784b57804f5c74434ad6ccb66650a015ffc [3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg561877.html [4]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1825195 [5]: https://bugzilla.redhat.com/show_bug.cgi?id=1644848 Christian Ehrhardt (2): qemu: do not define known no-op features qemuxml2argvtest: add test for remove cpu features src/qemu/qemu_command.c | 23 +++ .../qemuxml2argvdata/cpu-host-model-cmt.args | 2 +- .../cpu-no-removed-features.args | 29 +++ .../cpu-no-removed-features.xml | 23 +++ tests/qemuxml2argvdata/cpu-tsc-frequency.args | 4 +-- tests/qemuxml2argvtest.c | 1 + 6 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.args create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.xml -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemuxml2argvtest: add test for remove cpu features
On 4/25/19 9:50 AM, Christian Ehrhardt wrote:
CPU features that always were a no-op in qemu got removed there.
We no more specify them as that would trigger errors and fail to start
qemu. This test ensures that those features really are not rendered into
qemu command line.
Without the related fix this test will trigger and fail like:
In 'tests/qemuxml2argvdata/cpu-no-removed-features.args':
Offset 371
Expect [ ]
Actual [,-osxsave,-ospke ]
Signed-off-by: Christian Ehrhardt
---
Reviewed-by: Daniel Henrique Barboza
.../cpu-no-removed-features.args | 29 +++
.../cpu-no-removed-features.xml | 23 +++
tests/qemuxml2argvtest.c | 1 +
3 files changed, 53 insertions(+)
create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.args
create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.xml
diff --git a/tests/qemuxml2argvdata/cpu-no-removed-features.args
b/tests/qemuxml2argvdata/cpu-no-removed-features.args
new file mode 100644
index 00..1e4af63dc3
--- /dev/null
+++ b/tests/qemuxml2argvdata/cpu-no-removed-features.args
@@ -0,0 +1,29 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=kvm,usb=off,dump-guest-core=off \
+-cpu core2duo \
+-m 214 \
+-realtime mlock=off \
+-smp 6,sockets=6,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/cpu-no-removed-features.xml
b/tests/qemuxml2argvdata/cpu-no-removed-features.xml
new file mode 100644
index 00..dc5a681901
--- /dev/null
+++ b/tests/qemuxml2argvdata/cpu-no-removed-features.xml
@@ -0,0 +1,23 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 6
+
+hvm
+
+
+
+core2duo
+
+
+
+
+ destroy
+ restart
+ destroy
+
+ /usr/bin/qemu-system-x86_64
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d5c6dc4c0c..edc19ace6f 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1649,6 +1649,7 @@ mymain(void)
DO_TEST("cpu-fallback", QEMU_CAPS_KVM);
DO_TEST_FAILURE("cpu-nofallback", QEMU_CAPS_KVM);
DO_TEST("cpu-strict1", QEMU_CAPS_KVM);
+DO_TEST("cpu-no-removed-features", QEMU_CAPS_KVM);
DO_TEST("cpu-numa1", NONE);
DO_TEST("cpu-numa2", NONE);
DO_TEST("cpu-numa-no-memory-element", NONE);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: do not define known no-op features
On 4/25/19 9:50 AM, Christian Ehrhardt wrote:
Qemu dropped cpu features for osxsave and ospke [1][2].
The reason for the instant removal is that those features were never
configurable as discussed in [3].
Fortunately the use cases adding those flags in the past are rare, but
they exist. One that I identified are e.g. older virt-install when used
with --cpu=host-model and there always could be the case of a user
adding it to the guest xml.
This triggers an issue like:
qemu-system-x86_64: can't apply global Broadwell-noTSX-x86_64-
cpu.osxsave=on: Property '.osxsave' not found
Ensure that this does no more break spawning newer qemu versions by
not rendering those features into the qemu command line.
Fixes: https://bugs.launchpad.net/fedora/+source/qemu/+bug/1825195
Resolves: https://bugzilla.redhat.com/1644848
I am not sure if I understood why this is the solution you opted for. I'll
start a discussion in the cover-letter, which seems more suitable.
As far as this patch goes:
Reviewed-by: Daniel Henrique Barboza
[1]: https://git.qemu.org/?p=qemu.git;a=commit;h=f1a2352
[2]: https://git.qemu.org/?p=qemu.git;a=commit;h=9ccb978
[3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg561877.html
Signed-off-by: Christian Ehrhardt
---
src/qemu/qemu_command.c | 23 +++
.../qemuxml2argvdata/cpu-host-model-cmt.args | 2 +-
tests/qemuxml2argvdata/cpu-tsc-frequency.args | 4 ++--
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1425d97b1e..e0c8ae50a1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7076,6 +7076,27 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd,
return 0;
}
+/**
+ * qemuFeatureNoEffect:
+ * @feature: CPU Feature
+ *
+ * Returns true, if the feature is known to have (never had) an effect on QEMU.
+ * Those features might be dropped in qemu without a longer deprecation cycle
+ * and must therefore be known e.g. to no more define them on command line.
+ */
+static bool
+qemuFeatureNoEffect(virCPUFeatureDefPtr feature)
+{
+if (!feature->name)
+return false;
+
+if (STREQ(feature->name, "osxsave"))
+return true;
+if (STREQ(feature->name, "ospke"))
+return true;
+
+return false;
+}
static int
qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
@@ -7144,6 +7165,8 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
virBufferAsprintf(buf, ",vendor=%s", cpu->vendor_id);
for (i = 0; i < cpu->nfeatures; i++) {
+if (qemuFeatureNoEffect(&(cpu->features[i])))
+continue;
switch ((virCPUFeaturePolicy) cpu->features[i].policy) {
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
diff --git a/tests/qemuxml2argvdata/cpu-host-model-cmt.args
b/tests/qemuxml2argvdata/cpu-host-model-cmt.args
index 36f706b836..42f969fd62 100644
--- a/tests/qemuxml2argvdata/cpu-host-model-cmt.args
+++ b/tests/qemuxml2argvdata/cpu-host-model-cmt.args
@@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \
-S \
-machine pc,accel=tcg,usb=off,dump-guest-core=off \
-cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,\
-+smx,+est,+tm2,+xtpr,+pdcm,+osxsave,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm \
++smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm \
-m 214 \
-realtime mlock=off \
-smp 6,sockets=6,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/cpu-tsc-frequency.args
b/tests/qemuxml2argvdata/cpu-tsc-frequency.args
index 55bcf89617..55b72b4404 100644
--- a/tests/qemuxml2argvdata/cpu-tsc-frequency.args
+++ b/tests/qemuxml2argvdata/cpu-tsc-frequency.args
@@ -12,8 +12,8 @@ QEMU_AUDIO_DRV=none \
-S \
-machine pc,accel=kvm,usb=off,dump-guest-core=off \
-cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,\
-+smx,+est,+tm2,+xtpr,+pdcm,+osxsave,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,\
-+invtsc,tsc-frequency=350400 \
++smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,+invtsc,\
+tsc-frequency=350400 \
-m 214 \
-realtime mlock=off \
-smp 1,sockets=1,cores=1,threads=1 \
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] test_driver: implement virDomainGetDiskErrors
Return the number of disks present in the configuration of the fake
driver when called with @errors as NULL and @maxerrors as 0.
Otherwise return 0 as the number of errors encountered.
Signed-off-by: Ilias Stamatis
---
src/test/test_driver.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 460c896ef6..5fa9ab30f1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3046,6 +3046,32 @@ static int testDomainSetAutostart(virDomainPtr domain,
return 0;
}
+static int testDomainGetDiskErrors(virDomainPtr dom,
+ virDomainDiskErrorPtr errors,
+ unsigned int maxerrors ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+int ret = -1;
+virDomainObjPtr vm = NULL;
+
+virCheckFlags(0, -1);
+
+if (!(vm = testDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto cleanup;
+
+if (!errors)
+ret = vm->def->ndisks;
+else
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
static char *testDomainGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED,
int *nparams)
{
@@ -6833,6 +6859,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */
.domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
.domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
+.domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */
.domainGetSchedulerType = testDomainGetSchedulerType, /* 0.3.2 */
.domainGetSchedulerParameters = testDomainGetSchedulerParameters, /* 0.3.2
*/
.domainGetSchedulerParametersFlags =
testDomainGetSchedulerParametersFlags, /* 0.9.2 */
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] QMP; unsigned 64-bit ints; JSON standards compliance
On 5/7/19 4:39 AM, Daniel P. Berrangé wrote: >> JSON is terrible at interoperability, so good luck with that. >> >> If you reduce your order to "the commonly used JSON libraries we know", >> we can talk. > > I don't particularly want us to rely on semantics of small known set > of JSON libs. I really do want us to do something that is capable of > working with any JSON impl that exists in any programming language. > > My suggested option 2 & 3 at least would manage that I believe, as > any credible JSON impl will be able to represent 32-bit integers > or strings without loosing data. > > Option 1 would not cope as some impls can't even cope with > signed 64-bit ints. > > I can think of some options: > > 1. Encode unsigned 64-bit integers as signed 64-bit integers. > > This follows the example that most C libraries map JSON ints > to 'long long int'. This is still relying on undefined > behaviour as apps don't need to support > 2^53-1. > > Apps would need to cast back to 'unsigned long long' for > those QMP fields they know are supposed to be unsigned. >> >> Ugly. It's also what we did until v2.10, August 2017. QMP's input >> direction still does it, for backward compatibility. Having qemu accept signed ints in place of large unsigned values is easy enough. But you are right that it loses precision when doubles are involved on the receiving end, and we cross the 2^53 barrier. >> > > > 2. Encode all 64-bit integers as a pair of 32-bit integers. > > This is fully compliant with the JSON spec as each half > is fully within the declared limits. App has to split or > assemble the 2 pieces from/to a signed/unsigned 64-bit > int as needed. >> >> Differently ugly. Particularly ugly as we turn 1<<55 from: "value":36028797018963968 into "value":[8388608,0] and now both qemu and the client end have to agree that an array of two integers is a valid replacement for any larger 64-bit quantity (presumably, we'd always accept the array form even for small integer values, but only produce the array form for large values). And while it manages just fine for uint64_t values, what rules would you place on int64_t values? That the resulting 2-integer array is combined with the first number as a 2's-complement signed value, and the second being a 32-bit unsigned value? >> > > > 3. Encode all 64-bit integers as strings > > The application has todo all parsing/formatting client > side. >> >> Yet another ugly. But less so than option 2. "value":36028797018963968 vs. "value":"36028797018963968" is at least tolerable. >> > > > None of these changes are backwards compatible, so I doubt we could make > the change transparently in QMP. Instead we would have to have a > QMP greeting message capability where the client can request enablement > of the enhanced integer handling. >> >> We might be able to do option 1 without capability negotiation. v2.10's >> change from option 1 to what we have now produced zero complaints. >> >> On the other hand, we made that change for a reason, so we may want a >> "send large integers as negative integers" capability regardless. >> > > Any of the three options above would likely work for libvirt, but I > would have a slight preference for either 2 or 3, so that we become > 100% standards compliant. If we're going to negotiate something, I'd lean towards option 3 (anywhere the introspection states that we accept 'int64' or similar, it is also appropriate to send a string value in its place). We'd also have to decide if we want to allow "0xabcd", or strictly insist on 43981, when stringizing an integer. And while qemu should accept a string or a number on input, we'd still have to decide/document whether it's response to the client capability negotiation is to output a string always, or only for values larger than the 2^53 threshold. >> >> There's no such thing. You mean "we maximize interoperability with >> common implementations of JSON". > > s/common/any/ > >> Let's talk implementation for a bit. >> >> Encoding and decoding integers in funny ways should be fairly easy in >> the QObject visitors. The generated QMP marshallers all use them. >> Trouble is a few commands still bypass the generated marshallers, and >> mess with the QObject themselves: >> >> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s >> comment. Should be harmless. >> >> * netdev_add: not QAPIfied. Eric's patches to QAPIfy it got stuck >> because they reject some abuses like passing numbers and bools as >> strings. >> >> * device_add: not QAPIfied. We're not sure QAPIfication is feasible. >> >> netdev_add and device_add both use qemu_opts_from_qdict(). Perhaps we >> could hack that to mirror what the QObject visitor do. >> >> Else, we might have to do it in the JSON parser. Sh
[libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
The way it works now the Makefile needs to be both make valid and automake valid. That is fine for now, but if we want to use anything more advanced, like conditionals, we cannot have it like that any more. So instead forward all ci-* rules to that file. Signed-off-by: Martin Kletzander --- Makefile.am | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 0d8bb733e6d2..442bae511828 100644 --- a/Makefile.am +++ b/Makefile.am @@ -35,6 +35,7 @@ EXTRA_DIST = \ libvirt-qemu.pc.in \ libvirt-lxc.pc.in \ libvirt-admin.pc.in \ + Makefile.ci \ Makefile.nonreentrant \ autogen.sh \ cfg.mk \ @@ -107,4 +108,5 @@ gen-AUTHORS: rm -f all.list maint.list contrib.list; \ fi -include Makefile.ci +ci-%: + $(MAKE) -f Makefile.ci $@ -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Add support for podman in Makefile.ci
This way more users can run our CI builds locally.
Signed-off-by: Martin Kletzander
---
Makefile.ci | 125 ++--
1 file changed, 93 insertions(+), 32 deletions(-)
diff --git a/Makefile.ci b/Makefile.ci
index 12a62167cc67..e2989ada313c 100644
--- a/Makefile.ci
+++ b/Makefile.ci
@@ -17,7 +17,7 @@ CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
# The directory holding the source inside the
-# container. ie where we told Docker to expose
+# container. ie where we want to expose
# the $(CI_HOST_SRCDIR) directory from the host
CI_CONT_SRCDIR = /src
@@ -46,14 +46,13 @@ CI_CONFIGURE_ARGS =
# cloning them
CI_SUBMODULES = $(shell git submodule | awk '{ print $$2 }')
-# Location of the Docker images we're going to pull
+# Location of the container images we're going to pull
# Can be useful to overridde to use a locally built
# image instead
CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-
-# Docker defaults to pulling the ':latest' tag but
-# if the Docker repo above uses different conventions
-# this can override it
+# The default tag is ':latest' but if the container
+# repo above uses different conventions this can override it
CI_IMAGE_TAG = :master
# We delete the virtual root after completion, set
@@ -71,24 +70,82 @@ CI_REUSE = 0
CI_UID = $(shell id -u)
CI_GID = $(shell id -g)
-# Docker doesn't require the IDs you run as to exist in
+# Container engine runtime we are going to use, can be overridden per make
+# invocation, if it is not, we try podman and then default to docker.
+ifeq ($(CI_CENGINE),)
+ CI_CENGINE = $(shell podman version >/dev/null && echo podman || echo
docker)
+endif
+
+# IDs you run as do not need to exist in
# the container's /etc/passwd & /etc/group files, but
-# if they do not, then libvirt's 'make check' will fail
+# if they do not, then libvirt's 'make check' will fail
# many tests.
-#
-# We do not directly mount /etc/{passwd,group} as Docker
-# is liable to mess with SELinux labelling which will
-# then prevent the host accessing them. Copying them
-# first is safer.
-CI_PWDB_MOUNTS = \
- --volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
- --volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
- $(NULL)
+ifeq ($(CI_CENGINE),podman)
+ CI_PWDB_MOUNTS = \
+ --volume /etc/group:/etc/group:ro,z \
+ --volume /etc/passwd:/etc/passwd:ro,z \
+ $(NULL)
+else
+ # We do not directly mount /etc/{passwd,group} as Docker
+ # is liable to mess with SELinux labelling which will
+ # then prevent the host accessing them. Copying them
+ # first is safer.
+ CI_PWDB_MOUNTS = \
+ --volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
+ --volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
+ $(NULL)
+endif
+
+ifeq ($(CI_CENGINE),docker)
+ # Docker containers can have very large ulimits
+ # for nofiles - as much as 1048576. This makes
+ # libvirt very slow at exec'ing programs.
+ CI_ULIMIT_FILES = 1024
+endif
-# Docker containers can have very large ulimits
-# for nofiles - as much as 1048576. This makes
-# libvirt very slow at exec'ing programs.
-CI_ULIMIT_FILES = 1024
+ifeq ($(CI_CENGINE),podman)
+ # Podman cannot reuse host namespace when running non-root containers.
Until
+ # support for --keep-uid is added we can just create another mapping
that will
+ # do that for us. Beware, that in
{uid,git}map=container_id:host_id:range,
+ # the host_id does actually refer to the uid in the first mapping where 0
+ # (root) is mapped to the current user and rest is offset.
+
+ # In order to set up this mapping, we need to keep all the user IDs to
prevent
+ # possible errors as some images might expect UIDs up to 9 (looking
at you
+ # fedora), so we don't want the overflowuid to be used for them. For
mapping
+ # all the other users properly ther eneeds to be some math done. Don't
worry,
+ # it's just addition and subtraction.
+
+ # 65536 ought to be enough (tm), but for really rare cases the maximums
might
+ # need to be higher, but that only happens when your /etc/sub{u,g}id
allow
+ # users to have more IDs. Unless --keep-uid is supported, let's do
this in a
+ # way that should work for everyone.
+ CI_MAX_UID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subuid)
+ CI_MAX_GID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subgid)
+ ifeq ($(CI_MAX_UID),)
+ CI_MAX_UID = 65536
+ endif
+ ifeq ($(CI_MAX_GID),)
+ CI_MAX_GID = 65536
+ endif
+ CI_UID_OTHER = $(shell echo $$(($(CI_UID)+1)))
+ CI_GID_OTHER = $(shell echo $$(($(CI_GID)+1)))
+ CI_UID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_UID)-$(CI_UID
+ CI_GID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_GID)-$(CI_GID
+
+
[libvirt] [PATCH 0/2] Support more container engines in Makefile.ci
Maybe someone can add support for libvirt-lxc later on =) Martin Kletzander (2): Don't include Makefile.ci in Makefile.am Add support for podman in Makefile.ci Makefile.am | 4 +- Makefile.ci | 125 ++-- 2 files changed, 96 insertions(+), 33 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/12] qemu: Refactor/simplify qemuDomainStorageSourceAccessRevoke
On Thu, Apr 18, 2019 at 04:43:02PM +0200, Peter Krempa wrote: Use qemuDomainStorageSourceAccessModify instead of the individual calls. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/12] qemu: Convert boolean flags to enum flags in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:00PM +0200, Peter Krempa wrote: Upcoming patches will add a few more flags. Add an enum to collect them so that we don't end up with multiple bools. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/12] qemu: Allow using qemuDomainStorageSourceAccessModify on singe images
On Thu, Apr 18, 2019 at 04:43:01PM +0200, Peter Krempa wrote: Add a new flag QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN to select whether to work on single image or full chain. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 27 +-- 1 file changed, 21 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Move tools under tests/tools/
On Tue, May 07, 2019 at 12:15:54 +0100, Daniel Berrange wrote: > On Tue, May 07, 2019 at 12:49:18PM +0200, Michal Privoznik wrote: > > On 5/7/19 11:13 AM, Daniel P. Berrangé wrote: > > > On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote: > > > > There are some scripts/binaries that are not tests themselves but > > > > rather fulfill support purpose. Separate them from the rest of > > > > the tests. > > > > > > > > Signed-off-by: Michal Privoznik > > > > --- [...] > > Personally, I find having code along side with the tests even more > > disaranged than what we have now. For some unit tests we can have strict > > naming scheme: virhash.c -> virhashtest.c; but for some more advanced tests > > qemuxml2*test this won't work. > > We don't need to rename everything - that could be src/qemu/qemuxml2argvtest.c > easily enough E that is super gross. I strongly prefer to keep the tests stashed into a directory (thus qemu/tests/qemuxml2argvtest.c) to avoid having a gazillion files in the main directory for the driver. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 0/4] guests: Introduce package_manager
As mentioned in [1] and detailed in [2], Fedora is planning to drop the 'yum' command and require users to call 'dnf' exclusively. In fact, it looks like this change has already been implemented in Rawhide, so we need to adapt sooner rather than later. The first two patches address the actual compatibility issue, while the remaining ones perform some cleanups which are either made much easier by the previous changes or I just happened to stumble upon while working on this :) [1] https://www.redhat.com/archives/libvir-list/2019-May/msg00043.html [2] https://fedoraproject.org/wiki/Changes/Retire_YUM_3 Andrea Bolognani (4): guests: Introduce package_manager guests: Use package_manager everywhere guests: Don't call out to the shell twice lcitool: Fix Dockerfile alignment guests/host_vars/libvirt-centos-7/main.yml| 1 + guests/host_vars/libvirt-debian-9/main.yml| 1 + guests/host_vars/libvirt-debian-sid/main.yml | 1 + guests/host_vars/libvirt-fedora-29/main.yml | 1 + guests/host_vars/libvirt-fedora-30/main.yml | 1 + .../host_vars/libvirt-fedora-rawhide/main.yml | 1 + guests/host_vars/libvirt-freebsd-11/main.yml | 1 + guests/host_vars/libvirt-freebsd-12/main.yml | 1 + .../libvirt-freebsd-current/main.yml | 1 + guests/host_vars/libvirt-ubuntu-18/main.yml | 1 + guests/lcitool| 48 ++- guests/playbooks/update/tasks/base.yml| 19 ++-- guests/playbooks/update/tasks/bootstrap.yml | 16 +-- 13 files changed, 42 insertions(+), 51 deletions(-) -- 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 1/4] guests: Introduce package_manager
Platforms that share the same package format might still want to use different package managers, a good example being CentOS with yum and Fedora with dnf. Signed-off-by: Andrea Bolognani --- guests/host_vars/libvirt-centos-7/main.yml| 1 + guests/host_vars/libvirt-debian-9/main.yml| 1 + guests/host_vars/libvirt-debian-sid/main.yml | 1 + guests/host_vars/libvirt-fedora-29/main.yml | 1 + guests/host_vars/libvirt-fedora-30/main.yml | 1 + guests/host_vars/libvirt-fedora-rawhide/main.yml | 1 + guests/host_vars/libvirt-freebsd-11/main.yml | 1 + guests/host_vars/libvirt-freebsd-12/main.yml | 1 + guests/host_vars/libvirt-freebsd-current/main.yml | 1 + guests/host_vars/libvirt-ubuntu-18/main.yml | 1 + 10 files changed, 10 insertions(+) diff --git a/guests/host_vars/libvirt-centos-7/main.yml b/guests/host_vars/libvirt-centos-7/main.yml index fa4fc67..94e29af 100644 --- a/guests/host_vars/libvirt-centos-7/main.yml +++ b/guests/host_vars/libvirt-centos-7/main.yml @@ -13,6 +13,7 @@ projects: - virt-viewer package_format: 'rpm' +package_manager: 'yum' os_name: 'CentOS' os_version: '7' diff --git a/guests/host_vars/libvirt-debian-9/main.yml b/guests/host_vars/libvirt-debian-9/main.yml index ec7e6b4..6b685a4 100644 --- a/guests/host_vars/libvirt-debian-9/main.yml +++ b/guests/host_vars/libvirt-debian-9/main.yml @@ -17,6 +17,7 @@ projects: - virt-viewer package_format: 'deb' +package_manager: 'apt-get' os_name: 'Debian' os_version: '9' diff --git a/guests/host_vars/libvirt-debian-sid/main.yml b/guests/host_vars/libvirt-debian-sid/main.yml index 1c7a29b..3808383 100644 --- a/guests/host_vars/libvirt-debian-sid/main.yml +++ b/guests/host_vars/libvirt-debian-sid/main.yml @@ -17,6 +17,7 @@ projects: - virt-viewer package_format: 'deb' +package_manager: 'apt-get' os_name: 'Debian' os_version: 'Sid' diff --git a/guests/host_vars/libvirt-fedora-29/main.yml b/guests/host_vars/libvirt-fedora-29/main.yml index bebf171..ac0228d 100644 --- a/guests/host_vars/libvirt-fedora-29/main.yml +++ b/guests/host_vars/libvirt-fedora-29/main.yml @@ -18,6 +18,7 @@ projects: - virt-viewer package_format: 'rpm' +package_manager: 'dnf' os_name: 'Fedora' os_version: '29' diff --git a/guests/host_vars/libvirt-fedora-30/main.yml b/guests/host_vars/libvirt-fedora-30/main.yml index 4ad27a6..491b112 100644 --- a/guests/host_vars/libvirt-fedora-30/main.yml +++ b/guests/host_vars/libvirt-fedora-30/main.yml @@ -18,6 +18,7 @@ projects: - virt-viewer package_format: 'rpm' +package_manager: 'dnf' os_name: 'Fedora' os_version: '30' diff --git a/guests/host_vars/libvirt-fedora-rawhide/main.yml b/guests/host_vars/libvirt-fedora-rawhide/main.yml index ed0a3fa..db46825 100644 --- a/guests/host_vars/libvirt-fedora-rawhide/main.yml +++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml @@ -28,6 +28,7 @@ projects: - virt-viewer+mingw64 package_format: 'rpm' +package_manager: 'dnf' os_name: 'Fedora' os_version: 'Rawhide' diff --git a/guests/host_vars/libvirt-freebsd-11/main.yml b/guests/host_vars/libvirt-freebsd-11/main.yml index ed805c9..e9f6d03 100644 --- a/guests/host_vars/libvirt-freebsd-11/main.yml +++ b/guests/host_vars/libvirt-freebsd-11/main.yml @@ -16,6 +16,7 @@ projects: - virt-viewer package_format: 'pkg' +package_manager: 'pkg' os_name: 'FreeBSD' os_version: '11' diff --git a/guests/host_vars/libvirt-freebsd-12/main.yml b/guests/host_vars/libvirt-freebsd-12/main.yml index 8bbe158..ba3ba62 100644 --- a/guests/host_vars/libvirt-freebsd-12/main.yml +++ b/guests/host_vars/libvirt-freebsd-12/main.yml @@ -16,6 +16,7 @@ projects: - virt-viewer package_format: 'pkg' +package_manager: 'pkg' os_name: 'FreeBSD' os_version: '12' diff --git a/guests/host_vars/libvirt-freebsd-current/main.yml b/guests/host_vars/libvirt-freebsd-current/main.yml index 62498fd..74e1856 100644 --- a/guests/host_vars/libvirt-freebsd-current/main.yml +++ b/guests/host_vars/libvirt-freebsd-current/main.yml @@ -16,6 +16,7 @@ projects: - virt-viewer package_format: 'pkg' +package_manager: 'pkg' os_name: 'FreeBSD' os_version: 'Current' diff --git a/guests/host_vars/libvirt-ubuntu-18/main.yml b/guests/host_vars/libvirt-ubuntu-18/main.yml index bb465e8..5b5bf0c 100644 --- a/guests/host_vars/libvirt-ubuntu-18/main.yml +++ b/guests/host_vars/libvirt-ubuntu-18/main.yml @@ -17,6 +17,7 @@ projects: - virt-viewer package_format: 'deb' +package_manager: 'apt-get' os_name: 'Ubuntu' os_version: '18' -- 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 2/4] guests: Use package_manager everywhere
Instead of hardcoding the name of the package manager in
commands, use the value obtained from the inventory.
In some cases this is necessary, eg. when RPM-based
distributions are involved; for most other cases we could
get away with keepking the hardcoded names, but it's better
to be completely consistent to hopefully avoid usage of the
wrong package manager slipping with further changes.
Signed-off-by: Andrea Bolognani
---
guests/lcitool | 40 +++--
guests/playbooks/update/tasks/base.yml | 12 +++
guests/playbooks/update/tasks/bootstrap.yml | 16 ++---
3 files changed, 29 insertions(+), 39 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 60a01fc..c179775 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -591,6 +591,7 @@ class Application:
facts = self._inventory.get_facts(host)
package_format = facts["package_format"]
+package_manager = facts["package_manager"]
os_name = facts["os_name"]
os_version = facts["os_version"]
os_full = os_name + os_version
@@ -651,6 +652,7 @@ class Application:
print("FROM {}".format(facts["docker_base"]))
varmap = {}
+varmap["package_manager"] = package_manager
varmap["pkgs"] = " \\\n".join(sorted(set(pkgs.values(
if package_format == "deb":
if args.cross_arch:
@@ -667,12 +669,12 @@ class Application:
sys.stdout.write(textwrap.dedent("""
RUN export DEBIAN_FRONTEND=noninteractive && \\
-apt-get update && \\
-apt-get dist-upgrade -y && \\
-apt-get install --no-install-recommends -y \\
+{package_manager} update && \\
+{package_manager} dist-upgrade -y && \\
+{package_manager} install --no-install-recommends -y \\
{pkgs} && \\
-apt-get autoremove -y && \\
-apt-get autoclean -y
+{package_manager} autoremove -y && \\
+{package_manager} autoclean -y
""").format(**varmap))
if args.cross_arch:
# Intentionally a separate RUN command from the above
@@ -681,12 +683,12 @@ class Application:
sys.stdout.write(textwrap.dedent("""
RUN export DEBIAN_FRONTEND=noninteractive && \\
dpkg --add-architecture {cross_arch} && \\
-apt-get update && \\
-apt-get dist-upgrade -y && \\
-apt-get install --no-install-recommends -y \\
+{package_manager} update && \\
+{package_manager} dist-upgrade -y && \\
+{package_manager} install --no-install-recommends -y \\
{cross_pkgs} && \\
-apt-get autoremove -y && \\
-apt-get autoclean -y
+{package_manager} autoremove -y && \\
+{package_manager} autoclean -y
ENV ABI "{cross_abi}"
ENV CONFIGURE_OPTS "--host={cross_abi} \\
@@ -696,18 +698,18 @@ class Application:
elif package_format == "rpm":
if os_name == "Fedora" and os_version == "Rawhide":
sys.stdout.write(textwrap.dedent("""
-RUN yum update -y --nogpgcheck fedora-gpg-keys && \\
-yum update -y && \\
-yum install -y {pkgs} && \\
-yum autoremove -y && \\
-yum clean all -y
+RUN {package_manager} update -y --nogpgcheck
fedora-gpg-keys && \\
+{package_manager} update -y && \\
+{package_manager} install -y {pkgs} && \\
+{package_manager} autoremove -y && \\
+{package_manager} clean all -y
""").format(**varmap))
else:
sys.stdout.write(textwrap.dedent("""
-RUN yum update -y && \\
-yum install -y {pkgs} && \\
-yum autoremove -y && \\
-yum clean all -y
+RUN {package_manager} update -y && \\
+{package_manager} install -y {pkgs} && \\
+{package_manager} autoremove -y && \\
+{package_manager} clean all -y
""").format(**varmap))
def run(self):
diff --git a/guests/playbooks/update/tasks/base.yml
b/guests/playbooks/update/tasks/base.yml
index d71d99f..59f6124 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -28,7 +28,7 @@
- os_version == 'Rawhide'
- name: Update installed packag
[libvirt] [jenkins-ci PATCH 4/4] lcitool: Fix Dockerfile alignment
Now that we are using package_manager everywhere instead of
hardcoding the names, it's finally possible to make the
alignment of the resulting Dockerfiles perfect.
Signed-off-by: Andrea Bolognani
---
guests/lcitool | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index c179775..5cf8efe 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -649,11 +649,13 @@ class Application:
if pkgs[package] is None or cross_policy in ["skip",
"foreign"]:
del pkgs[package]
+pkg_align = " \\\n" + (" " * len("RUN " + package_manager + " "))
+
print("FROM {}".format(facts["docker_base"]))
varmap = {}
varmap["package_manager"] = package_manager
-varmap["pkgs"] = " \\\n".join(sorted(set(pkgs.values(
+varmap["pkgs"] = pkg_align[1:] +
pkg_align.join(sorted(set(pkgs.values(
if package_format == "deb":
if args.cross_arch:
deb_arch = Util.native_arch_to_deb_arch(args.cross_arch)
@@ -663,7 +665,7 @@ class Application:
varmap["cross_arch"] = deb_arch
pkg_names = [p + ":" + deb_arch for p in cross_pkgs.values()]
pkg_names.append(gcc)
-varmap["cross_pkgs"] = " \\\n
".join(sorted(set(pkg_names)))
+varmap["cross_pkgs"] = pkg_align[1:] +
pkg_align.join(sorted(set(pkg_names)))
varmap["cross_abi"] = abi
varmap["cross_lib"] = lib
@@ -671,8 +673,7 @@ class Application:
RUN export DEBIAN_FRONTEND=noninteractive && \\
{package_manager} update && \\
{package_manager} dist-upgrade -y && \\
-{package_manager} install --no-install-recommends -y \\
-{pkgs} && \\
+{package_manager} install --no-install-recommends -y
{pkgs} && \\
{package_manager} autoremove -y && \\
{package_manager} autoclean -y
""").format(**varmap))
@@ -685,8 +686,7 @@ class Application:
dpkg --add-architecture {cross_arch} && \\
{package_manager} update && \\
{package_manager} dist-upgrade -y && \\
-{package_manager} install --no-install-recommends -y \\
-{cross_pkgs} && \\
+{package_manager} install --no-install-recommends -y
{cross_pkgs} && \\
{package_manager} autoremove -y && \\
{package_manager} autoclean -y
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 3/4] guests: Don't call out to the shell twice
We're already doing this for FreeBSD, so do it for CentOS
and Fedora as well.
Signed-off-by: Andrea Bolognani
---
guests/playbooks/update/tasks/base.yml | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/guests/playbooks/update/tasks/base.yml
b/guests/playbooks/update/tasks/base.yml
index 59f6124..8fe114e 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -56,16 +56,7 @@
- package_format == 'pkg'
- name: Clean up packages after update
- command: '{{ package_manager }} clean packages -y'
- args:
-warn: no
- when:
-- package_format == 'rpm'
-
-- name: Clean up packages after update
- command: '{{ package_manager }} autoremove -y'
- args:
-warn: no
+ shell: '{{ package_manager }} clean packages -y && {{ package_manager }}
autoremove -y'
when:
- package_format == 'rpm'
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] tests: Move tools under tests/tools/
There are some scripts/binaries that are not tests themselves but
rather fulfill support purpose. Separate them from the rest of
the tests.
Signed-off-by: Michal Privoznik
---
diff to v1:
- Found a way to generate a binary into a subdir. So no new Makefile.am
is introduced
.gitignore | 2 +-
cfg.mk | 4 ++--
tests/Makefile.am | 22 +++-
tests/qemucapabilitiestest.c| 4 ++--
tests/testutils.c | 2 +-
tests/{ => tools}/.valgrind.supp| 0
tests/tools/Makefile.inc.am | 23 +
tests/{ => tools}/check-file-access.pl | 2 +-
tests/{ => tools}/file_access_whitelist.txt | 0
tests/{ => tools}/group-qemu-caps.pl| 0
tests/{ => tools}/oomtrace.pl | 0
tests/{ => tools}/qemucapsprobe.c | 2 +-
tests/{ => tools}/qemucapsprobemock.c | 0
tests/{ => tools}/test-wrap-argv.pl | 2 +-
14 files changed, 35 insertions(+), 28 deletions(-)
rename tests/{ => tools}/.valgrind.supp (100%)
create mode 100644 tests/tools/Makefile.inc.am
rename tests/{ => tools}/check-file-access.pl (98%)
rename tests/{ => tools}/file_access_whitelist.txt (100%)
rename tests/{ => tools}/group-qemu-caps.pl (100%)
rename tests/{ => tools}/oomtrace.pl (100%)
rename tests/{ => tools}/qemucapsprobe.c (96%)
rename tests/{ => tools}/qemucapsprobemock.c (100%)
rename tests/{ => tools}/test-wrap-argv.pl (98%)
diff --git a/.gitignore b/.gitignore
index 16eb4a3e2e..c231d394f3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,7 +170,7 @@
/tests/*.trs
/tests/*test
/tests/commandhelper
-/tests/qemucapsprobe
+/tests/tools/qemucapsprobe
!/tests/virsh-self-test
!/tests/virt-aa-helper-test
!/tests/virt-admin-self-test
diff --git a/cfg.mk b/cfg.mk
index b785089910..5e055023ee 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1169,10 +1169,10 @@ header-ifdef:
test-wrap-argv:
$(AM_V_GEN)$(VC_LIST) | $(GREP) -E '\.(ldargs|args)' | xargs \
- $(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check
+ $(PERL) $(top_srcdir)/tests/tools/test-wrap-argv.pl --check
group-qemu-caps:
- $(AM_V_GEN)$(PERL) $(top_srcdir)/tests/group-qemu-caps.pl --check
$(top_srcdir)/
+ $(AM_V_GEN)$(PERL) $(top_srcdir)/tests/tools/group-qemu-caps.pl --check
$(top_srcdir)/
# sc_po_check can fail if generated files are not built first
sc_po_check: \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 46d94d2236..c63e8a6e70 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -75,7 +75,6 @@ MOCKLIBS_LIBS = \
../src/libvirt.la
EXTRA_DIST = \
- .valgrind.supp \
bhyvexml2argvdata \
bhyveargv2xmldata \
bhyvexml2xmloutdata \
@@ -107,7 +106,6 @@ EXTRA_DIST = \
nwfilterxml2firewalldata \
nwfilterxml2xmlin \
nwfilterxml2xmlout \
- oomtrace.pl \
qemuagentdata \
qemuargv2xmldata \
qemublocktestdata \
@@ -285,12 +283,10 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \
qemusecuritytest \
qemufirmwaretest \
$(NULL)
-test_helpers += qemucapsprobe
test_libraries += libqemumonitortestutils.la \
libqemutestdriver.la \
qemuxml2argvmock.la \
qemucaps2xmlmock.la \
- qemucapsprobemock.la \
qemucpumock.la \
$(NULL)
endif WITH_QEMU
@@ -442,15 +438,13 @@ EXTRA_DIST += $(test_scripts)
if WITH_LINUX
check-access: file-access-clean
VIR_TEST_FILE_ACCESS=1 $(MAKE) $(AM_MAKEFLAGS) check
- $(PERL) check-file-access.pl | sort -u
+ $(PERL) tools/check-file-access.pl | sort -u
file-access-clean:
> test_file_access.txt
endif WITH_LINUX
-EXTRA_DIST += \
- check-file-access.pl \
- file_access_whitelist.txt
+include tools/Makefile.inc.am
if WITH_TESTS
noinst_PROGRAMS = $(test_programs) $(test_helpers)
@@ -478,7 +472,7 @@ TESTS_ENVIRONMENT = \
VALGRIND = valgrind --quiet --leak-check=full --trace-children=yes \
--trace-children-skip="*/tools/virsh","*/tests/commandhelper" \
- --suppressions=$(abs_srcdir)/.valgrind.supp
+ --suppressions=$(abs_srcdir)/tools/.valgrind.supp
valgrind:
$(MAKE) check VG="$(LIBTOOL) --mode=execute $(VALGRIND)"
@@ -603,16 +597,6 @@ qemucapabilitiestest_SOURCES = \
qemucapabilitiestest_LDADD = libqemumonitortestutils.la \
$(qemu_LDADDS) $(LDADDS)
-qemucapsprobe_SOURCES = \
- qemucapsprobe.c
-qemucapsprobe_LDADD = \
- libqemutestdriver.la $(LDADDS)
-
-qemucapsprobemock_la_SOURCES = \
- qemucapsprobemock.c
-qemucapsprobemock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
-qemucapsprobemock_la_LIBADD = $(MOCKLIBS_LIBS)
-
qemucommandutiltest_SOURCES = \
qemucommandutiltest.c \
testutils.c testutils.h \
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.
Re: [libvirt] [PATCH] tests: Move tools under tests/tools/
On 5/7/19 1:15 PM, Daniel P. Berrangé wrote:
On Tue, May 07, 2019 at 12:49:18PM +0200, Michal Privoznik wrote:
On 5/7/19 11:13 AM, Daniel P. Berrangé wrote:
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
There are some scripts/binaries that are not tests themselves but
rather fulfill support purpose. Separate them from the rest of
the tests.
Signed-off-by: Michal Privoznik
---
.gitignore | 2 +-
Makefile.am | 2 +-
cfg.mk | 4 +-
configure.ac| 1 +
tests/Makefile.am | 22 +-
tests/qemucapabilitiestest.c| 4 +-
tests/testutils.c | 2 +-
tests/{ => tools}/.valgrind.supp| 0
tests/tools/Makefile.am | 85 +
tests/{ => tools}/check-file-access.pl | 0
tests/{ => tools}/file_access_whitelist.txt | 0
tests/{ => tools}/group-qemu-caps.pl| 0
tests/{ => tools}/oomtrace.pl | 0
tests/{ => tools}/qemucapsprobe.c | 0
tests/{ => tools}/qemucapsprobemock.c | 0
tests/{ => tools}/test-wrap-argv.pl | 2 +-
16 files changed, 98 insertions(+), 26 deletions(-)
rename tests/{ => tools}/.valgrind.supp (100%)
create mode 100644 tests/tools/Makefile.am
rename tests/{ => tools}/check-file-access.pl (100%)
rename tests/{ => tools}/file_access_whitelist.txt (100%)
rename tests/{ => tools}/group-qemu-caps.pl (100%)
rename tests/{ => tools}/oomtrace.pl (100%)
rename tests/{ => tools}/qemucapsprobe.c (100%)
rename tests/{ => tools}/qemucapsprobemock.c (100%)
rename tests/{ => tools}/test-wrap-argv.pl (98%)
If we're going to re-arrange our tests, then I would really like to
see the goal be to remove the top level "tests/" directory entirely.
Instead move the tests under the src/ sub-directory that corresponds
to the code being tested. eg we should have src/util/virhashtest.c
alongside src/util/virhash.c So instead of one giant test dir we
have everything distributed.
That way if you are running "make" in the src/ directory, you don't
need to change dir to do tests.
I don't know what your workflow is, but I have a terminal open at
toplevel libvirt.git and then run vim over individual files, e.g.
libvirt.git $ vim src/qemu/qemu_driver.c
libvirt.git $ vim tools/virsh.c
This way I can have tags in libvirt.git and can access them from
whatever file I'm in. Running binaries is a bit more verbose:
libvirt.git $ ./src/libvirtd --listen
For tests I have a separate terminal open which entered tests:
libvirt.git/tests $
So the benefin of having virhashtest next to virhash.c highly depends on
one's workflow.
It will also let us split up the
ever growing set of makefile rules for tests.
How is that? Won't we have to have the rules to build tests in
src/Makefile.am?
Most of the rules will be in the many src/*/Makefile.in.am files
Yeah, while the overall number of rules won't change, they'll be spread
accross multiple Makefiles.
Personally, I find having code along side with the tests even more
disaranged than what we have now. For some unit tests we can have strict
naming scheme: virhash.c -> virhashtest.c; but for some more advanced tests
qemuxml2*test this won't work.
We don't need to rename everything - that could be src/qemu/qemuxml2argvtest.c
easily enough
Okay, let me send a v2 and see what others think.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/12] qemu: domain: Rename qemuDomainStorageSourceChainAccessPrepare
On Thu, Apr 18, 2019 at 04:42:59PM +0200, Peter Krempa wrote: The function will be able to deal with non-chains too so drop 'Chain' and also change the suffix to 'Modify' as it's used both for setup and teardown. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Move tools under tests/tools/
On Tue, May 07, 2019 at 12:49:18PM +0200, Michal Privoznik wrote:
> On 5/7/19 11:13 AM, Daniel P. Berrangé wrote:
> > On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
> > > There are some scripts/binaries that are not tests themselves but
> > > rather fulfill support purpose. Separate them from the rest of
> > > the tests.
> > >
> > > Signed-off-by: Michal Privoznik
> > > ---
> > > .gitignore | 2 +-
> > > Makefile.am | 2 +-
> > > cfg.mk | 4 +-
> > > configure.ac| 1 +
> > > tests/Makefile.am | 22 +-
> > > tests/qemucapabilitiestest.c| 4 +-
> > > tests/testutils.c | 2 +-
> > > tests/{ => tools}/.valgrind.supp| 0
> > > tests/tools/Makefile.am | 85 +
> > > tests/{ => tools}/check-file-access.pl | 0
> > > tests/{ => tools}/file_access_whitelist.txt | 0
> > > tests/{ => tools}/group-qemu-caps.pl| 0
> > > tests/{ => tools}/oomtrace.pl | 0
> > > tests/{ => tools}/qemucapsprobe.c | 0
> > > tests/{ => tools}/qemucapsprobemock.c | 0
> > > tests/{ => tools}/test-wrap-argv.pl | 2 +-
> > > 16 files changed, 98 insertions(+), 26 deletions(-)
> > > rename tests/{ => tools}/.valgrind.supp (100%)
> > > create mode 100644 tests/tools/Makefile.am
> > > rename tests/{ => tools}/check-file-access.pl (100%)
> > > rename tests/{ => tools}/file_access_whitelist.txt (100%)
> > > rename tests/{ => tools}/group-qemu-caps.pl (100%)
> > > rename tests/{ => tools}/oomtrace.pl (100%)
> > > rename tests/{ => tools}/qemucapsprobe.c (100%)
> > > rename tests/{ => tools}/qemucapsprobemock.c (100%)
> > > rename tests/{ => tools}/test-wrap-argv.pl (98%)
> >
> > If we're going to re-arrange our tests, then I would really like to
> > see the goal be to remove the top level "tests/" directory entirely.
> >
> > Instead move the tests under the src/ sub-directory that corresponds
> > to the code being tested. eg we should have src/util/virhashtest.c
> > alongside src/util/virhash.c So instead of one giant test dir we
> > have everything distributed.
> >
> > That way if you are running "make" in the src/ directory, you don't
> > need to change dir to do tests. It will also let us split up the
> > ever growing set of makefile rules for tests.
>
> How is that? Won't we have to have the rules to build tests in
> src/Makefile.am?
Most of the rules will be in the many src/*/Makefile.in.am files
> Personally, I find having code along side with the tests even more
> disaranged than what we have now. For some unit tests we can have strict
> naming scheme: virhash.c -> virhashtest.c; but for some more advanced tests
> qemuxml2*test this won't work.
We don't need to rename everything - that could be src/qemu/qemuxml2argvtest.c
easily enough
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Move tools under tests/tools/
On 5/7/19 11:13 AM, Daniel P. Berrangé wrote:
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
There are some scripts/binaries that are not tests themselves but
rather fulfill support purpose. Separate them from the rest of
the tests.
Signed-off-by: Michal Privoznik
---
.gitignore | 2 +-
Makefile.am | 2 +-
cfg.mk | 4 +-
configure.ac| 1 +
tests/Makefile.am | 22 +-
tests/qemucapabilitiestest.c| 4 +-
tests/testutils.c | 2 +-
tests/{ => tools}/.valgrind.supp| 0
tests/tools/Makefile.am | 85 +
tests/{ => tools}/check-file-access.pl | 0
tests/{ => tools}/file_access_whitelist.txt | 0
tests/{ => tools}/group-qemu-caps.pl| 0
tests/{ => tools}/oomtrace.pl | 0
tests/{ => tools}/qemucapsprobe.c | 0
tests/{ => tools}/qemucapsprobemock.c | 0
tests/{ => tools}/test-wrap-argv.pl | 2 +-
16 files changed, 98 insertions(+), 26 deletions(-)
rename tests/{ => tools}/.valgrind.supp (100%)
create mode 100644 tests/tools/Makefile.am
rename tests/{ => tools}/check-file-access.pl (100%)
rename tests/{ => tools}/file_access_whitelist.txt (100%)
rename tests/{ => tools}/group-qemu-caps.pl (100%)
rename tests/{ => tools}/oomtrace.pl (100%)
rename tests/{ => tools}/qemucapsprobe.c (100%)
rename tests/{ => tools}/qemucapsprobemock.c (100%)
rename tests/{ => tools}/test-wrap-argv.pl (98%)
If we're going to re-arrange our tests, then I would really like to
see the goal be to remove the top level "tests/" directory entirely.
Instead move the tests under the src/ sub-directory that corresponds
to the code being tested. eg we should have src/util/virhashtest.c
alongside src/util/virhash.c So instead of one giant test dir we
have everything distributed.
That way if you are running "make" in the src/ directory, you don't
need to change dir to do tests. It will also let us split up the
ever growing set of makefile rules for tests.
How is that? Won't we have to have the rules to build tests in
src/Makefile.am?
Personally, I find having code along side with the tests even more
disaranged than what we have now. For some unit tests we can have strict
naming scheme: virhash.c -> virhashtest.c; but for some more advanced
tests qemuxml2*test this won't work.
BTW: I've found a way to have Makefile.inc.am and have it working so
I'll post a v2 shortly.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] network: Delay creating private chains until starting network
On Tue, Apr 30, 2019 at 02:34:44PM -0600, Jim Fehlig wrote:
> Automated performance tests found that network-centric workloads suffered
> a 20 percent decrease when the host libvirt was updated from 5.0.0 to
> 5.1.0. On the test hosts libvirtd is enabled to start at boot and the
> "default" network is defined, but it is not set to autostart.
>
> libvirt 5.1.0 introduced private firewall chains with commit 5f1e6a7d.
> The chains are created at libvirtd start, which triggers loading the
> conntrack kernel module. Subsequent tracking and processing of
> connections resulted in the performance hit. Prior to commit 5f1e6a7d
> the conntrack module would not be loaded until starting a network,
> when libvirt added rules to the builtin chains.
>
> Restore the behavior of previous libvirt versions by delaying
> the creation of private chains until the first network is started.
>
> Signed-off-by: Jim Fehlig
> ---
>
> I briefly discussed this issue with Daniel on IRC and just now finding
> time to bring it to the list for broader discussion. The adjustment to
> the test file feels hacky. The whole approach might by hacky, hence the
> RFC.
The test file hackyness is something we had before, so not a big
deal imho.
>
> src/network/bridge_driver_linux.c | 64 +++---
> .../nat-default-linux.args| 116 ++
> 2 files changed, 131 insertions(+), 49 deletions(-)
>
> diff --git a/src/network/bridge_driver_linux.c
> b/src/network/bridge_driver_linux.c
> index f2827543ca..a3a09caeba 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -35,44 +35,10 @@ VIR_LOG_INIT("network.bridge_driver_linux");
>
> #define PROC_NET_ROUTE "/proc/net/route"
>
> -static virErrorPtr errInitV4;
> -static virErrorPtr errInitV6;
> +static bool pvtChainsCreated;
>
> void networkPreReloadFirewallRules(bool startup)
> {
> -bool created = false;
> -int rc;
> -
> -/* We create global rules upfront as we don't want
> - * the perf hit of conditionally figuring out whether
> - * to create them each time a network is started.
> - *
> - * Any errors here are saved to be reported at time
> - * of starting the network though as that makes them
> - * more likely to be seen by a human
> - */
> -rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
> -if (rc < 0) {
> -errInitV4 = virSaveLastError();
> -virResetLastError();
> -} else {
> -virFreeError(errInitV4);
> -errInitV4 = NULL;
> -}
> -if (rc)
> -created = true;
> -
> -rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6);
> -if (rc < 0) {
> -errInitV6 = virSaveLastError();
> -virResetLastError();
> -} else {
> -virFreeError(errInitV6);
> -errInitV6 = NULL;
> -}
> -if (rc)
> -created = true;
> -
> /*
> * If this is initial startup, and we just created the
> * top level private chains we either
> @@ -86,8 +52,8 @@ void networkPreReloadFirewallRules(bool startup)
> * rules will be present. Thus we can safely just tell it
> * to always delete from the builin chain
> */
> -if (startup && created)
> -iptablesSetDeletePrivate(false);
> +if (startup)
> +iptablesSetDeletePrivate(true);
This is problematic. It means that when upgrading libvirt we will
never delete the legacy rules from the built-in chains.
We needed to create the chains upfront, so that when we recreate
rules for existing running networks, we'll upgrade them to use the
libvirt chains instead of built-in chains.
So I think we'll need to keep the code to create libvirt chains
in this networkPreReloadFirewallRules, but *only* run it if we
have existing virtual networks that are active.
That way when libvirt starts on fresh boot, chains will be crated
only when a network is started. If libvirt is upgraded on running
host, then we'll still do thoings early.
> }
>
>
> @@ -701,19 +667,19 @@ int networkAddFirewallRules(virNetworkDefPtr def)
> virFirewallPtr fw = NULL;
> int ret = -1;
>
> -if (errInitV4 &&
> -(virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
> - virNetworkDefGetRouteByIndex(def, AF_INET, 0))) {
> -virSetError(errInitV4);
> -return -1;
> -}
> +if (!pvtChainsCreated) {
> +if (iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4) < 0 &&
> +(virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
> + virNetworkDefGetRouteByIndex(def, AF_INET, 0)))
> +return -1;
>
> -if (errInitV6 &&
> -(virNetworkDefGetIPByIndex(def, AF_INET6, 0) ||
> - virNetworkDefGetRouteByIndex(def, AF_INET6, 0) ||
> - def->ipv6nogw)) {
> -virSetError(errInitV6);
> -return -1;
> +if (iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6) < 0 &&
> +(virNetworkDefGetIPByIndex(def, AF_INET6,
[libvirt] [PATCH] spec: Bump minimum supported Fedora version to 29
Fedora 30 is out, which means that Fedora 28 is going to be
EOL very soon. Let's get ahead of the game and drop support
for it right now.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 2 +-
mingw-libvirt.spec.in | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 876b2f0897..e07041c0b9 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -4,7 +4,7 @@
# that's still supported by the vendor. It may work on other distros
# or versions, but no effort will be made to ensure that going forward.
%define min_rhel 7
-%define min_fedora 28
+%define min_fedora 29
%if (0%{?fedora} && 0%{?fedora} >= %{min_fedora}) || (0%{?rhel} && 0%{?rhel}
>= %{min_rhel})
%define supported_platform 1
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 8a96ea914c..9add033669 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -3,7 +3,7 @@
# This spec file assumes you are building on a Fedora version
# that's still supported by the vendor. It may work on other distros
# or versions, but no effort will be made to ensure that going forward.
-%define min_fedora 28
+%define min_fedora 29
%if 0%{?fedora} && 0%{?fedora} >= %{min_fedora}
%define supported_platform 1
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6] tests: perform cross compiler builds on GitLab CI
On Tue, 2019-04-16 at 11:19 +0100, Daniel P. Berrangé wrote: > +.job_template: &job_definition > + script: > +- mkdir vpath > +- cd vpath We use build/ for CentOS CI and Travis CI builds, so let's stick with that here as well. > +- ../autogen.sh $CONFIGURE_OPTS We have some code to show the contents of config.log when this steps fails in other CI environments, would it be a good idea to have it here as well? Everything else looks reasonable, at least based on my extremely shallow knowledge of GitLab CI, so as far as I'm concerned we can just merge this and fix whatever issue might pop up later. Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] QMP; unsigned 64-bit ints; JSON standards compliance
On Tue, May 07, 2019 at 10:47:06AM +0200, Markus Armbruster wrote: > > The Golang JSON parser decodes JSON numbers to float64 by default so > > will have this precision limitation too, though at least they provide > > a backdoor for custom parsing from the original serialized representation. > > > >> > QEMU, and indeed many applications, want to handle 64-bit integers. > >> > The C JSON library impls have traditionally mapped integers to the > >> > data type 'long long int' which gives a min/max of -(2^63) / 2^63-1. > >> > > >> > QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1. > > Correct. > > Support for integers 2^63..2^64-1 is relatively recent: commit > 2bc7cfea095 (v2.10, 2017). > > Since we really needed these, the QObject input visitor silently casts > negative integers to uint64_t. It still does for backward > compatibility. Commit 5923f85fb82 (right after 2bc7cfea095) explains > > The input visitor will cast i64 input to u64 for compatibility > reasons (existing json QMP client already use negative i64 for large > u64, and expect an implicit cast in qemu). > > Note: before the patch, uint64_t values above INT64_MAX are sent over > json QMP as negative values, e.g. UINT64_MAX is sent as -1. After the > patch, they are sent unmodified. Clearly a bug fix, but we have to > consider compatibility issues anyway. libvirt should cope fine, > because its parsing of unsigned integers accepts negative values > modulo 2^64. There's hope that other clients will, too. So QEMU reading stuff sent by libvirt in a back compatible manner is ok. The problem was specifically when a QEMU reply sent back UINT64_MAX value as a positive number. > >> > Libvirt has historically used the YAJL library which uses 'long long int' > >> > and thus can't officially go beyond 2^63-1 values. Fortunately it lets > >> > libvirt get at the raw json string, so libvirt can re-parse the value > >> > to get an 'unsigned long long'. > >> > > >> > We recently tried to switch to Jansson because YAJL has a dead upstream > >> > for many years and countless unanswered bugs & patches. Unfortunately we > >> > forgot about this need for 2^64-1 max, and Jansson also uses 'long long > >> > int' > >> > and raises a fatal parse error for unsigned 64-bit values above 2^63-1. > >> > It > >> > also provides no backdoor for libvirt todo its own integer parsing. Thus > >> > we had to abort our switch to jansson as it broke parsing QEMU's JSON: > >> > > >> > https://bugzilla.redhat.com/show_bug.cgi?id=1614569 > >> > > >> > Other JSON libraries we've investigated have similar problems. I imagine > >> > the same may well be true of non-C based JOSN impls, though I've not > >> > investigated in any detail. > >> > > >> > Essentially libvirt is stuck with either using the dead YAJL library > >> > forever, or writing its own JSON parser (most likely copying QEMU's > >> > JSON code into libvirt's git). > >> > > >> > This feels like a very unappealing situation to be in as not being > >> > able to use a JSON library of our choice is loosing one of the key > >> > benefits of using a standard data format. > >> > > >> > Thus I'd like to see a solution to this to allow QMP to be reliably > >> > consumed by any JSON library that exists. > > JSON is terrible at interoperability, so good luck with that. > > If you reduce your order to "the commonly used JSON libraries we know", > we can talk. I don't particularly want us to rely on semantics of small known set of JSON libs. I really do want us to do something that is capable of working with any JSON impl that exists in any programming language. My suggested option 2 & 3 at least would manage that I believe, as any credible JSON impl will be able to represent 32-bit integers or strings without loosing data. Option 1 would not cope as some impls can't even cope with signed 64-bit ints. > >> > I can think of some options: > >> > > >> > 1. Encode unsigned 64-bit integers as signed 64-bit integers. > >> > > >> > This follows the example that most C libraries map JSON ints > >> > to 'long long int'. This is still relying on undefined > >> > behaviour as apps don't need to support > 2^53-1. > >> > > >> > Apps would need to cast back to 'unsigned long long' for > >> > those QMP fields they know are supposed to be unsigned. > > Ugly. It's also what we did until v2.10, August 2017. QMP's input > direction still does it, for backward compatibility. > > >> > > >> > > >> > 2. Encode all 64-bit integers as a pair of 32-bit integers. > >> > > >> > This is fully compliant with the JSON spec as each half > >> > is fully within the declared limits. App has to split or > >> > assemble the 2 pieces from/to a signed/unsigned 64-bit > >> > int as needed. > > Differently ugly. > > >> > > >> > > >> > 3. Encode all 64-bit integers as strings > >> > > >> > The application has todo
Re: [libvirt] [PATCH v2 2/2] drm/i915/gvt: export mdev device version to sysfs for Intel vGPU
On Sun, 5 May 2019 21:51:02 -0400
Yan Zhao wrote:
> This feature implements the version attribute for Intel's vGPU mdev
> devices.
>
> version attribute is rw.
> It's used to check device compatibility for two mdev devices.
> version string format and length are private for vendor driver. vendor
> driver is able to define them freely.
>
> For Intel vGPU of gen8 and gen9, the mdev device version
> consists of 3 fields: "vendor id" + "device id" + "mdev type".
>
> Reading from a vGPU's version attribute, a string is returned in below
> format: --. e.g.
> 8086-193b-i915-GVTg_V5_2.
>
> Writing a string to a vGPU's version attribute will trigger GVT to check
> whether a vGPU identified by the written string is compatible with
> current vGPU owning this version attribute. errno is returned if the two
> vGPUs are incompatible. The length of written string is returned in
> compatible case.
>
> For other platforms, and for GVT not supporting vGPU live migration
> feature, errnos are returned when read/write of mdev devices' version
> attributes.
>
> For old GVT versions where no version attributes exposed in sysfs, it is
> regarded as not supporting vGPU live migration.
>
> For future platforms, besides the current 2 fields in vendor proprietary
> part, more fields may be added to identify Intel vGPU well for live
> migration purpose.
>
> v2:
> 1. removed 32 common part of version string
> (Alex Williamson)
> 2. do not register version attribute for GVT not supporting live
> migration.(Cornelia Huck)
> 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> incompatible. (Cornelia Huck)
Should go below '---'.
>
> Cc: Alex Williamson
> Cc: Erik Skultety
> Cc: "Dr. David Alan Gilbert"
> Cc: Cornelia Huck
> Cc: "Tian, Kevin"
> Cc: Zhenyu Wang
> Cc: "Wang, Zhi A"
> c: Neo Jia
> Cc: Kirti Wankhede
>
> Signed-off-by: Yan Zhao
> ---
> drivers/gpu/drm/i915/gvt/Makefile | 2 +-
> drivers/gpu/drm/i915/gvt/device_version.c | 87 +++
> drivers/gpu/drm/i915/gvt/gvt.c| 51 +
> drivers/gpu/drm/i915/gvt/gvt.h| 6 ++
> 4 files changed, 145 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
>
(...)
> diff --git a/drivers/gpu/drm/i915/gvt/device_version.c
> b/drivers/gpu/drm/i915/gvt/device_version.c
> new file mode 100644
> index ..bd4cdcbdba95
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> THE
> + * SOFTWARE.
> + *
> + * Authors:
> + *Yan Zhao
> + */
> +#include
> +#include "i915_drv.h"
> +
> +static bool is_compatible(const char *self, const char *remote)
> +{
> + if (strlen(remote) != strlen(self))
> + return false;
> +
> + return (strncmp(self, remote, strlen(self))) ? false : true;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private
> *dev_priv)
> +{
> + if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> + return -ENODEV;
> +
> + return PAGE_SIZE;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
> + char *buf, const char *mdev_type)
> +{
> + int cnt = 0, ret = 0;
> + const char *str = NULL;
> +
> + /* currently only gen8 & gen9 are supported */
> + if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> + return -ENODEV;
> +
> + /* vendor id + device id + mdev type */
> + /* vendor id */
> + cnt = snprintf(buf, 5, "%04x", PCI_VENDOR_ID_INTEL);
> + buf += cnt;
> + ret += cnt;
> +
> + /* device id */
> + cnt = snprintf(buf, 6, "-%04x", INTEL_DEVID(dev_priv));
> + buf += cnt;
> + ret += cnt;
> +
> + /* mdev type */
> + str = mdev_type;
Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
On Sun, 5 May 2019 21:49:04 -0400 Yan Zhao wrote: > version attribute is used to check two mdev devices' compatibility. > > The key point of this version attribute is that it's rw. > User space has no need to understand internal of device version and no > need to compare versions by itself. > Compared to reading version strings from both two mdev devices being > checked, user space only reads from one mdev device's version attribute. > After getting its version string, user space writes this string into the > other mdev device's version attribute. Vendor driver of mdev device > whose version attribute being written will check device compatibility of > the two mdev devices for user space and return success for compatibility > or errno for incompatibility. I'm still missing a bit _what_ is actually supposed to be compatible/incompatible. I'd assume some internal state descriptions (even if this is not actually limited to migration). > So two readings of version attributes + checking in user space are now > changed to one reading + one writing of version attributes + checking in > vendor driver. I'm not sure that needs to go into the patch description (sounds like it is rather a change log?) > Format and length of version strings are now private to vendor driver > who can define them freely. Same here; simply drop the 'now'? > > __ user space > /\ \ > / \write > / read \ > __/__ ___\|/___ > | version | | version |-->check compatibility > --- --- > mdev device A mdev device B > > This version attribute is optional. If a mdev device does not provide > with a version attribute, this mdev device is incompatible to all other > mdev devices. Again, I'd like an explanation here what kind of compatibility we're talking about. > > Live migration is able to take advantage of this version attribute. > Before user space actually starts live migration, it can first check > whether two mdev devices are compatible. > > v2: > 1. added detailed intent and usage > 2. made definition of version string completely private to vendor driver >(Alex Williamson) > 3. abandoned changes to sample mdev drivers (Alex Williamson) > 4. mandatory --> optional (Cornelia Huck) > 5. added description for errno (Cornelia Huck) This changelog should go below the ---, so that it does not actually show up in the patch description later :) > > Cc: Alex Williamson > Cc: Erik Skultety > Cc: "Dr. David Alan Gilbert" > Cc: Cornelia Huck > Cc: "Tian, Kevin" > Cc: Zhenyu Wang > Cc: "Wang, Zhi A" > Cc: Neo Jia > Cc: Kirti Wankhede > Cc: Daniel P. Berrangé > Cc: Christophe de Dinechin > > Signed-off-by: Yan Zhao > --- > Documentation/vfio-mediated-device.txt | 140 + > 1 file changed, 140 insertions(+) > > diff --git a/Documentation/vfio-mediated-device.txt > b/Documentation/vfio-mediated-device.txt > index c3f69bcaf96e..013a764968eb 100644 > --- a/Documentation/vfio-mediated-device.txt > +++ b/Documentation/vfio-mediated-device.txt > @@ -202,6 +202,7 @@ Directories and files under the sysfs for Each Physical > Device >| | |--- available_instances >| | |--- device_api >| | |--- description > + | | |--- version >| | |--- [devices] >| |--- [] >| | |--- create > @@ -209,6 +210,7 @@ Directories and files under the sysfs for Each Physical > Device >| | |--- available_instances >| | |--- device_api >| | |--- description > + | | |--- version >| | |--- [devices] >| |--- [] >| |--- create > @@ -216,6 +218,7 @@ Directories and files under the sysfs for Each Physical > Device >| |--- available_instances >| |--- device_api >| |--- description > + | |--- version >| |--- [devices] > > * [mdev_supported_types] > @@ -246,6 +249,143 @@ Directories and files under the sysfs for Each Physical > Device >This attribute should show the number of devices of type that > can be >created. > > +* version > + > + This attribute is rw, and is optional. > + It is used to check device compatibility between two mdev devices and is > + accessed in pairs between the two mdev devices being checked. > + The intent of this attribute is to make an mdev device's version opaque to > + user space, so instead of reading two mdev devices' version strings and > + comparing in userspace, user space should only read one mdev device's > version > + attribute, and writes this version string into the other mdev device's > version > + attribute. Then vendor driver of mdev device whose version attribute being > + written would check the incoming version string and tell user space whether > + the two mdev devices are compatible via return value. That's why this > + attribute
Re: [libvirt] [PATCH] tests: Move tools under tests/tools/
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
> There are some scripts/binaries that are not tests themselves but
> rather fulfill support purpose. Separate them from the rest of
> the tests.
>
> Signed-off-by: Michal Privoznik
> ---
> .gitignore | 2 +-
> Makefile.am | 2 +-
> cfg.mk | 4 +-
> configure.ac| 1 +
> tests/Makefile.am | 22 +-
> tests/qemucapabilitiestest.c| 4 +-
> tests/testutils.c | 2 +-
> tests/{ => tools}/.valgrind.supp| 0
> tests/tools/Makefile.am | 85 +
> tests/{ => tools}/check-file-access.pl | 0
> tests/{ => tools}/file_access_whitelist.txt | 0
> tests/{ => tools}/group-qemu-caps.pl| 0
> tests/{ => tools}/oomtrace.pl | 0
> tests/{ => tools}/qemucapsprobe.c | 0
> tests/{ => tools}/qemucapsprobemock.c | 0
> tests/{ => tools}/test-wrap-argv.pl | 2 +-
> 16 files changed, 98 insertions(+), 26 deletions(-)
> rename tests/{ => tools}/.valgrind.supp (100%)
> create mode 100644 tests/tools/Makefile.am
> rename tests/{ => tools}/check-file-access.pl (100%)
> rename tests/{ => tools}/file_access_whitelist.txt (100%)
> rename tests/{ => tools}/group-qemu-caps.pl (100%)
> rename tests/{ => tools}/oomtrace.pl (100%)
> rename tests/{ => tools}/qemucapsprobe.c (100%)
> rename tests/{ => tools}/qemucapsprobemock.c (100%)
> rename tests/{ => tools}/test-wrap-argv.pl (98%)
If we're going to re-arrange our tests, then I would really like to
see the goal be to remove the top level "tests/" directory entirely.
Instead move the tests under the src/ sub-directory that corresponds
to the code being tested. eg we should have src/util/virhashtest.c
alongside src/util/virhash.c So instead of one giant test dir we
have everything distributed.
That way if you are running "make" in the src/ directory, you don't
need to change dir to do tests. It will also let us split up the
ever growing set of makefile rules for tests.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] x86: Multi-key Total Memory Encryption (Intel)
On Mon, May 06, 2019 at 08:56:55PM +, Carvalho, Larkins L wrote: > Hello Peter, > > Thank you for the feedback. > I'll submit another patch series. > > I have a question regarding the test cap replies file. > We are querying Qemu to get the capability information. > The Qemu patch is not yet upstream, hence, we are not able to create > the replies file for the same. Either leave out the replies file, or just apply your MKTME patches to upstream QEMU in your local checkout & generate replies file from that. Then you can update the patch with new replies file once the QEMU pieces finally merge. > Is it ok to submit the patch for the feedback without the replies file? > Since the Qemu support for MKTME is not yet upstream and without Qemu > support we cannot merge this changes to Libvirt. Libvirt won't merge the patches until the corresponding QEMU piece is merged, but we have happy to review patches at any time. Indeed it is *good* to have libvirt patches reviewed before QEMU stuff is merged, as that makes it easier if we spot things that can be improved in QEMU for help libvirt. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] vfio/mdev: add version field as mandatory attribute for mdev device
On Tue, 7 May 2019 01:39:13 -0400 Yan Zhao wrote: > On Tue, Apr 30, 2019 at 11:29:08PM +0800, Cornelia Huck wrote: > > If I followed the discussion correctly, I think you plan to drop this > > format, don't you? I'd be happy if a vendor driver can use a simple > > number without any prefixes if it so chooses. > > > > I also like the idea of renaming this "migration_version" so that it is > > clear we're dealing with versioning of the migration capability (and > > not a version of the device or so). > hi Cornelia, > sorry I just saw this mail after sending v2 of this patch set... > yes, I dropped the common part and vendor driver now can define whatever it > wishes to identify a device version. Ok, I'll look at v2. > However, I don't agree to rename it to "migration_version", as it still may > bring some kind of confusing with the migration version a vendor driver is > using, e.g. vendor driver changes migration code and increases that migration > version. > In fact, what info we want to get from this attribute is whether this mdev > device is compatible with another mdev device, which is tied to device, and > not > necessarily bound to migration. > > do you think so? I'm not 100% convinced; but we can continue the discussion on v2. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] QMP; unsigned 64-bit ints; JSON standards compliance
Daniel P. Berrangé writes: > On Tue, Apr 30, 2019 at 03:45:46PM +0100, Dr. David Alan Gilbert wrote: >> * Daniel P. Berrangé (berra...@redhat.com) wrote: >> > The QEMU QMP service is based on JSON which is nice because that is a >> > widely supported "standard" data format. >> > >> > except QEMU's implementation (and indeed most impls) are not strictly >> > standards compliant. As we'll see below, speaking of "strict standard compliance" is meaningless, because the standard is too weak. What you mean is "lowest common denominator of relevant implementations". >> > Specifically the problem is around representing 64-bit integers, whether >> > signed or unsigned. >> > >> > The JSON standard declares that largest integer is 2^53-1 and the >> > likewise the smallest is -(2^53-1): >> > >> > >> > http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer This is the ECMAScript standard, not the JSON standard. For JSON, the situation is more confused. See below. >> > A crazy limit inherited from its javascript origins IIUC. >> >> E. > > Looking a bit deeper it seems this limit comes from the use of double > precision floating point for storing integers. 2^53-1 is the largest > integer value that can be stored in a 64-bit float without loss of > precision. Correct. JavaScript has only one type of number, and they're all represented as double precision floating-point. Perfectly sensible for a prototype done in ten days. Move fast and leave things broken forever. JSON is derived from JavaScript. RFC 4672 is notoriously useless on precision and range. RFC 7159 attempts to contain the damage by spelling out how useless it: This specification allows implementations to set limits on the range and precision of numbers accepted. Since software that implements IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is generally available and widely used, good interoperability can be achieved by implementations that expect no more precision or range than these provide, in the sense that implementations will approximate JSON numbers within the expected precision. A JSON number such as 1E400 or 3.141592653589793238462643383279 may indicate potential interoperability problems, since it suggests that the software that created it expects receiving software to have greater capabilities for numeric magnitude and precision than is widely available. Note that when such software is used, numbers that are integers and are in the range [-(2**53)+1, (2**53)-1] are interoperable in the sense that implementations will agree exactly on their numeric values. RFC 8259 keeps this unchanged. QEMU does conform to RFC 8259. It sets limits on range and precision of numbers accepted: double precision \union the integers between INT64_MIN and UINT64_MAX inclusive. The trouble is that conformance doesn't imply interoperability. Many other implementations follow JavaScript and limit numbers to double. > The Golang JSON parser decodes JSON numbers to float64 by default so > will have this precision limitation too, though at least they provide > a backdoor for custom parsing from the original serialized representation. > >> > QEMU, and indeed many applications, want to handle 64-bit integers. >> > The C JSON library impls have traditionally mapped integers to the >> > data type 'long long int' which gives a min/max of -(2^63) / 2^63-1. >> > >> > QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1. Correct. Support for integers 2^63..2^64-1 is relatively recent: commit 2bc7cfea095 (v2.10, 2017). Since we really needed these, the QObject input visitor silently casts negative integers to uint64_t. It still does for backward compatibility. Commit 5923f85fb82 (right after 2bc7cfea095) explains The input visitor will cast i64 input to u64 for compatibility reasons (existing json QMP client already use negative i64 for large u64, and expect an implicit cast in qemu). Note: before the patch, uint64_t values above INT64_MAX are sent over json QMP as negative values, e.g. UINT64_MAX is sent as -1. After the patch, they are sent unmodified. Clearly a bug fix, but we have to consider compatibility issues anyway. libvirt should cope fine, because its parsing of unsigned integers accepts negative values modulo 2^64. There's hope that other clients will, too. >> > Libvirt has historically used the YAJL library which uses 'long long int' >> > and thus can't officially go beyond 2^63-1 values. Fortunately it lets >> > libvirt get at the raw json string, so libvirt can re-parse the value >> > to get an 'unsigned long long'. >> > >> > We recently tried to switch to Jansson because YAJL has a dead upstream >> > for many years and countless unanswered bugs & patches. Unfortunately we >> > forgot about this need for 2^64-1 max, and Jansson also uses 'long long >> >
Re: [libvirt] [PATCH 2/3] virtestmock: Initialize symbols from stat() and its friends
On Mon, May 06, 2019 at 04:34:07PM +0200, Michal Privoznik wrote:
Introduced by ff376c6283c97.
Previously, init_syms() was called from stat() mock and its
friends. This is crucial because checkPath() might call
printFile() which in turn calls real_fopen(). But if stat() or
one of its friends is the first function called then because of
lacking init_syms() call no real_* is initialized.
The other thing is that we really want the recorded action to be
"stat" instead of __FUNCTION__ because there's no good in
recording that it was __xstat64 who touched some file.
Signed-off-by: Michal Privoznik
---
tests/virtestmock.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/virtestmock.c b/tests/virtestmock.c
index bc62312444..ad6958ac87 100644
--- a/tests/virtestmock.c
+++ b/tests/virtestmock.c
@@ -190,7 +190,9 @@ int access(const char *path, int mode)
}
-#define VIR_MOCK_STAT_HOOK CHECK_PATH(path)
+#define VIR_MOCK_STAT_HOOK \
+init_syms(); \
+checkPath(path, "stat")
Just for the cleanliness of things, I would prefer the macro to be written as a
single block:
do {
init_syms(); \
checkPath(path, "stat")
while (0)
even when it is not used like that (at least not yet).
ACK either way.
#include "virmockstathelpers.c"
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] lib: Build sources before running 'check-access'
ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virfilemock: Init symbols in canonicalize_file_name()
ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] network: Delay creating private chains until starting network
On 4/30/19 10:34 PM, Jim Fehlig wrote: Automated performance tests found that network-centric workloads suffered a 20 percent decrease when the host libvirt was updated from 5.0.0 to 5.1.0. On the test hosts libvirtd is enabled to start at boot and the "default" network is defined, but it is not set to autostart. libvirt 5.1.0 introduced private firewall chains with commit 5f1e6a7d. The chains are created at libvirtd start, which triggers loading the conntrack kernel module. Subsequent tracking and processing of connections resulted in the performance hit. Prior to commit 5f1e6a7d the conntrack module would not be loaded until starting a network, when libvirt added rules to the builtin chains. Restore the behavior of previous libvirt versions by delaying the creation of private chains until the first network is started. Signed-off-by: Jim Fehlig --- I briefly discussed this issue with Daniel on IRC and just now finding time to bring it to the list for broader discussion. The adjustment to the test file feels hacky. The whole approach might by hacky, hence the RFC. src/network/bridge_driver_linux.c | 64 +++--- .../nat-default-linux.args| 116 ++ 2 files changed, 131 insertions(+), 49 deletions(-) I like this. I was under impression that these rules are created if and only if there's a NATed network found at startup when libvirt is reading the config files. But that doesn't seem to be the case - we create the rules even if all users have is one network with . You have my +1. I'm not going give explicit ACK (at least for now) until Dan speaks his mind. He had some opinions and good ideas when I tried to fix a problem in the same area of the code. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] tests: domaincaps: Add QEMU 4.0.0 for s390x
On 5/6/19 2:42 PM, Andrea Bolognani wrote:
On Fri, 2019-05-03 at 10:47 +0200, Boris Fiuczynski wrote:
[...]
+++ b/tests/domaincapstest.c
@@ -453,6 +453,10 @@ mymain(void)
DO_TEST_QEMU("4.0.0", "caps_4.0.0",
"/usr/bin/qemu-system-x86_64", NULL,
"x86_64", VIR_DOMAIN_VIRT_KVM);
+
+DO_TEST_QEMU("4.0.0", "caps_4.0.0",
+ "/usr/bin/qemu-system-s390x", NULL,
+ "s390x", VIR_DOMAIN_VIRT_KVM);
It's pretty annoying that we have to update this test program
manually every time new .replies are added. Easy to miss, too, and
in fact we seem to only have coverage for about half the possible
input files.
We should modify this so that dropping .replies into the source
directory is enough for the test program to pick them up, just like
qemucapabilitiestest and qemucaps2xmltest do.
But that's work for another day, so in the meantime
Reviewed-by: Andrea Bolognani
Andrea, thanks for the reviewing, reworking and pushing.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
