Re: [libvirt-ci RFC PATCH 00/13] Introduce a new global TOML config file for lcitool
On Thu, Apr 23, 2020 at 06:46:09PM +0200, Andrea Bolognani wrote: > On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote: > > This series is trying to consolidate the number of config files we currently > > recognize under ~/.config/lcitool to a single global TOML config file. > > Thanks > > to this effort we can expose more seetings than we previously could which > > will > > come handy in terms of generating cloudinit images for OpenStack. > > > > Patches 1-4 patches are just a little extra - not heavily related to the > > series > > See patch 5/13 why TOML was selected. > > First of all, thanks for tackling this! It's something that we've > sorely needed for a while now. > > Now, I know I'm the one who suggested TOML in the first place... But > looking at the series now I can't help but think, why not YAML? O:-) To be honest, even before you originally mentioned TOML, I myself had INI in mind, so then I thought, yeah, why not go with TOML then, it's similar and more powerful. I did some comparison of several formats, because like you say, with YAML we'd be more close to Ansible and I was on the cusp of choosing between YAML and TOML and I felt like TOML was still more readable and expressive in terms of simple configuration (and that's what Linux users are IMO used to from INI). I was never a big fan of YAML really and when the dictionaries and list happen to intertwine and nest a lot, YAML looses its readability quite quickly IMO, which I never felt with TOML, but it's fair to say that my TOML experience is very limited. That said, I don't expect us to have such a massive config, so that multiple levels of YAML nesting will be necessary :). > > Since we're using it extensively already due to Ansible, I think it > would make sense to use it for the configuration file as well. It's > easy enough to consume for a human, and we wouldn't need to drag in > an additional dependency. I also believe, perhaps naively, that > adapting your code to use YAML instead of TOML wouldn't require much > work - from the Python point of view, you basically end up with a > dictionary in both cases. > > Feel free to argue against this suggestion! For example, if you agree > with it in principle but feel like it's unfair of me to ask you to > rework your code, I'm happy to port it myself. I'd still prefer TOML, but I don't really have a compelling reason to argue against YAML other than readability which I already admitted to be just a matter of taste. Now on a more serious note, I'll wait for your detailed review and then rework it in YAML in vX. > > As for the rest of the series - I've only skimmed it so far, but I > did not spot anything horribly wrong with it at first glance. I'll > provide an actual, detailed review next week. Okay, thanks. -- Erik Skultety
Re: [PATCH v2] docs: Document reserved PCI addresses for QEMU
On 4/21/20 2:00 PM, Michal Privoznik wrote: >From time to time we are asked which PCI addresses are reserved in QEMU. Let's document them in one place, it's easier than reconstructing the list from the code each time. Signed-off-by: Michal Privoznik --- diff to v1: - All Laine's comment worked in, hopefully. docs/pci-addresses.rst | 50 ++ 1 file changed, 50 insertions(+) diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst index 885d50517a..4f123c7eb9 100644 --- a/docs/pci-addresses.rst +++ b/docs/pci-addresses.rst @@ -235,3 +235,53 @@ guest OS rather than as ``0001:08:00.1``, which is the address of the device on the host. Of course, all the rules and behaviors described above still apply. + + +Reserved addresses +== + +Due to some historical reasons hypervisors might expect some PCI +devices to appear at certain addresses instead of 'random' ones. +For QEMU this is machine type and guest architecture dependant. +But to give you at least a gist here is list of reserved PCI +addresses: + +For ``I440FX`` the following devices are hard coded into QEMU and can't be +moved or eliminated: Maybe instead of just saying "I440FX", you could say "the x86_64 architecture's 'i440fx'-based machinetypes" (or something like that), both to let readers know that i440fx is a machinetype, as well as to indicate that this isn't the exact name of the machinetype, but a class of machinetypes. + + == +:00:00.0 Host bridge +:00:01.0 ISA bridge +:00:01.1 primary IDE controller +:00:01.2 PIIX3 USB controller +:00:01.3 ACPI (power management) and SMBus controller + == + +The following addresses will be used as default ones for the corresponding +devices (if the address is free or a different address wasn't provided for the +device): + + == +:00:02.0 primary video card + == Do you think we need to explicitly say "it's okay to use this address for any other device"? Or is what's said here enough. I'm undecided, fine with it as is though. + + +For ``Q35`` the following devices are hard coded into QEMU and can't be moved +or eliminated: Similar note for "q35" as for "i440fx" + + === +:00:00.0 Host bridge +:00:1f.2 primary SATA controller +:00:1f.0 ISA bridge +:00:1f.3 SMBus + === + +The following addresses will be used as default ones for the corresponding +devices (if the address is free or a different address wasn't provided for the +device) because that's how real ``Q35`` would do it: + + === +:00:1a.0 USB2 controller +:00:1b.0 ICH9 sound chip +:00:1d.0 USB2 controller + === I notice you removed the dmi-to-pci-bridge. Which is fine with me, just wanted you to know that I know that you know ;-) Reviewed-by: Laine Stump
Re: [libvirt PATCH] CONTRIBUTING: Include information on build dependencies
On 4/21/20 1:16 PM, Laine Stump wrote: On 4/20/20 6:54 AM, Andrea Bolognani wrote: libvirt depends on a ton of packages, so trying to install them all by using the classic approach of repeatedly running configure and reacting to each failure by installing the corresponding missing package will inevitably lead to frustration. Luckily there's an easy solution to get most dependencies installed in one fell swoop, and we just need to document it. Signed-off-by: Andrea Bolognani Reviewed-by: Laine Stump --- CONTRIBUTING.rst | 19 +++ 1 file changed, 19 insertions(+) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 68c7b547c6..f476700fdd 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -17,3 +17,22 @@ your git clone run: $ make You'll find the freshly-built document in ``docs/contribute.html``. + +If ``configure`` fails because of missing dependencies, you can set +up your system by calling + +:: + + $ sudo dnf builddep libvirt + +if you're on a RHEL-based distribution or + +:: + + $ sudo apt-get build-dep libvirt + +if you're on a Debian-based one. + +You might still be missing some dependencies if your distribution is +shipping an old libvirt version, but that will get you much closer to +where you need to be to build successfully from source. BTW, I just went through this on a new Fedora 31 machine (AMD Ryzen 3950x! w00t!!) that I had installed with "Fedora Workstation", and thought it might be useful to list exactly what was still missing for certain "developer build" tasks after running "dnf builddep libvirt" on a clean OS install. I found that I also had to install the following: 1) dnf install make This was required (duh!) to build from source tar, but not a part of the base OS install nor in Build-Requires (I guess it makes sense - what self-respecting Linux distro doesn't have basic build tools like make in the base install?) 2) dnf install autoconf automake libtool These three were required (but not in base OS + "dnf builddep libvirt" to do a successful "./autogen.sh --system && make check" (i.e. build from a fresh git clone of the tree). Again, I can see why autoconf, automake, and libtool wouldn't be in the specfile Build-Requires:, since they *aren't* required when building from a source tar, which already includes the configure script and Makefile.in's that are generated using autotools. 3) dnf install rpm-build This one was required (again - duh!) to run "make rpm". I guess it makes sense that it's not in the Build-Requires in some ways, since it isn't required to build the binaries, only to build the rpm file. (But on the other hand, the entire purpose of libvirt.spec is to build rpms. although, on the *other* other hand, you'd think that rpm-build would be included in any minimal build environment anyway (and apparently it *is* - e.g. the buildroot for Fedora koji). 4) dnf install cppi dwarves python3-flake8 These three were *not* required to successfully complete any build operation, but parts of "make syntax-check" were skipped because they weren't present (and so not having them might result in a developer believing that their patches had passed "make syntax-check, when in fact they had not). At least cppi and python3-flake8 can't be Build-Required: in the specfile for building because they aren't available on RHEL8/CentOS8 (at least not without EPEL - haven't checked there yet, but of course EPEL packages aren't available in the official build environment, so it's kind of irrelevant)(oh, and also make syntax-check isn't run as part of building the rpms, so those programs are never run during an official build anyway, i.e. adding them to the Build-Requires: of the specfile would be a lie :-) === Anyway, does anyone think it's worth adding a short bit to this file about these extra packages? Or should we keep this file simple and rather let newcomers (and old timers who've forgotten and are setting up a new machine) figure it out for themselves?
[PATCH v2] news: Include new DHCP network feature
This commit includes an entry for new network DHCP lease time information inside news.xml. Signed-off-by: Julio Faracco --- docs/news.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 956018b512..9c36f3bd51 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -88,6 +88,16 @@ hotplugging PCI devices. + + + Lease time option included for network DHCP settings + + + Users can now configure expiry time for leases for networks where + libvirt manages DHCP. The time can be specified for whole range + and/or fine tuned per individual host. + + -- 2.25.3
[libvirt-php] libvirt_list_storagepools() gives an error if all the storage pools on the host node are active
Hi, While using the function libvirt_list_storagepools() I found that if all the storage pools on the host node are active, the function doesn't return the list of storage pools but it gives the following error instead: names in virConnectListDefinedStoragePools must not be NULL Expected result, return an array of all the storage pools names on the host node regardless of their status (active or inactive). virsh output for the equivalent command: $ virsh pool-list --all Name State Autostart default active yes resources active yes templates active yes virtual_machines_hdd active yes virtual_machines_ssd active yes Also in the same scenario (all storage pools active) libvirt_list_inactive_storagepools() gives the same error detailed above. I wouldn't say this is actually wrong but wouldn't it be better if these functions returns FALSE instead if there are no results (-no inactive storage pools in this case-)? System information: OS: Debian Buster PHP version (fast cgi): 7.3.14 Libvirt PHP version: latest from git repository @ gitlab Libvirt version: 5.0.0 KVM host libvirt version: 5.9.0 Thanks, Fernando
Re: [PATCH] qemu: Move interlocking of blockjobs and checkpoints after liveness check
On Thu, Apr 23, 2020 at 04:13:43PM +0200, Peter Krempa wrote: > qemuDomainSupportsCheckpointsBlockjobs checks if the > QEMU_CAPS_INCREMENTAL_BACKUP capability is supported to do the > interlocking. Capabilities are not present when the VM isn't running > though which would create false errors. > > Move the checks after the liveness check in block job implementations. > > Signed-off-by: Peter Krempa Reviewed-by: Pavel Mores > --- > src/qemu/qemu_driver.c | 17 ++--- > 1 file changed, 6 insertions(+), 11 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 00d276061e..5ecf12deef 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -17439,6 +17439,9 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm, > if (virDomainObjCheckActive(vm) < 0) > goto endjob; > > +if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) > +goto endjob; > + > if (!(disk = qemuDomainDiskByName(vm->def, path))) > goto endjob; > > @@ -17994,6 +17997,9 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, > if (virDomainObjCheckActive(vm) < 0) > goto endjob; > > +if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) > +goto endjob; > + > if (!(disk = qemuDomainDiskByName(vm->def, path))) > goto endjob; > > @@ -18278,9 +18284,6 @@ qemuDomainBlockRebase(virDomainPtr dom, const char > *path, const char *base, > if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0) > goto cleanup; > > -if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) > -goto cleanup; > - > /* For normal rebase (enhanced blockpull), the common code handles > * everything, including vm cleanup. */ > if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY)) > @@ -18364,9 +18367,6 @@ qemuDomainBlockCopy(virDomainPtr dom, const char > *disk, const char *destxml, > if (virDomainBlockCopyEnsureACL(dom->conn, vm->def) < 0) > goto cleanup; > > -if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) > -goto cleanup; > - > for (i = 0; i < nparams; i++) { > virTypedParameterPtr param = ¶ms[i]; > > @@ -18429,11 +18429,6 @@ qemuDomainBlockPull(virDomainPtr dom, const char > *path, unsigned long bandwidth, > return -1; > } > > -if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) { > -virDomainObjEndAPI(&vm); > -return -1; > -} > - > /* qemuDomainBlockPullCommon consumes the reference on @vm */ > return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags); > } > -- > 2.26.0 >
Re: [libvirt-ci RFC PATCH 00/13] Introduce a new global TOML config file for lcitool
On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote: > This series is trying to consolidate the number of config files we currently > recognize under ~/.config/lcitool to a single global TOML config file. Thanks > to this effort we can expose more seetings than we previously could which will > come handy in terms of generating cloudinit images for OpenStack. > > Patches 1-4 patches are just a little extra - not heavily related to the > series > See patch 5/13 why TOML was selected. First of all, thanks for tackling this! It's something that we've sorely needed for a while now. Now, I know I'm the one who suggested TOML in the first place... But looking at the series now I can't help but think, why not YAML? O:-) Since we're using it extensively already due to Ansible, I think it would make sense to use it for the configuration file as well. It's easy enough to consume for a human, and we wouldn't need to drag in an additional dependency. I also believe, perhaps naively, that adapting your code to use YAML instead of TOML wouldn't require much work - from the Python point of view, you basically end up with a dictionary in both cases. Feel free to argue against this suggestion! For example, if you agree with it in principle but feel like it's unfair of me to ask you to rework your code, I'm happy to port it myself. As for the rest of the series - I've only skimmed it so far, but I did not spot anything horribly wrong with it at first glance. I'll provide an actual, detailed review next week. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 5/5] backup: Store error message for failed backups
On 4/16/20 4:55 AM, Peter Krempa wrote: If a backup job fails midway it's hard to figure out what happened as it's running asynchronous. Use the VIR_DOMAIN_JOB_ERRMSG job statistics field to pass through the error from the first failed backup-blockjob so that both the consumer of the virDomainGetJobStats and the corresponding event can see the error. event 'job-completed' for domain backup-test: operation: 9 time_elapsed: 46 disk_total: 104857600 disk_processed: 10158080 disk_remaining: 94699520 success: 0 errmsg: No space left on device Nice. virsh domjobinfo backup-test --completed --anystats Job type: Failed Operation:Backup Time elapsed: 46 ms File processed: 9.688 MiB File remaining: 90.312 MiB File total: 100.000 MiB Error message:No space left on device https://bugzilla.redhat.com/show_bug.cgi?id=1812827 Signed-off-by: Peter Krempa --- Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 4/5] qemu: domain: Add 'errmsg' field to qemuDomainJobInfo
On 4/16/20 4:55 AM, Peter Krempa wrote: The field can be used by jobs to add an optional error message to a completed (failed) job. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_domain.h | 2 ++ 2 files changed, 5 insertions(+) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 3/5] API: Add VIR_DOMAIN_JOB_ERRMSG domain job statistics field
On 4/16/20 4:55 AM, Peter Krempa wrote: In some cases it's useful to report the error which caused the domain job to fail. Add an optional field for holding the error message so that it can be later retrieved from statistics of a completed job. Add the field name macro and code for extracting it in virsh. Signed-off-by: Peter Krempa --- include/libvirt/libvirt-domain.h | 9 + tools/virsh-domain.c | 8 2 files changed, 17 insertions(+) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH] qemu: Move interlocking of blockjobs and checkpoints after liveness check
On 4/23/20 9:13 AM, Peter Krempa wrote: qemuDomainSupportsCheckpointsBlockjobs checks if the QEMU_CAPS_INCREMENTAL_BACKUP capability is supported to do the interlocking. Capabilities are not present when the VM isn't running though which would create false errors. Move the checks after the liveness check in block job implementations. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH 2/8] qemuhotplugtest: detach: Add expected 'object-del' to disk-scsi-multipath case
The test verifies unplug of a disk with the persistent reservations helper. The 'object-del' used to remove the helper was not mentioned in the list of expected commands. Signed-off-by: Peter Krempa --- tests/qemuhotplugtest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9a787f9d11..628689d27a 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -763,7 +763,8 @@ mymain(void) "human-monitor-command", HMP("")); DO_TEST_DETACH("base-live", "disk-scsi-multipath", false, false, "device_del", QMP_DEVICE_DELETED("scsi0-0-0-0") QMP_OK, - "human-monitor-command", HMP("")); + "human-monitor-command", HMP(""), + "object-del", QMP_OK); DO_TEST_ATTACH("base-live", "qemu-agent", false, true, "chardev-add", QMP_OK, -- 2.26.0
[PATCH 5/8] qemucapabilitiesdata: riscv: Remove call to 'query-machines'
The riscv capabilities code doesn't use the data. Signed-off-by: Peter Krempa --- .../caps_3.0.0.riscv32.replies| 42 --- .../caps_3.0.0.riscv64.replies| 42 --- .../caps_4.0.0.riscv32.replies| 42 --- .../caps_4.0.0.riscv64.replies| 42 --- 4 files changed, 168 deletions(-) diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.replies b/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.replies index efb18678a2..8159b26d19 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.replies +++ b/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.replies @@ -14718,45 +14718,3 @@ ], "id": "libvirt-33" } - -{ - "execute": "query-machines", - "id": "libvirt-34" -} - -{ - "return": [ -{ - "hotpluggable-cpus": false, - "name": "spike_v1.9.1", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_e", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "none", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "spike_v1.10", - "is-default": true, - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "virt", - "cpu-max": 8 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_u", - "cpu-max": 1 -} - ], - "id": "libvirt-34" -} diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.replies b/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.replies index 018e52f8da..995ca86784 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.replies +++ b/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.replies @@ -14718,45 +14718,3 @@ ], "id": "libvirt-33" } - -{ - "execute": "query-machines", - "id": "libvirt-34" -} - -{ - "return": [ -{ - "hotpluggable-cpus": false, - "name": "spike_v1.9.1", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_e", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "none", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "spike_v1.10", - "is-default": true, - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "virt", - "cpu-max": 8 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_u", - "cpu-max": 1 -} - ], - "id": "libvirt-34" -} diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies index 2d63851d3a..93fd47e8d3 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies @@ -17963,45 +17963,3 @@ ], "id": "libvirt-40" } - -{ - "execute": "query-machines", - "id": "libvirt-41" -} - -{ - "return": [ -{ - "hotpluggable-cpus": false, - "name": "virt", - "cpu-max": 8 -}, -{ - "hotpluggable-cpus": false, - "name": "none", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "spike_v1.10", - "is-default": true, - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_u", - "cpu-max": 4 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_e", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "spike_v1.9.1", - "cpu-max": 1 -} - ], - "id": "libvirt-41" -} diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies index 4df475d7c0..448c9d2402 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies @@ -17963,45 +17963,3 @@ ], "id": "libvirt-40" } - -{ - "execute": "query-machines", - "id": "libvirt-41" -} - -{ - "return": [ -{ - "hotpluggable-cpus": false, - "name": "virt", - "cpu-max": 8 -}, -{ - "hotpluggable-cpus": false, - "name": "none", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "spike_v1.10", - "is-default": true, - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_u", - "cpu-max": 4 -}, -{ - "hotpluggable-cpus": false, - "name": "sifive_e", - "cpu-max": 1 -}, -{ - "hotpluggable-cpus": false, - "name": "spike_v1.9.1", - "cpu-max": 1 -} - ], - "id": "libvirt-41" -} -- 2.26.0
[PATCH 4/8] qemumonitortestutils: Make test monitor failures more prominent
Until now we've tried to report errors from the test monitor code by passing them back as failures from the qemu we simulate. This doesn't work well in cases when the monitor logic does not detect failures or has fallback code. Additionally there isn't much use for continuing the test execution after first failure as in most cases the test data will be misaligned and all other calls will fail as well. To make the errors more obvious this patch moves away from reporting them via the simulated monitor to reporting them to stderr and exit()ing afterwards. While this might be less convenient when developing tests it actually makes failures in the test suite really obvious and doesn't require any opt-in from the tests themselves. Signed-off-by: Peter Krempa --- build-aux/syntax-check.mk| 1 + tests/qemumonitortestutils.c | 121 +-- 2 files changed, 61 insertions(+), 61 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index cbcdf445aa..bf8832a2a5 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -900,6 +900,7 @@ sc_flake8: sc_prohibit_exit_in_tests: @prohibit='\nitems == 0) { -return qemuMonitorTestAddUnexpectedErrorResponse(test, cmdstr); +qemuMonitorTestError("unexpected command: '%s'", cmdstr); } else { qemuMonitorTestItemPtr item = test->items[0]; ret = (item->cb)(test, item, cmdstr); @@ -527,10 +542,7 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, if (virQEMUQAPISchemaPathGet(schemapath, test->qapischema, &schemaroot) < 0 || !schemaroot) { -if (qemuMonitorTestAddErrorResponse(test, -"command '%s' not found in QAPI schema", -cmdname) == 0) -return 1; +qemuMonitorTestError("command '%s' not found in QAPI schema", cmdname); return -1; } @@ -546,12 +558,10 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, cmdname, NULLSTR(argstr), virBufferCurrentContent(&debug)); } -if (qemuMonitorTestAddErrorResponse(test, -"failed to validate arguments of '%s' " -"against QAPI schema " -"(to see debug output use VIR_TEST_DEBUG=2)", -cmdname) == 0) -return 1; +qemuMonitorTestError("failed to validate arguments of '%s' " + "against QAPI schema " + "(to see debug output use VIR_TEST_DEBUG=2)", + cmdname); return -1; } @@ -573,8 +583,10 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr test, if (!(val = virJSONValueFromString(cmdstr))) return -1; -if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) -return qemuMonitorTestAddErrorResponse(test, "Missing command name in %s", cmdstr); +if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) { +qemuMonitorTestError("Missing command name in %s", cmdstr); +return -1; +} cmdargs = virJSONValueObjectGet(val, "arguments"); if ((rc = qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, cmdargs)) < 0) @@ -583,8 +595,8 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr test, return 0; if (data->command_name && STRNEQ(data->command_name, cmdname)) { -return qemuMonitorTestAddInvalidCommandResponse(test, data->command_name, -cmdname); +qemuMonitorTestErrorInvalidCommand(data->command_name, cmdname); +return -1; } else { return qemuMonitorTestAddResponse(test, data->response); } @@ -617,7 +629,6 @@ qemuMonitorTestProcessCommandVerbatim(qemuMonitorTestPtr test, { struct qemuMonitorTestHandlerData *data = item->opaque; g_autofree char *reformatted = NULL; -g_autofree char *errmsg = NULL; g_autoptr(virJSONValue) json = NULL; virJSONValuePtr cmdargs; const char *cmdname; @@ -646,13 +657,9 @@ qemuMonitorTestProcessCommandVerbatim(qemuMonitorTestPtr test, ret = qemuMonitorTestAddResponse(test, data->response); } else { if (data->cmderr) { -errmsg = g_strdup_printf("%s: %s", data->cmderr, cmdstr); - -ret = qemuMonitorTestAddErrorResponseInternal(test, errmsg); +qemuMonitorTestError("%s: %s", data->cmderr, cmdstr); } else { -ret = qemuMonitorTestAddInvalidCommandResponse(test, - data->command_name, - cmdstr); +qemuMonitorTestErrorInvalidCommand(data->command_name, cmdstr);
[PATCH 3/8] qemuhotplugtest: cpu: x86-modern-individual: Remove invalid test case
One of the test cases attempted to use test data meant for modern qemu without asserting the 'modern' flag. Since that changes the commands used to query state it won't work with data meant for the modern case. Remove the invalid test case. Signed-off-by: Peter Krempa --- tests/qemuhotplugtest.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 628689d27a..8afe7f7faa 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -886,7 +886,6 @@ mymain(void) DO_TEST_CPU_INDIVIDUAL("x86-modern-individual-add", "7", true, true, false); DO_TEST_CPU_INDIVIDUAL("x86-modern-individual-add", "6,7", true, true, true); DO_TEST_CPU_INDIVIDUAL("x86-modern-individual-add", "7", false, true, true); -DO_TEST_CPU_INDIVIDUAL("x86-modern-individual-add", "7", true, false, true); DO_TEST_CPU_INDIVIDUAL("ppc64-modern-individual", "16-23", true, true, false); DO_TEST_CPU_INDIVIDUAL("ppc64-modern-individual", "16-22", true, true, true); -- 2.26.0
[PATCH 8/8] qemumonitortestutils: Enforce consumption of all items in test monitor
To prevent unexpected situations where a change in code would stop looking at some of the tested commands go unnoticed add a mechanism to force consumption of all test items. Since there are a few tests which would be hard to fix add also a mechanism to opt-out of the check. Signed-off-by: Peter Krempa --- tests/cputest.c | 2 ++ tests/qemuhotplugtest.c | 2 ++ tests/qemumonitorjsontest.c | 2 ++ tests/qemumonitortestutils.c | 31 ++- tests/qemumonitortestutils.h | 2 ++ 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/cputest.c b/tests/cputest.c index 869d016ffc..21f47a6853 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -482,6 +482,8 @@ cpuTestMakeQEMUCaps(const struct data *data) if (!(testMon = qemuMonitorTestNewFromFile(json, driver.xmlopt, true))) goto error; +qemuMonitorTestAllowUnusedCommands(testMon); + cpu = virCPUDefNew(); cpu->model = g_strdup("host"); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 65867a0122..9a215ab303 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -453,6 +453,8 @@ testQemuHotplugCpuPrepare(const char *test, &driver, data->vm, qmpschema))) goto error; +qemuMonitorTestAllowUnusedCommands(data->mon); + priv->mon = qemuMonitorTestGetMonitor(data->mon); virObjectUnlock(priv->mon); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 615bc8c102..60c816d1d1 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -796,6 +796,8 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt, if (!(data.test = qemuMonitorTestNewSchema(xmlopt, schema))) goto cleanup; +qemuMonitorTestAllowUnusedCommands(data.test); + if (qemuMonitorTestAddItemExpect(data.test, "chardev-add", expectargs, true, jsonreply) < 0) goto cleanup; diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 1af56c6d87..0b6188b4ca 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -57,6 +57,8 @@ struct _qemuMonitorTest { bool running; bool started; +bool allowUnusedCommands; + char *incoming; size_t incomingLength; size_t incomingCapacity; @@ -423,8 +425,15 @@ qemuMonitorTestFree(qemuMonitorTestPtr test) VIR_FREE(test->incoming); VIR_FREE(test->outgoing); -for (i = 0; i < test->nitems; i++) +for (i = 0; i < test->nitems; i++) { +if (!test->allowUnusedCommands) { +g_fprintf(stderr, + "\nunused test monitor item '%s'\n", + NULLSTR(test->items[i]->identifier)); +} + qemuMonitorTestItemFree(test->items[i]); +} VIR_FREE(test->items); if (test->tmpdir && rmdir(test->tmpdir) < 0) @@ -432,6 +441,11 @@ qemuMonitorTestFree(qemuMonitorTestPtr test) VIR_FREE(test->tmpdir); +if (!test->allowUnusedCommands && +test->nitems != 0) { +qemuMonitorTestError("unused test monitor items are not allowed for this test\n"); +} + virMutexDestroy(&test->lock); VIR_FREE(test); } @@ -1290,6 +1304,21 @@ qemuMonitorTestNewFromFile(const char *fileName, } +/** + * qemuMonitorTestAllowUnusedCommands: + * @test: test monitor object + * + * By default all test items/commands must be used by the test. This function + * allows to override the requirement for individual tests e.g. if it's necessary + * to test some negative scenarios which would not use all commands. + */ +void +qemuMonitorTestAllowUnusedCommands(qemuMonitorTestPtr test) +{ +test->allowUnusedCommands = true; +} + + static int qemuMonitorTestFullAddItem(qemuMonitorTestPtr test, const char *filename, diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h index 384002d086..f45e85 100644 --- a/tests/qemumonitortestutils.h +++ b/tests/qemumonitortestutils.h @@ -50,6 +50,8 @@ void *qemuMonitorTestItemGetPrivateData(qemuMonitorTestItemPtr item); int qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, const char *errmsg, ...); +void qemuMonitorTestAllowUnusedCommands(qemuMonitorTestPtr test); + int qemuMonitorTestAddItem(qemuMonitorTestPtr test, const char *command_name, const char *response); -- 2.26.0
[PATCH 6/8] qemuhotplugtest: Remove 'drive_del' expectation from failed cases
On failure 'drive_del' is not issued. Signed-off-by: Peter Krempa --- tests/qemuhotplugtest.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 8afe7f7faa..65867a0122 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -748,8 +748,7 @@ mymain(void) /* Disk added */ "device_add", QMP_OK); DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2", false, false, "device_del", QMP_DEVICE_DELETED("scsi3-0-5-6") QMP_OK, "human-monitor-command", HMP("")); @@ -759,8 +758,7 @@ mymain(void) "human-monitor-command", HMP("OK\\r\\n"), "device_add", QMP_OK); DO_TEST_DETACH("base-live", "disk-scsi-multipath", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-live", "disk-scsi-multipath", false, false, "device_del", QMP_DEVICE_DELETED("scsi0-0-0-0") QMP_OK, "human-monitor-command", HMP(""), -- 2.26.0
[PATCH 7/8] qemumonitortestutils: Store a string identifying test monitor entry
For each test monitor entry store an optional string which will allow to identify it. This will be used later when checking that all registered monitor commands were used. Signed-off-by: Peter Krempa --- tests/qemuagenttest.c| 17 - tests/qemumonitortestutils.c | 10 ++ tests/qemumonitortestutils.h | 1 + 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 943251df77..2c3a1efb09 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -471,7 +471,8 @@ testQemuAgentShutdown(const void *data) priv.event = QEMU_AGENT_EVENT_SHUTDOWN; priv.mode = "halt"; -if (qemuMonitorTestAddHandler(test, qemuAgentShutdownTestMonitorHandler, +if (qemuMonitorTestAddHandler(test, "guest-shutdown", + qemuAgentShutdownTestMonitorHandler, &priv, NULL) < 0) goto cleanup; @@ -485,7 +486,8 @@ testQemuAgentShutdown(const void *data) priv.event = QEMU_AGENT_EVENT_SHUTDOWN; priv.mode = "powerdown"; -if (qemuMonitorTestAddHandler(test, qemuAgentShutdownTestMonitorHandler, +if (qemuMonitorTestAddHandler(test, "guest-shutdown", + qemuAgentShutdownTestMonitorHandler, &priv, NULL) < 0) goto cleanup; @@ -499,7 +501,9 @@ testQemuAgentShutdown(const void *data) priv.event = QEMU_AGENT_EVENT_RESET; priv.mode = "reboot"; -if (qemuMonitorTestAddHandler(test, qemuAgentShutdownTestMonitorHandler, +if (qemuMonitorTestAddHandler(test, + "guest-shutdown", + qemuAgentShutdownTestMonitorHandler, &priv, NULL) < 0) goto cleanup; @@ -720,7 +724,8 @@ testQemuAgentTimeout(const void *data) goto cleanup; } -if (qemuMonitorTestAddHandler(test, qemuAgentTimeoutTestMonitorHandler, +if (qemuMonitorTestAddHandler(test, NULL, + qemuAgentTimeoutTestMonitorHandler, NULL, NULL) < 0) goto cleanup; @@ -734,7 +739,9 @@ testQemuAgentTimeout(const void *data) if (qemuMonitorTestAddAgentSyncResponse(test) < 0) goto cleanup; -if (qemuMonitorTestAddHandler(test, qemuAgentTimeoutTestMonitorHandler, +if (qemuMonitorTestAddHandler(test, + NULL, + qemuAgentTimeoutTestMonitorHandler, NULL, NULL) < 0) goto cleanup; diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index bc3b2f5f92..1af56c6d87 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -43,6 +43,7 @@ VIR_LOG_INIT("tests.qemumonitortestutils"); struct _qemuMonitorTestItem { +char *identifier; qemuMonitorTestResponseCallback cb; void *opaque; virFreeCallback freecb; @@ -88,6 +89,8 @@ qemuMonitorTestItemFree(qemuMonitorTestItemPtr item) if (!item) return; +g_free(item->identifier); + if (item->freecb) (item->freecb)(item->opaque); @@ -436,6 +439,7 @@ qemuMonitorTestFree(qemuMonitorTestPtr test) int qemuMonitorTestAddHandler(qemuMonitorTestPtr test, + const char *identifier, qemuMonitorTestResponseCallback cb, void *opaque, virFreeCallback freecb) @@ -445,6 +449,7 @@ qemuMonitorTestAddHandler(qemuMonitorTestPtr test, if (VIR_ALLOC(item) < 0) goto error; +item->identifier = g_strdup(identifier); item->cb = cb; item->freecb = freecb; item->opaque = opaque; @@ -617,6 +622,7 @@ qemuMonitorTestAddItem(qemuMonitorTestPtr test, data->response = g_strdup(response); return qemuMonitorTestAddHandler(test, + command_name, qemuMonitorTestProcessCommandDefault, data, qemuMonitorTestHandlerDataFree); } @@ -700,6 +706,7 @@ qemuMonitorTestAddItemVerbatim(qemuMonitorTestPtr test, goto error; return qemuMonitorTestAddHandler(test, + command, qemuMonitorTestProcessCommandVerbatim, data, qemuMonitorTestHandlerDataFree); @@ -766,6 +773,7 @@ qemuMonitorTestAddAgentSyncResponse(qemuMonitorTestPtr test) } return qemuMonitorTestAddHandler(test, + "agent-sync", qemuMonitorTestProcessGuestAgentSync, NULL, NULL); } @@ -884,6 +892,7 @@ qemuMonitorTestAddItemParams(qemuMonitorTestPtr test, va_end(args); return qemuMonitorTestAddHandler(test, +
[PATCH 0/8] tests: qemumonitor: Make fake qemu monitor failures more obvious
See patch 4 and 8 for details. Peter Krempa (8): tests: monitor: Rename qemuMonitorReportError to qemuMonitorTestAddErrorResponse qemuhotplugtest: detach: Add expected 'object-del' to disk-scsi-multipath case qemuhotplugtest: cpu: x86-modern-individual: Remove invalid test case qemumonitortestutils: Make test monitor failures more prominent qemucapabilitiesdata: riscv: Remove call to 'query-machines' qemuhotplugtest: Remove 'drive_del' expectation from failed cases qemumonitortestutils: Store a string identifying test monitor entry qemumonitortestutils: Enforce consumption of all items in test monitor build-aux/syntax-check.mk | 1 + tests/cputest.c | 2 + tests/qemuagenttest.c | 31 +-- .../caps_3.0.0.riscv32.replies| 42 - .../caps_3.0.0.riscv64.replies| 42 - .../caps_4.0.0.riscv32.replies| 42 - .../caps_4.0.0.riscv64.replies| 42 - tests/qemuhotplugtest.c | 12 +- tests/qemumonitorjsontest.c | 2 + tests/qemumonitortestutils.c | 176 +++--- tests/qemumonitortestutils.h | 5 +- 11 files changed, 141 insertions(+), 256 deletions(-) -- 2.26.0
[PATCH 1/8] tests: monitor: Rename qemuMonitorReportError to qemuMonitorTestAddErrorResponse
It's a method of the test monitor and it adds a response to the monitor output. The original qemuMonitorTestAddErrorResponse method is renamed to qemuMonitorTestAddErrorResponseInternal Signed-off-by: Peter Krempa --- tests/qemuagenttest.c| 14 +++--- tests/qemumonitortestutils.c | 82 ++-- tests/qemumonitortestutils.h | 2 +- 3 files changed, 49 insertions(+), 49 deletions(-) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 42ef81ac9a..943251df77 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -413,7 +413,7 @@ qemuAgentShutdownTestMonitorHandler(qemuMonitorTestPtr test, return -1; if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) { -ret = qemuMonitorReportError(test, "Missing command name in %s", cmdstr); +ret = qemuMonitorTestAddErrorResponse(test, "Missing command name in %s", cmdstr); goto cleanup; } @@ -424,20 +424,20 @@ qemuAgentShutdownTestMonitorHandler(qemuMonitorTestPtr test, } if (!(args = virJSONValueObjectGet(val, "arguments"))) { -ret = qemuMonitorReportError(test, - "Missing arguments section"); +ret = qemuMonitorTestAddErrorResponse(test, + "Missing arguments section"); goto cleanup; } if (!(mode = virJSONValueObjectGetString(args, "mode"))) { -ret = qemuMonitorReportError(test, "Missing shutdown mode"); +ret = qemuMonitorTestAddErrorResponse(test, "Missing shutdown mode"); goto cleanup; } if (STRNEQ(mode, data->mode)) { -ret = qemuMonitorReportError(test, - "expected shutdown mode '%s' got '%s'", - data->mode, mode); +ret = qemuMonitorTestAddErrorResponse(test, + "expected shutdown mode '%s' got '%s'", + data->mode, mode); goto cleanup; } diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 992d2a4e71..dc753dd417 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -122,8 +122,8 @@ qemuMonitorTestAddResponse(qemuMonitorTestPtr test, static int -qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, -const char *usermsg) +qemuMonitorTestAddErrorResponseInternal(qemuMonitorTestPtr test, +const char *usermsg) { virBuffer buf = VIR_BUFFER_INITIALIZER; g_autofree char *escapemsg = NULL; @@ -161,7 +161,7 @@ qemuMonitorTestAddUnexpectedErrorResponse(qemuMonitorTestPtr test, msg = g_strdup_printf("unexpected command: '%s'", command); -return qemuMonitorTestAddErrorResponse(test, msg); +return qemuMonitorTestAddErrorResponseInternal(test, msg); } @@ -175,12 +175,12 @@ qemuMonitorTestAddInvalidCommandResponse(qemuMonitorTestPtr test, msg = g_strdup_printf("expected command '%s' got '%s'", expectedcommand, actualcommand); -return qemuMonitorTestAddErrorResponse(test, msg); +return qemuMonitorTestAddErrorResponseInternal(test, msg); } int G_GNUC_PRINTF(2, 3) -qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...) +qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, const char *errmsg, ...) { va_list msgargs; g_autofree char *msg = NULL; @@ -527,9 +527,9 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, if (virQEMUQAPISchemaPathGet(schemapath, test->qapischema, &schemaroot) < 0 || !schemaroot) { -if (qemuMonitorReportError(test, - "command '%s' not found in QAPI schema", - cmdname) == 0) +if (qemuMonitorTestAddErrorResponse(test, +"command '%s' not found in QAPI schema", +cmdname) == 0) return 1; return -1; } @@ -546,11 +546,11 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, cmdname, NULLSTR(argstr), virBufferCurrentContent(&debug)); } -if (qemuMonitorReportError(test, - "failed to validate arguments of '%s' " - "against QAPI schema " - "(to see debug output use VIR_TEST_DEBUG=2)", - cmdname) == 0) +if (qemuMonitorTestAddErrorResponse(test, +"failed to validate arguments of '%s' " +"against QAPI schema " +"(to see debug output use VIR_TEST_DEBUG=2)", +cmdname) =
Re: [libvirt PATCH 3/3] qemu: block: external snapshot-delete implementation for straightforward cases
On Wed, Apr 22, 2020 at 03:49:57PM +0200, Peter Krempa wrote: > On Thu, Apr 16, 2020 at 17:07:35 +0200, Pavel Mores wrote: > > On Fri, Apr 03, 2020 at 05:56:59PM +0200, Pavel Mores wrote: > > > On Tue, Mar 31, 2020 at 12:06:15PM +0200, Peter Krempa wrote: > > > > On Tue, Mar 31, 2020 at 11:18:29 +0200, Pavel Mores wrote: > > > > > +} > > > > > + > > > > > +if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) > > > > > +topPath = disk->src->path; > > > > > +else > > > > > +topPath = snapdisk->src->path; > > > > > > > > You must not use paths for doing this lookup. Paths at best work for > > > > local files only and this would make the code not future proof. > > > > > > > > Also you want to verify that: > > > > - images you want to merge are actually in the backing chain > > > > - the backing chain looks as snapshots describe it (e.g you unplug vda > > > > and plug a different chain back) > > > > > > Let me check with you how exactly to perform this verification. > > > > > > To retrieve the names of the disks included in a snapshot, I can use its > > > virDomainSnapshotDef which contains an array of disks > > > (virDomainSnapshotDiskDef's) whose 'name' member can be used to identify > > > disks. > > > > > > To get the state of the chain at moment the snapshot was created, I can > > > use the > > > 'src' member and walk its 'backingStore' list to examine the whole chain. > > > > > > To get the currently running configuration, I assume I can use the names > > > of the > > > relevant disks (obtained from the snapshot as mentioned above) to get a > > > virDomainDiskDefPtr for each of them, whose 'src' member points to a > > > virStorageSource that represents the topmost image in the disk's chain. > > > From > > > there, I can walk the 'backingStore' list to get the other images in the > > > chain > > > all the way to the base image. > > > > > > The verification succeeds if the disk names and their chains in the > > > snapshot > > > and the running config correspond. Is the above correct? > > > > > > If so, I'm still not certain how to compare two virStorageSource's. How > > > is > > > identity of two different virStorageSource instances is established? > > > > After having a closer look into this I'm unfortunately even less clear as to > > how to perform the checks mentioned in the review. > > Well unfortunately developing this is the main part why deleting > snapshots is complicated. > > > In addition to the problem of establishing virStorageSource identity, a > > similar > > problem seems to come up with disks. They often seem to be identified and > > referred to by the target name, however what happens if a snapshot is taken > > for > > 'vda', then the disk's target is changed to 'vdb' and now the snapshot is > > to be > > In such case I'd consier it as being different. Similarly as we can't > really guarantee that the image described by a virStorageSource is the > same as it was when taking the snapshot. As long as the file is named > the same you can consider it identical IMO. > > > deleted? Is there a way to find out that the disk is still there, only > > under a > > different name? > > No and it doesn't seem to be worth doing that. > > > > > And as far as comparing chains goes - I know can retrieve the chain for a > > running disk and I can retrieve what the chain looked like at the point a > > snapshot was made. However, how do I establish they are equivalent? How > > can I > > What I meant is that you need to establish that the images you are going > to merge and the images you are going to merge into are the same > described by the snapshots and at the same time present in the current > backing chain of the disk. If they are not in the snapshot metadata > you'd potentially merge something unwanted, but this is covered by the > previous paragraphs. > > You need to also verify that they are currently opened by qemu as you > can't do blockjobs on images which are not part of the chain. > > > tell that snapshots in both chains compare identical in the first place? Is > > the snapshot name stable and persistent enough to be useful for this? Is it > > enough for chains to be equivalent that the chain at the point of snapshot > > creation be a superset of the currently running chain? Probably yes, > > provided > > snapshots can't be inserted in the middle of a chain - but could that ever > > happen? > > The main problem is that the configuration may change from the time when > the snapshot was taken. Either the XML or the on disk setup. In case a > user issues a blockjob manually which merges parts of the chain you > won't be able to delete the snapshot with data reorganisation via the > API. Similarly to when the VM config changes to point to other images > and or remove or add disks. The snapshot code should be able to at least > properly reject the operation if it's unclear whether we can do the > r
Re: [PATCH] news: Include new DHCP network feature
Em qui., 23 de abr. de 2020 às 11:10, Michal Privoznik escreveu: > > On 4/23/20 3:19 PM, Julio Faracco wrote: > > This commit includes an entry for new network DHCP lease time > > information inside news.xml. > > > > Signed-off-by: Julio Faracco > > --- > > docs/news.xml | 12 > > 1 file changed, 12 insertions(+) > > > > diff --git a/docs/news.xml b/docs/news.xml > > index 956018b512..3a013ffbbd 100644 > > --- a/docs/news.xml > > +++ b/docs/news.xml > > @@ -88,6 +88,18 @@ > > hotplugging PCI devices. > > > > > > + > > + > > + Lease time option included for network DHCP settings > > + > > + > > + Dnsmasq does not support to configure a global lease time. There > > + is no configuration option that could specify lease time. The > > + only way to configure lease time is setting it for each range's > > + and host's lines mainly. Libvirt now can configure lease time > > + for range and host. > > + > > + > > > > > > > > > > I don't think we need to document dnsmasq's limitations. How about simple: Thinking that way... Yes... I agree. > > Users can now configure expiry time for leases for networks where > libvirt manages DHCP. The time can be specified for whole range > and/or fine tuned per individual host. > > > Michal >
Re: [PATCH] news: Include new DHCP network feature
On 4/23/20 3:19 PM, Julio Faracco wrote: This commit includes an entry for new network DHCP lease time information inside news.xml. Signed-off-by: Julio Faracco --- docs/news.xml | 12 1 file changed, 12 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 956018b512..3a013ffbbd 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -88,6 +88,18 @@ hotplugging PCI devices. + + + Lease time option included for network DHCP settings + + + Dnsmasq does not support to configure a global lease time. There + is no configuration option that could specify lease time. The + only way to configure lease time is setting it for each range's + and host's lines mainly. Libvirt now can configure lease time + for range and host. + + I don't think we need to document dnsmasq's limitations. How about simple: Users can now configure expiry time for leases for networks where libvirt manages DHCP. The time can be specified for whole range and/or fine tuned per individual host. Michal
[PATCH] qemu: Move interlocking of blockjobs and checkpoints after liveness check
qemuDomainSupportsCheckpointsBlockjobs checks if the QEMU_CAPS_INCREMENTAL_BACKUP capability is supported to do the interlocking. Capabilities are not present when the VM isn't running though which would create false errors. Move the checks after the liveness check in block job implementations. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 00d276061e..5ecf12deef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17439,6 +17439,9 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm, if (virDomainObjCheckActive(vm) < 0) goto endjob; +if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) +goto endjob; + if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; @@ -17994,6 +17997,9 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (virDomainObjCheckActive(vm) < 0) goto endjob; +if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) +goto endjob; + if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; @@ -18278,9 +18284,6 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0) goto cleanup; -if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) -goto cleanup; - /* For normal rebase (enhanced blockpull), the common code handles * everything, including vm cleanup. */ if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY)) @@ -18364,9 +18367,6 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, if (virDomainBlockCopyEnsureACL(dom->conn, vm->def) < 0) goto cleanup; -if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) -goto cleanup; - for (i = 0; i < nparams; i++) { virTypedParameterPtr param = ¶ms[i]; @@ -18429,11 +18429,6 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, return -1; } -if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) { -virDomainObjEndAPI(&vm); -return -1; -} - /* qemuDomainBlockPullCommon consumes the reference on @vm */ return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags); } -- 2.26.0
[PATCH] news: Include new DHCP network feature
This commit includes an entry for new network DHCP lease time information inside news.xml. Signed-off-by: Julio Faracco --- docs/news.xml | 12 1 file changed, 12 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 956018b512..3a013ffbbd 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -88,6 +88,18 @@ hotplugging PCI devices. + + + Lease time option included for network DHCP settings + + + Dnsmasq does not support to configure a global lease time. There + is no configuration option that could specify lease time. The + only way to configure lease time is setting it for each range's + and host's lines mainly. Libvirt now can configure lease time + for range and host. + + -- 2.25.3
[PATCH 2/4] tests: more fine-granular tests for virtio-options
Add separate tests for individual options and devices for virtio-options to have the ability to do more fine-granular testing of various combinations. Also, add negative tests for unavailable capabilities. Reviewed-by: Boris Fiuczynski Signed-off-by: Bjoern Walk --- .../virtio-options-controller-ats.args| 32 ++ .../virtio-options-controller-ats.xml | 38 +++ .../virtio-options-controller-iommu.args | 34 +++ .../virtio-options-controller-iommu.xml | 38 +++ .../virtio-options-controller-packed.args | 32 ++ .../virtio-options-controller-packed.xml | 38 +++ .../virtio-options-disk-ats.args | 36 +++ .../virtio-options-disk-ats.xml | 34 +++ .../virtio-options-disk-iommu.args| 36 +++ .../virtio-options-disk-iommu.xml | 34 +++ .../virtio-options-disk-packed.args | 36 +++ .../virtio-options-disk-packed.xml| 34 +++ .../virtio-options-fs-ats.args| 34 +++ .../virtio-options-fs-ats.xml | 34 +++ .../virtio-options-fs-iommu.args | 34 +++ .../virtio-options-fs-iommu.xml | 34 +++ .../virtio-options-fs-packed.args | 34 +++ .../virtio-options-fs-packed.xml | 34 +++ .../virtio-options-input-ats.args | 30 ++ .../virtio-options-input-ats.xml | 30 ++ .../virtio-options-input-iommu.args | 30 ++ .../virtio-options-input-iommu.xml| 30 ++ .../virtio-options-input-packed.args | 30 ++ .../virtio-options-input-packed.xml | 30 ++ .../virtio-options-memballoon-ats.args| 28 ++ .../virtio-options-memballoon-ats.xml | 23 + .../virtio-options-memballoon-iommu.args | 28 ++ .../virtio-options-memballoon-iommu.xml | 23 + .../virtio-options-memballoon-packed.args | 28 ++ .../virtio-options-memballoon-packed.xml | 23 + .../virtio-options-net-ats.args | 34 +++ .../virtio-options-net-ats.xml| 34 +++ .../virtio-options-net-iommu.args | 34 +++ .../virtio-options-net-iommu.xml | 34 +++ .../virtio-options-net-packed.args| 34 +++ .../virtio-options-net-packed.xml | 34 +++ .../virtio-options-rng-ats.args | 32 ++ .../virtio-options-rng-ats.xml| 32 ++ .../virtio-options-rng-iommu.args | 34 +++ .../virtio-options-rng-iommu.xml | 32 ++ .../virtio-options-rng-packed.args| 32 ++ .../virtio-options-rng-packed.xml | 32 ++ .../virtio-options-video-ats.args | 34 +++ .../virtio-options-video-ats.xml | 36 +++ .../virtio-options-video-iommu.args | 34 +++ .../virtio-options-video-iommu.xml| 36 +++ .../virtio-options-video-packed.args | 34 +++ .../virtio-options-video-packed.xml | 36 +++ tests/qemuxml2argvtest.c | 99 +++ 49 files changed, 1666 insertions(+) create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-ats.args create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-ats.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-iommu.args create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-iommu.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-packed.args create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-packed.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-ats.args create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-ats.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-iommu.args create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-iommu.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-packed.args create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-packed.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-ats.args create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-ats.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-iommu.args create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-iommu.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-packed.args create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-packed.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-input-ats.args create mode 100644 tests/qemuxml2argvdata/virtio-options-input-ats.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-input-iommu.args create mode 100644 tests/qemuxml2argvdata/virtio-options-input-iommu.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-input-packed.args create mode 100644 tests/qemuxml2argvdata/virtio
[PATCH 3/4] qemu: move virtio capability validation
Move capability validation of virtio options from command line generation to post-parse device validation where it belongs. Signed-off-by: Bjoern Walk --- src/qemu/qemu_command.c | 41 ++-- src/qemu/qemu_validate.c | 70 +++-- tests/qemuxml2argvtest.c | 84 3 files changed, 120 insertions(+), 75 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 95402fc4..ca9d3f10 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -588,39 +588,20 @@ qemuBuildVirtioDevStr(virBufferPtr buf, static int qemuBuildVirtioOptionsStr(virBufferPtr buf, - virDomainVirtioOptionsPtr virtio, - virQEMUCapsPtr qemuCaps) + virDomainVirtioOptionsPtr virtio) { if (!virtio) return 0; if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the iommu setting is not supported " - "with this QEMU binary")); -return -1; -} virBufferAsprintf(buf, ",iommu_platform=%s", virTristateSwitchTypeToString(virtio->iommu)); } if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_ATS)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the ats setting is not supported with this " - "QEMU binary")); -return -1; -} virBufferAsprintf(buf, ",ats=%s", virTristateSwitchTypeToString(virtio->ats)); } if (virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PACKED_QUEUES)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the packed setting is not supported with this " - "QEMU binary")); -return -1; -} virBufferAsprintf(buf, ",packed=%s", virTristateSwitchTypeToString(virtio->packed)); } @@ -2150,7 +2131,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); } -if (qemuBuildVirtioOptionsStr(&opt, disk->virtio, qemuCaps) < 0) +if (qemuBuildVirtioOptionsStr(&opt, disk->virtio) < 0) return NULL; if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) @@ -2615,7 +2596,7 @@ qemuBuildVHostUserFsCommandLine(virCommandPtr cmd, virBufferAsprintf(&opt, ",queue-size=%llu", fs->queue_size); virBufferAddLit(&opt, ",tag="); virQEMUBuildBufferEscapeComma(&opt, fs->dst); -if (qemuBuildVirtioOptionsStr(&opt, fs->virtio, priv->qemuCaps) < 0) +if (qemuBuildVirtioOptionsStr(&opt, fs->virtio) < 0) return -1; if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, priv->qemuCaps) < 0) @@ -2685,7 +2666,7 @@ qemuBuildFSDevStr(const virDomainDef *def, virBufferAddLit(&opt, ",mount_tag="); virQEMUBuildBufferEscapeComma(&opt, fs->dst); -if (qemuBuildVirtioOptionsStr(&opt, fs->virtio, qemuCaps) < 0) +if (qemuBuildVirtioOptionsStr(&opt, fs->virtio) < 0) return NULL; if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) @@ -2917,7 +2898,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->iothread); } -if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0) +if (qemuBuildVirtioOptionsStr(&buf, def->virtio) < 0) return -1; break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: @@ -2964,7 +2945,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, ",vectors=%d", def->opts.vioserial.vectors); } -if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0) +if (qemuBuildVirtioOptionsStr(&buf, def->virtio) < 0) return -1; break; @@ -3916,7 +3897,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, if (bootindex) virBufferAsprintf(&buf, ",bootindex=%u", bootindex); if (usingVirtio && -qemuBuildVirtioOptionsStr(&buf, net->virtio, qemuCaps) < 0) +qemuBuildVirtioOptionsStr(&buf, net->virtio) < 0) return NULL; return virBufferContentAndReset(&buf); @@ -4158,7 +4139,7 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, virTristateSwitchTypeToString(def->memballoon->autodeflate)); } -if (qemuBuildVirtioOptionsStr(&buf, def->memballoon->virtio, qemuCaps) < 0) +if (qemuBuildVirtioOptionsSt
[PATCH 0/4] move validation of virtio options
Move validation of virtio options (iommu, ats, packed) from QEMU command line generation to domain validation. As a drive-by, increase the granularity of tests for virtio options. Bjoern Walk (4): tests: use latest caps for virtio-options test tests: more fine-granular tests for virtio-options qemu: move virtio capability validation qemu: command: make qemuBuildVirtioOptionsStr void src/qemu/qemu_command.c | 59 +++--- src/qemu/qemu_validate.c | 70 +++- .../virtio-options-controller-ats.args| 32 ++ .../virtio-options-controller-ats.xml | 38 +++ .../virtio-options-controller-iommu.args | 34 ++ .../virtio-options-controller-iommu.xml | 38 +++ .../virtio-options-controller-packed.args | 32 ++ .../virtio-options-controller-packed.xml | 38 +++ .../virtio-options-disk-ats.args | 36 +++ .../virtio-options-disk-ats.xml | 34 ++ .../virtio-options-disk-iommu.args| 36 +++ .../virtio-options-disk-iommu.xml | 34 ++ .../virtio-options-disk-packed.args | 36 +++ .../virtio-options-disk-packed.xml| 34 ++ .../virtio-options-fs-ats.args| 34 ++ .../virtio-options-fs-ats.xml | 34 ++ .../virtio-options-fs-iommu.args | 34 ++ .../virtio-options-fs-iommu.xml | 34 ++ .../virtio-options-fs-packed.args | 34 ++ .../virtio-options-fs-packed.xml | 34 ++ .../virtio-options-input-ats.args | 30 ++ .../virtio-options-input-ats.xml | 30 ++ .../virtio-options-input-iommu.args | 30 ++ .../virtio-options-input-iommu.xml| 30 ++ .../virtio-options-input-packed.args | 30 ++ .../virtio-options-input-packed.xml | 30 ++ .../virtio-options-memballoon-ats.args| 28 + .../virtio-options-memballoon-ats.xml | 23 .../virtio-options-memballoon-iommu.args | 28 + .../virtio-options-memballoon-iommu.xml | 23 .../virtio-options-memballoon-packed.args | 28 + .../virtio-options-memballoon-packed.xml | 23 .../virtio-options-net-ats.args | 34 ++ .../virtio-options-net-ats.xml| 34 ++ .../virtio-options-net-iommu.args | 34 ++ .../virtio-options-net-iommu.xml | 34 ++ .../virtio-options-net-packed.args| 34 ++ .../virtio-options-net-packed.xml | 34 ++ .../virtio-options-rng-ats.args | 32 ++ .../virtio-options-rng-ats.xml| 32 ++ .../virtio-options-rng-iommu.args | 34 ++ .../virtio-options-rng-iommu.xml | 32 ++ .../virtio-options-rng-packed.args| 32 ++ .../virtio-options-rng-packed.xml | 32 ++ .../virtio-options-video-ats.args | 34 ++ .../virtio-options-video-ats.xml | 36 +++ .../virtio-options-video-iommu.args | 34 ++ .../virtio-options-video-iommu.xml| 36 +++ .../virtio-options-video-packed.args | 34 ++ .../virtio-options-video-packed.xml | 36 +++ .../virtio-options.x86_64-latest.args | 69 tests/qemuxml2argvdata/virtio-options.xml | 5 +- tests/qemuxml2argvtest.c | 101 -- .../virtio-options.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 16 +-- 55 files changed, 1818 insertions(+), 70 deletions(-) create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-ats.args create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-ats.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-iommu.args create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-iommu.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-packed.args create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-packed.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-ats.args create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-ats.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-iommu.args create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-iommu.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-packed.args create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-packed.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-ats.args create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-ats.xml create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-iommu.args create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-iommu.xml create mode 100644 tests/qemuxml2ar
[PATCH 4/4] qemu: command: make qemuBuildVirtioOptionsStr void
Now that qemuBuildVirtioOptionsStr can not fail anymore, remove its return value and make it void. Signed-off-by: Bjoern Walk --- src/qemu/qemu_command.c | 38 +- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ca9d3f10..169a418f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -586,12 +586,12 @@ qemuBuildVirtioDevStr(virBufferPtr buf, return 0; } -static int +static void qemuBuildVirtioOptionsStr(virBufferPtr buf, virDomainVirtioOptionsPtr virtio) { if (!virtio) -return 0; +return; if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { virBufferAsprintf(buf, ",iommu_platform=%s", @@ -605,8 +605,6 @@ qemuBuildVirtioOptionsStr(virBufferPtr buf, virBufferAsprintf(buf, ",packed=%s", virTristateSwitchTypeToString(virtio->packed)); } - -return 0; } static int @@ -2131,8 +2129,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); } -if (qemuBuildVirtioOptionsStr(&opt, disk->virtio) < 0) -return NULL; +qemuBuildVirtioOptionsStr(&opt, disk->virtio); if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) return NULL; @@ -2596,8 +2593,7 @@ qemuBuildVHostUserFsCommandLine(virCommandPtr cmd, virBufferAsprintf(&opt, ",queue-size=%llu", fs->queue_size); virBufferAddLit(&opt, ",tag="); virQEMUBuildBufferEscapeComma(&opt, fs->dst); -if (qemuBuildVirtioOptionsStr(&opt, fs->virtio) < 0) -return -1; +qemuBuildVirtioOptionsStr(&opt, fs->virtio); if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, priv->qemuCaps) < 0) return -1; @@ -2666,8 +2662,7 @@ qemuBuildFSDevStr(const virDomainDef *def, virBufferAddLit(&opt, ",mount_tag="); virQEMUBuildBufferEscapeComma(&opt, fs->dst); -if (qemuBuildVirtioOptionsStr(&opt, fs->virtio) < 0) -return NULL; +qemuBuildVirtioOptionsStr(&opt, fs->virtio); if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) return NULL; @@ -2898,8 +2893,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->iothread); } -if (qemuBuildVirtioOptionsStr(&buf, def->virtio) < 0) -return -1; +qemuBuildVirtioOptionsStr(&buf, def->virtio); break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: virBufferAddLit(&buf, "lsi"); @@ -2945,8 +2939,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, ",vectors=%d", def->opts.vioserial.vectors); } -if (qemuBuildVirtioOptionsStr(&buf, def->virtio) < 0) -return -1; +qemuBuildVirtioOptionsStr(&buf, def->virtio); break; case VIR_DOMAIN_CONTROLLER_TYPE_CCID: @@ -3896,9 +3889,8 @@ qemuBuildNicDevStr(virDomainDefPtr def, return NULL; if (bootindex) virBufferAsprintf(&buf, ",bootindex=%u", bootindex); -if (usingVirtio && -qemuBuildVirtioOptionsStr(&buf, net->virtio) < 0) -return NULL; +if (usingVirtio) +qemuBuildVirtioOptionsStr(&buf, net->virtio); return virBufferContentAndReset(&buf); } @@ -4139,8 +4131,7 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, virTristateSwitchTypeToString(def->memballoon->autodeflate)); } -if (qemuBuildVirtioOptionsStr(&buf, def->memballoon->virtio) < 0) -return -1; +qemuBuildVirtioOptionsStr(&buf, def->memballoon->virtio); if (qemuCommandAddExtDevice(cmd, &def->memballoon->info) < 0) return -1; @@ -4231,8 +4222,7 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) return NULL; -if (qemuBuildVirtioOptionsStr(&buf, dev->virtio) < 0) -return NULL; +qemuBuildVirtioOptionsStr(&buf, dev->virtio); return virBufferContentAndReset(&buf); } @@ -4542,8 +4532,7 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0) return NULL; -if (qemuBuildVirtioOptionsStr(&buf, video->virtio) < 0) -return NULL; +qemuBuildVirtioOptionsStr(&buf, video->virtio); return virBufferContentAndReset(&buf); } @@ -5758,8 +5747,7 @@ qemuBuildRNGDevStr(const virDomainDef *def, virBufferAddLit(&buf, ",period=1000"); } -if (qemuBuildVirtioOptionsStr(&buf, dev->virtio) < 0) -return NULL; +qemuBuildVirtioOptionsStr(&buf, dev->virtio); if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) return NULL; -- 2.24.1
[PATCH 1/4] tests: use latest caps for virtio-options test
Convert the virtio-options test in qemuxml2argv and qemuxml2xml to use the latest available QEMU capabilities. Reviewed-by: Boris Fiuczynski Signed-off-by: Bjoern Walk --- .../virtio-options.x86_64-latest.args | 69 +++ tests/qemuxml2argvdata/virtio-options.xml | 5 +- tests/qemuxml2argvtest.c | 12 +--- .../virtio-options.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 16 + 5 files changed, 76 insertions(+), 27 deletions(-) create mode 100644 tests/qemuxml2argvdata/virtio-options.x86_64-latest.args create mode 12 tests/qemuxml2xmloutdata/virtio-options.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/virtio-options.x86_64-latest.args b/tests/qemuxml2argvdata/virtio-options.x86_64-latest.args new file mode 100644 index ..e523f73f --- /dev/null +++ b/tests/qemuxml2argvdata/virtio-options.x86_64-latest.args @@ -0,0 +1,69 @@ +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 guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-cpu qemu64 \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-device virtio-scsi-pci,iommu_platform=on,ats=on,packed=on,id=scsi0,bus=pci.0,\ +addr=0x8 \ +-device virtio-serial-pci,id=virtio-serial0,iommu_platform=on,ats=on,packed=on,\ +bus=pci.0,addr=0x9 \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/img1",\ +"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\ +"file":"libvirt-1-storage"}' \ +-device virtio-blk-pci,scsi=off,iommu_platform=on,ats=on,packed=on,bus=pci.0,\ +addr=0xa,drive=libvirt-1-format,id=virtio-disk0,bootindex=1 \ +-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/fs1 \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=fs1,iommu_platform=on,\ +ats=on,packed=on,bus=pci.0,addr=0x3 \ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ +path=/export/fs2 \ +-device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,mount_tag=fs2,iommu_platform=on,\ +ats=on,packed=on,bus=pci.0,addr=0x4 \ +-netdev user,id=hostnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:56:58:5a:5c,bus=pci.0,\ +addr=0x6,iommu_platform=on,ats=on,packed=on \ +-device virtio-mouse-pci,id=input0,bus=pci.0,addr=0xe,iommu_platform=on,ats=on,\ +packed=on \ +-device virtio-keyboard-pci,id=input1,bus=pci.0,addr=0x10,iommu_platform=on,\ +ats=on,packed=on \ +-device virtio-tablet-pci,id=input2,bus=pci.0,addr=0x11,iommu_platform=on,\ +ats=on,packed=on \ +-device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\ +addr=0x12,iommu_platform=on,ats=on,packed=on \ +-chardev socket,id=chr-vu-video0,fd=1729 \ +-device vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\ +addr=0x2,iommu_platform=on,ats=on,packed=on \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,iommu_platform=on,\ +ats=on,packed=on \ +-object rng-random,id=objrng0,filename=/dev/random \ +-device virtio-rng-pci,rng=objrng0,id=rng0,iommu_platform=on,ats=on,packed=on,\ +bus=pci.0,addr=0xd \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/virtio-options.xml b/tests/qemuxml2argvdata/virtio-options.xml index 3ca27840..ba1bf7c0 100644 --- a/tests/qemuxml2argvdata/virtio-options.xml +++ b/tests/qemuxml2argvdata/virtio-options.xml @@ -8,6 +8,9 @@ hvm + +qemu64 + destroy restart @@ -20,7 +23,7 @@ - + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 66472a5e..dffca323 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2981,17 +2981,7 @@ mymain(void) DO_TEST_PARSE_ERROR("cpu-hotplug-granularity", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -DO_TEST("virtio-options", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_KEYBOARD, -QEMU_CAPS_VIRTIO_MOUSE, QEMU_CAPS_VIRTIO_TABLET, -QEMU_CAPS_VIRTIO_INPUT_HOST, -QEMU_CAPS_DEVICE_VIRTIO_GPU, -QEMU_CAPS_DEVICE_VHOST_USER_GPU, -QEMU_CAPS_DEVICE_VIRTIO_
Re: [PATCH v3 0/2] Include lease time option into DHCP settings
Hi Michal, Sure! I will copy you to review the content. Feel free to change (for anyone who wants also). Thanks Laine and Daniel for other comments too. I forgot to mention them previously. -- Julio Cesar Faracco Em qui., 23 de abr. de 2020 às 06:01, Michal Privoznik escreveu: > > On 4/22/20 10:05 PM, Julio Faracco wrote: > > This series is based on latest series from Nehal. It includes a new > > entry called under and from scope. > > This was implemented to include independent lease time for each line and > > dnsmasq option. So, users are able to define one lease time for ranges > > and other different for each host entry. The new syntax is simlar with: > > > > > > > > > > > > > > > > > > > > > > It will produce a option in dnsmasq configuration file: > > dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0,14m > > > > And some contents into hostsfile: > > 00:16:3e:77:e2:ed,192.168.122.10,a.example.com,1h > > > > This series includes some test cases to cover lease time XML syntax > > also. Now, each test case requires a hostsfile to test this specific > > setting. > > > > - v1-v2: Change XML syntax according Daniel's suggestion. > > - v2-v3: Fix memory leak and test dependency issue. > > > > Julio Faracco (2): > >conf: Add option for settings > >tests: Add tests for to cover dnsmasq settings > > > > docs/schemas/basictypes.rng | 8 + > > docs/schemas/network.rng | 20 +++ > > src/conf/network_conf.c | 159 +++--- > > src/conf/network_conf.h | 27 ++- > > src/libvirt_private.syms | 3 + > > src/network/bridge_driver.c | 56 +- > > src/network/bridge_driver.h | 1 + > > src/test/test_driver.c| 2 +- > > src/util/virdnsmasq.c | 60 --- > > src/util/virdnsmasq.h | 3 + > > src/vbox/vbox_network.c | 16 +- > > .../dhcp6-nat-network.hostsfile | 7 + > > .../dhcp6-network.hostsfile | 5 + > > .../dhcp6host-routed-network.hostsfile| 7 + > > .../networkxml2confdata/leasetime-hours.conf | 16 ++ > > .../leasetime-hours.hostsfile | 2 + > > tests/networkxml2confdata/leasetime-hours.xml | 19 +++ > > .../leasetime-infinite.conf | 16 ++ > > .../leasetime-infinite.hostsfile | 2 + > > .../leasetime-infinite.xml| 19 +++ > > .../leasetime-minutes.conf| 16 ++ > > .../leasetime-minutes.hostsfile | 2 + > > .../networkxml2confdata/leasetime-minutes.xml | 19 +++ > > .../leasetime-seconds.conf| 16 ++ > > .../leasetime-seconds.hostsfile | 2 + > > .../networkxml2confdata/leasetime-seconds.xml | 19 +++ > > ...t-network-dns-srv-record-minimal.hostsfile | 2 + > > .../nat-network-dns-srv-record.hostsfile | 2 + > > .../nat-network-dns-txt-record.hostsfile | 2 + > > .../nat-network-mtu.hostsfile | 2 + > > .../nat-network-name-with-quotes.hostsfile| 2 + > > .../networkxml2confdata/nat-network.hostsfile | 2 + > > .../ptr-domains-auto.hostsfile| 2 + > > tests/networkxml2conftest.c | 42 - > > tests/networkxml2xmlin/leasetime-hours.xml| 19 +++ > > tests/networkxml2xmlin/leasetime-infinite.xml | 19 +++ > > tests/networkxml2xmlin/leasetime-minutes.xml | 19 +++ > > tests/networkxml2xmlin/leasetime-seconds.xml | 19 +++ > > tests/networkxml2xmlout/leasetime-hours.xml | 21 +++ > > .../networkxml2xmlout/leasetime-infinite.xml | 21 +++ > > tests/networkxml2xmlout/leasetime-minutes.xml | 21 +++ > > tests/networkxml2xmlout/leasetime-seconds.xml | 21 +++ > > tests/networkxml2xmltest.c| 4 + > > 43 files changed, 676 insertions(+), 66 deletions(-) > > create mode 100644 tests/networkxml2confdata/dhcp6-nat-network.hostsfile > > create mode 100644 tests/networkxml2confdata/dhcp6-network.hostsfile > > create mode 100644 > > tests/networkxml2confdata/dhcp6host-routed-network.hostsfile > > create mode 100644 tests/networkxml2confdata/leasetime-hours.conf > > create mode 100644 tests/networkxml2confdata/leasetime-hours.hostsfile > > create mode 100644 tests/networkxml2confdata/leasetime-hours.xml > > create mode 100644 tests/networkxml2confdata/leasetime-infinite.conf > > create mode 100644 tests/networkxml2confdata/leasetime-infinite.hostsfile > > create mode 100644 tests/networkxml2confdata/leasetime-infinite.xml > > create mode 100644 tests/networkxml2confdata/leasetime-minutes.conf > > create mode 100644 tests/networkxml2confdata/leasetime-minutes.hostsfile > > create mode 100644 tests/networkxml2co
[libvirt-perl PATCH 3/3] gitlab: add a simple job that publishes the API docs as HTML
The Perl modules have inline POD docs. This can be converted to HTML and published as a GitLab artifact. The rendered HTML is kind of ugly but improving this is left as an exercise for the future. Signed-off-by: Daniel P. Berrangé --- .gitlab-ci.yml | 14 ++ 1 file changed, 14 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 48c7840..32db3e4 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -3,6 +3,7 @@ stages: - prebuild - containers - build + - docs .container_job_template: &container_job_definition image: docker:stable @@ -158,3 +159,16 @@ build-ubuntu-1604: build-ubuntu-1804: <<: *build_job_definition image: ${CI_REGISTRY_IMAGE}/buildenv-ubuntu-1804:latest + +apiref: + stage: docs + image: perl + script: +- perl -MPod::Simple::HTMLBatch -e Pod::Simple::HTMLBatch::go lib apiref + artifacts: +expose_as: 'API Reference' +name: 'apiref' +when: on_success +expire_in: 30 days +paths: + - apiref -- 2.25.3
[libvirt-perl PATCH 0/3] gitlab: introduce CI coverage
This series introduces CI jobs for the Perl binding, enabling the switch over to use of Merge Requests with pre-merge build validation. Daniel P. Berrangé (3): Build: bump min perl to 5.16.0 gitlab: add CI jobs for validating build across platforms gitlab: add a simple job that publishes the API docs as HTML .gitlab-ci.yml| 158 ++ Build.PL | 2 +- ci/libvirt-centos-7.dkr | 12 +++ ci/libvirt-centos-8.dkr | 13 +++ ci/libvirt-debian-10.dkr | 14 +++ ci/libvirt-debian-9.dkr | 14 +++ ci/libvirt-debian-sid.dkr | 14 +++ ci/libvirt-fedora-30.dkr | 13 +++ ci/libvirt-fedora-31.dkr | 13 +++ ci/libvirt-fedora-rawhide.dkr | 13 +++ ci/libvirt-opensuse-151.dkr | 11 +++ ci/libvirt-ubuntu-1604.dkr| 14 +++ ci/libvirt-ubuntu-1804.dkr| 14 +++ ci/refresh| 16 14 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 ci/libvirt-centos-7.dkr create mode 100644 ci/libvirt-centos-8.dkr create mode 100644 ci/libvirt-debian-10.dkr create mode 100644 ci/libvirt-debian-9.dkr create mode 100644 ci/libvirt-debian-sid.dkr create mode 100644 ci/libvirt-fedora-30.dkr create mode 100644 ci/libvirt-fedora-31.dkr create mode 100644 ci/libvirt-fedora-rawhide.dkr create mode 100644 ci/libvirt-opensuse-151.dkr create mode 100644 ci/libvirt-ubuntu-1604.dkr create mode 100644 ci/libvirt-ubuntu-1804.dkr create mode 100755 ci/refresh -- 2.25.3
[libvirt-perl PATCH 2/3] gitlab: add CI jobs for validating build across platforms
This introduces CI jobs that replace the current jobs used on Jenkins for every platform except FreeBSD. A merge request workflow requires the user to fork the primary git repo into their personal namespace. In general the changes need to be tested against the current libvirt git master. If the user has a fork of the main libvirt repo, we don't want to use that by default as it may be out of date. The general goal is that the CI jobs are self-contained and don't depend on the build artifacts from the libvirt repo. We also want to avoid having an explicit dependency on the libvirt-ci repo, or on the Quay.io service. Contributors to the Perl module need to be able to make code changes which imply CI environment changes and be able to test them in isolation. Thus, the dockerfile recipes for each distro are added in the ci/ sub-directory. The first stage of the CI jobs is to use these recipes to build and publish a container image. These images are then used in the second stage to perform the actual build. The container image build is cached, inheriting from both the primary libvirt project namespace, and the user's private project namespace. Thus the performance hit of building container images will only be felt the first time the project is forked, or when the parent Docker images are rebuilt. The dockerfiles were originally generated using lcitool, but if the user makes a change that introduces new build dependencies, the corresponding packages can be added to the dockerfile recipes directly in the same commit. The change can be propagated back into the libvirt-ci.git repo asynchronously. The build job will do a minimal(-ish) build of libvirt git master and then build the rest of the code against that. Ideally the main libvirt configure script would have a way to request a minimal build of just the API and test driver, but for now we settle for just --without-libvirt which culls a large number of the drivers fairly easily. Signed-off-by: Daniel P. Berrangé --- .gitlab-ci.yml| 144 ++ ci/libvirt-centos-7.dkr | 12 +++ ci/libvirt-centos-8.dkr | 13 +++ ci/libvirt-debian-10.dkr | 14 ci/libvirt-debian-9.dkr | 14 ci/libvirt-debian-sid.dkr | 14 ci/libvirt-fedora-30.dkr | 13 +++ ci/libvirt-fedora-31.dkr | 13 +++ ci/libvirt-fedora-rawhide.dkr | 13 +++ ci/libvirt-opensuse-151.dkr | 11 +++ ci/libvirt-ubuntu-1604.dkr| 14 ci/libvirt-ubuntu-1804.dkr| 14 ci/refresh| 16 13 files changed, 305 insertions(+) create mode 100644 ci/libvirt-centos-7.dkr create mode 100644 ci/libvirt-centos-8.dkr create mode 100644 ci/libvirt-debian-10.dkr create mode 100644 ci/libvirt-debian-9.dkr create mode 100644 ci/libvirt-debian-sid.dkr create mode 100644 ci/libvirt-fedora-30.dkr create mode 100644 ci/libvirt-fedora-31.dkr create mode 100644 ci/libvirt-fedora-rawhide.dkr create mode 100644 ci/libvirt-opensuse-151.dkr create mode 100644 ci/libvirt-ubuntu-1604.dkr create mode 100644 ci/libvirt-ubuntu-1804.dkr create mode 100755 ci/refresh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 50dae92..48c7840 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,6 +1,50 @@ stages: - prebuild + - containers + - build + +.container_job_template: &container_job_definition + image: docker:stable + stage: containers + services: +- docker:dind + before_script: +- export TAG="${CI_REGISTRY_IMAGE}/buildenv-${NAME}:latest" +- export COMMON_TAG="${CI_REGISTRY}/libvirt/libvirt-perl/buildenv-${NAME}:latest" +- docker info +- docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD} + script: +- docker pull ${TAG} || docker pull ${COMMON_TAG} || true +- docker build --cache-from ${TAG} --cache-from ${COMMON_TAG} --tag ${TAG} -f ci/libvirt-${NAME}.dkr ci +- docker push ${TAG} + after_script: +- docker logout + +.build_job_template: &build_job_definition + stage: build + before_script: +- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)" +- export SCRATCH_DIR="/tmp/scratch" +- export VROOT="${SCRATCH_DIR}/vroot" +- export LD_LIBRARY_PATH="${VROOT}/lib" +- export PATH="${PATH}:${VROOT}/bin" +- export PKG_CONFIG_PATH="${VROOT}/lib/pkgconfig" +- export TEST_MAINTAINER=1 +- export CONFIGURE_ARGS="--without-libvirtd" + script: +- pushd ${PWD} +- mkdir -p ${SCRATCH_DIR} +- cd ${SCRATCH_DIR} +- git clone --depth 1 https://gitlab.com/libvirt/libvirt.git src +- mkdir build +- cd build +- ../src/autogen.sh --prefix=${VROOT} ${CONFIGURE_ARGS} +- make install +- popd +- perl Build.PL +- perl Build +- perl Build test # Check that all commits are signed-off for the DCO. # Skip on "libvirt" namespace, since we only need to run @@ -14,3 +58,103 @@ check-dco: except: variables: - $CI_PROJECT_NAMESPACE == 'libvirt'
[libvirt-perl PATCH 1/3] Build: bump min perl to 5.16.0
Based on the supported platforms list, the oldest Perl we need to support is from RHEL-7, version 5.16.0 Signed-off-by: Daniel P. Berrangé --- Build.PL | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Build.PL b/Build.PL index ca3c786..7764e41 100755 --- a/Build.PL +++ b/Build.PL @@ -73,7 +73,7 @@ my $b = Module::Build->new( dist_author => 'Daniel Berrange ', dist_abstract => 'libvirt Perl API', requires => { -'perl' => '5.8.0', +'perl' => '5.16.0', }, extra_compiler_flags => $GCC_CFLAGS . $LIBVIRT_CFLAGS, extra_linker_flags => $LIBVIRT_LIBS, -- 2.25.3
[PATCH] networkxml2xmltest: Complete renaming of @actual
In 97a0aa2467 the @actual variable was renamed to @confactual. However, the commit missed non-Linux case resulting in a broken build. Signed-off-by: Michal Privoznik --- Pushed under trivial and build-breaker rules. tests/networkxml2conftest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 5d0bba7924..b35de50f0b 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -58,7 +58,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, "except-interface=lo\n"))) goto fail; VIR_FREE(confactual); -actual = g_steal_pointer(&tmp); +confactual = g_steal_pointer(&tmp); #endif if (virTestCompareToFile(confactual, outconf) < 0) -- 2.26.2
Re: [PATCH v1 0/7] add Spectre related PowerPC features
Ping On 4/16/20 6:24 PM, Daniel Henrique Barboza wrote: Hi, This series implements 3 Spectre related PowerPC features that were added back in QEMU 2.12: - CFPC: Cache Flush on Privilege Change - SBBC: Speculation Barrier Bounds Checking - IBS: Indirect Branch Speculation These options aren't much of a problem for users using latest hardware and guests with recent Linux kernels. Users with outdated hardware/firmware or trying to run AIX guests/guests with older kernels, however, will need to fine tune these options because QEMU defaults won't work. Instead of making users rely on elements to hardcode the options in the XML, let's support them in Libvirt. Daniel Henrique Barboza (7): qemu: Add capability for CFPC pSeries feature qemu: Implement the CFPC pSeries feature qemu: Add capability for SBBC pSeries feature qemu: Implement the SBBC pSeries feature qemu: Add capability for IBS pSeries feature qemu: Implement the IBS pSeries feature news: Update for the recent added pSeries features docs/formatdomain.html.in | 36 + docs/news.xml | 10 ++ docs/schemas/domaincommon.rng | 47 ++ src/conf/domain_conf.c| 134 ++ src/conf/domain_conf.h| 38 + src/libvirt_private.syms | 3 + src/qemu/qemu_capabilities.c | 8 ++ src/qemu/qemu_capabilities.h | 5 + src/qemu/qemu_command.c | 15 ++ src/qemu/qemu_validate.c | 33 + .../caps_2.12.0.ppc64.xml | 3 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 3 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 3 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 3 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 3 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 3 + tests/qemuxml2argvdata/pseries-features.args | 3 +- tests/qemuxml2argvdata/pseries-features.xml | 3 + tests/qemuxml2argvtest.c | 53 ++- tests/qemuxml2xmloutdata/pseries-features.xml | 3 + tests/qemuxml2xmltest.c | 5 +- 21 files changed, 411 insertions(+), 3 deletions(-)
Re: [PATCH 0/4] qemuhotplugtest: Fix timeouts
On 4/23/20 11:40 AM, Peter Krempa wrote: The test takes too long for no good reason. One was caused by the compiler inlining too much and second one was a deliberate decision. Peter Krempa (4): qemuDomainGetUnplugTimeout: Add G_GNUC_NO_INLINE mock:qemuDomainGetUnplugTimeout: Remove unused attribute for '@vm' mock: qemuDomainGetUnplugTimeout: Decrease timeout qemuhotplugtest: detach: Remove commands which are not issued src/qemu/qemu_hotplug.c | 2 +- tests/qemuhotplugmock.c | 6 +++--- tests/qemuhotplugtest.c | 9 +++-- 3 files changed, 7 insertions(+), 10 deletions(-) Reviewed-by: Michal Privoznik Michal
[PATCH 2/4] mock:qemuDomainGetUnplugTimeout: Remove unused attribute for '@vm'
'@vm' is used to use a different timeout for ppc64 guests. Signed-off-by: Peter Krempa --- tests/qemuhotplugmock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c index 8e5b07788d..8fe7fe7448 100644 --- a/tests/qemuhotplugmock.c +++ b/tests/qemuhotplugmock.c @@ -39,7 +39,7 @@ init_syms(void) } unsigned long long -qemuDomainGetUnplugTimeout(virDomainObjPtr vm G_GNUC_UNUSED) +qemuDomainGetUnplugTimeout(virDomainObjPtr vm) { /* Wait only 100ms for DEVICE_DELETED event. Give a greater * timeout in case of PSeries guest to be consistent with the -- 2.26.0
[PATCH 3/4] mock: qemuDomainGetUnplugTimeout: Decrease timeout
We always queue the DEVICE_DELETED events before successful return from the command so that tests are reliable. This means we can decrease the unplug timeout as it's guaranteed to be executed in correct order. According to my testing it shaves off ~450ms of test run: real0m0.721s vs. real0m0.259s Signed-off-by: Peter Krempa --- tests/qemuhotplugmock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c index 8fe7fe7448..d2324913cf 100644 --- a/tests/qemuhotplugmock.c +++ b/tests/qemuhotplugmock.c @@ -45,8 +45,8 @@ qemuDomainGetUnplugTimeout(virDomainObjPtr vm) * timeout in case of PSeries guest to be consistent with the * original logic. */ if (qemuDomainIsPSeries(vm->def)) -return 200; -return 100; +return 20; +return 10; } -- 2.26.0
[PATCH 4/4] qemuhotplugtest: detach: Remove commands which are not issued
The 'human-monitor-command' equates to the 'drive-del' command issued by the hotplug code on successful detach of a device. This means that it's not issued for failed attempts and thus should not be added to the expected list. Unfortunately our test monitor doesn't ensure that all expected commands were consumed. Signed-off-by: Peter Krempa --- tests/qemuhotplugtest.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d9244dca44..9a787f9d11 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -715,8 +715,7 @@ mymain(void) "human-monitor-command", HMP("OK\\r\\n"), "device_add", QMP_OK); DO_TEST_DETACH("base-live", "disk-virtio", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-live", "disk-virtio", false, false, "device_del", QMP_DEVICE_DELETED("virtio-disk4") QMP_OK, "human-monitor-command", HMP("")); @@ -725,8 +724,7 @@ mymain(void) "human-monitor-command", HMP("OK\\r\\n"), "device_add", QMP_OK); DO_TEST_DETACH("base-live", "disk-usb", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-live", "disk-usb", false, false, "device_del", QMP_DEVICE_DELETED("usb-disk16") QMP_OK, "human-monitor-command", HMP("")); @@ -735,8 +733,7 @@ mymain(void) "human-monitor-command", HMP("OK\\r\\n"), "device_add", QMP_OK); DO_TEST_DETACH("base-live", "disk-scsi", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-live", "disk-scsi", false, false, "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK, "human-monitor-command", HMP("")); -- 2.26.0
[PATCH 1/4] qemuDomainGetUnplugTimeout: Add G_GNUC_NO_INLINE
The function is mocked in qemuhotplugmock.so. Recent clang versions decided to inline it so the mock stopped working resulting in qemuhotplugtest wasting 15 seconds waiting for timeouts. Signed-off-by: Peter Krempa --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 14654a17d7..60d0729f1e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5122,7 +5122,7 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm) } -unsigned long long +unsigned long long G_GNUC_NO_INLINE qemuDomainGetUnplugTimeout(virDomainObjPtr vm) { if (qemuDomainIsPSeries(vm->def)) -- 2.26.0
[PATCH 0/4] qemuhotplugtest: Fix timeouts
The test takes too long for no good reason. One was caused by the compiler inlining too much and second one was a deliberate decision. Peter Krempa (4): qemuDomainGetUnplugTimeout: Add G_GNUC_NO_INLINE mock:qemuDomainGetUnplugTimeout: Remove unused attribute for '@vm' mock: qemuDomainGetUnplugTimeout: Decrease timeout qemuhotplugtest: detach: Remove commands which are not issued src/qemu/qemu_hotplug.c | 2 +- tests/qemuhotplugmock.c | 6 +++--- tests/qemuhotplugtest.c | 9 +++-- 3 files changed, 7 insertions(+), 10 deletions(-) -- 2.26.0
Re: [libvirt PATCH 3/5] Remove all usage of virRun
On 4/22/20 6:52 PM, Ján Tomko wrote: Catch the individual usage not removed in previous commits. Signed-off-by: Ján Tomko --- src/lxc/lxc_driver.c | 5 +++-- src/qemu/qemu_domain.c | 16 src/security/security_apparmor.c | 11 +++ src/util/virnetdev.c | 24 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index ca02631f7f..55eb110522 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -203,15 +203,10 @@ load_profile(virSecurityManagerPtr mgr G_GNUC_UNUSED, static int remove_profile(const char *profile) { -int rc = -1; -const char * const argv[] = { -VIRT_AA_HELPER, "-D", "-u", profile, NULL -}; +g_autoptr(virCommand) cmd = virCommandArgList(VIRT_AA_HELPER, "-D", "-u", + profile, NULL); s/virCommandArgList/virCommandNewArgList/ Michal
Re: [libvirt PATCH 0/5] Remove virRun in favor of virCommand
On 4/22/20 6:52 PM, Ján Tomko wrote: Ján Tomko (5): openvz: switch from virRun to virCommand vmware: use virCommand instead of virRun Remove all usage of virRun util: remove references to virRun/virExec util: remove virRun src/libvirt_private.syms | 1 - src/lxc/lxc_driver.c | 5 +- src/openvz/openvz_driver.c | 96 ++-- src/qemu/qemu_domain.c | 16 +++--- src/security/security_apparmor.c | 11 +--- src/util/vircommand.c| 35 src/util/vircommand.h| 2 - src/util/virnetdev.c | 24 +++- src/util/virprocess.c| 4 +- src/vmware/vmware_conf.c | 9 ++- src/vmware/vmware_driver.c | 69 ++- 11 files changed, 98 insertions(+), 174 deletions(-) Reviewed-by: Michal Privoznik Michal
Re: [PATCH v3 0/2] Include lease time option into DHCP settings
On 4/22/20 10:05 PM, Julio Faracco wrote: This series is based on latest series from Nehal. It includes a new entry called under and from scope. This was implemented to include independent lease time for each line and dnsmasq option. So, users are able to define one lease time for ranges and other different for each host entry. The new syntax is simlar with: It will produce a option in dnsmasq configuration file: dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0,14m And some contents into hostsfile: 00:16:3e:77:e2:ed,192.168.122.10,a.example.com,1h This series includes some test cases to cover lease time XML syntax also. Now, each test case requires a hostsfile to test this specific setting. - v1-v2: Change XML syntax according Daniel's suggestion. - v2-v3: Fix memory leak and test dependency issue. Julio Faracco (2): conf: Add option for settings tests: Add tests for to cover dnsmasq settings docs/schemas/basictypes.rng | 8 + docs/schemas/network.rng | 20 +++ src/conf/network_conf.c | 159 +++--- src/conf/network_conf.h | 27 ++- src/libvirt_private.syms | 3 + src/network/bridge_driver.c | 56 +- src/network/bridge_driver.h | 1 + src/test/test_driver.c| 2 +- src/util/virdnsmasq.c | 60 --- src/util/virdnsmasq.h | 3 + src/vbox/vbox_network.c | 16 +- .../dhcp6-nat-network.hostsfile | 7 + .../dhcp6-network.hostsfile | 5 + .../dhcp6host-routed-network.hostsfile| 7 + .../networkxml2confdata/leasetime-hours.conf | 16 ++ .../leasetime-hours.hostsfile | 2 + tests/networkxml2confdata/leasetime-hours.xml | 19 +++ .../leasetime-infinite.conf | 16 ++ .../leasetime-infinite.hostsfile | 2 + .../leasetime-infinite.xml| 19 +++ .../leasetime-minutes.conf| 16 ++ .../leasetime-minutes.hostsfile | 2 + .../networkxml2confdata/leasetime-minutes.xml | 19 +++ .../leasetime-seconds.conf| 16 ++ .../leasetime-seconds.hostsfile | 2 + .../networkxml2confdata/leasetime-seconds.xml | 19 +++ ...t-network-dns-srv-record-minimal.hostsfile | 2 + .../nat-network-dns-srv-record.hostsfile | 2 + .../nat-network-dns-txt-record.hostsfile | 2 + .../nat-network-mtu.hostsfile | 2 + .../nat-network-name-with-quotes.hostsfile| 2 + .../networkxml2confdata/nat-network.hostsfile | 2 + .../ptr-domains-auto.hostsfile| 2 + tests/networkxml2conftest.c | 42 - tests/networkxml2xmlin/leasetime-hours.xml| 19 +++ tests/networkxml2xmlin/leasetime-infinite.xml | 19 +++ tests/networkxml2xmlin/leasetime-minutes.xml | 19 +++ tests/networkxml2xmlin/leasetime-seconds.xml | 19 +++ tests/networkxml2xmlout/leasetime-hours.xml | 21 +++ .../networkxml2xmlout/leasetime-infinite.xml | 21 +++ tests/networkxml2xmlout/leasetime-minutes.xml | 21 +++ tests/networkxml2xmlout/leasetime-seconds.xml | 21 +++ tests/networkxml2xmltest.c| 4 + 43 files changed, 676 insertions(+), 66 deletions(-) create mode 100644 tests/networkxml2confdata/dhcp6-nat-network.hostsfile create mode 100644 tests/networkxml2confdata/dhcp6-network.hostsfile create mode 100644 tests/networkxml2confdata/dhcp6host-routed-network.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-hours.conf create mode 100644 tests/networkxml2confdata/leasetime-hours.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-hours.xml create mode 100644 tests/networkxml2confdata/leasetime-infinite.conf create mode 100644 tests/networkxml2confdata/leasetime-infinite.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-infinite.xml create mode 100644 tests/networkxml2confdata/leasetime-minutes.conf create mode 100644 tests/networkxml2confdata/leasetime-minutes.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-minutes.xml create mode 100644 tests/networkxml2confdata/leasetime-seconds.conf create mode 100644 tests/networkxml2confdata/leasetime-seconds.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-seconds.xml create mode 100644 tests/networkxml2confdata/nat-network-dns-srv-record-minimal.hostsfile create mode 100644 tests/networkxml2confdata/nat-network-dns-srv-record.hostsfile create mode 100644 tests/networkxml2confdata/nat-network-dns-txt-record.hostsfile create mode 100644 tests/networkxml2confdata/nat-network-mtu.hostsfile create mode 100644 tests/networkxml2confdata/nat-ne
Re: [PATCH v3 2/2] tests: Add tests for to cover dnsmasq settings
On 4/22/20 10:05 PM, Julio Faracco wrote: New tests are required to cover some new XML syntax entry for option. This includes schema testing and other features like unit attribute and lease value. This commit includes hostsfile checks adding new files for each test case that is manipulating tag. Signed-off-by: Julio Faracco --- .../dhcp6-nat-network.hostsfile | 7 + .../dhcp6-network.hostsfile | 5 .../dhcp6host-routed-network.hostsfile| 7 + .../networkxml2confdata/leasetime-hours.conf | 16 +++ .../leasetime-hours.hostsfile | 2 ++ tests/networkxml2confdata/leasetime-hours.xml | 19 + .../leasetime-infinite.conf | 16 +++ .../leasetime-infinite.hostsfile | 2 ++ .../leasetime-infinite.xml| 19 + .../leasetime-minutes.conf| 16 +++ .../leasetime-minutes.hostsfile | 2 ++ .../networkxml2confdata/leasetime-minutes.xml | 19 + .../leasetime-seconds.conf| 16 +++ .../leasetime-seconds.hostsfile | 2 ++ .../networkxml2confdata/leasetime-seconds.xml | 19 + ...t-network-dns-srv-record-minimal.hostsfile | 2 ++ .../nat-network-dns-srv-record.hostsfile | 2 ++ .../nat-network-dns-txt-record.hostsfile | 2 ++ .../nat-network-mtu.hostsfile | 2 ++ .../nat-network-name-with-quotes.hostsfile| 2 ++ .../networkxml2confdata/nat-network.hostsfile | 2 ++ .../ptr-domains-auto.hostsfile| 2 ++ tests/networkxml2conftest.c | 27 +-- tests/networkxml2xmlin/leasetime-hours.xml| 19 + tests/networkxml2xmlin/leasetime-infinite.xml | 19 + tests/networkxml2xmlin/leasetime-minutes.xml | 19 + tests/networkxml2xmlin/leasetime-seconds.xml | 19 + tests/networkxml2xmlout/leasetime-hours.xml | 21 +++ .../networkxml2xmlout/leasetime-infinite.xml | 21 +++ tests/networkxml2xmlout/leasetime-minutes.xml | 21 +++ tests/networkxml2xmlout/leasetime-seconds.xml | 21 +++ tests/networkxml2xmltest.c| 4 +++ 32 files changed, 370 insertions(+), 2 deletions(-) create mode 100644 tests/networkxml2confdata/dhcp6-nat-network.hostsfile create mode 100644 tests/networkxml2confdata/dhcp6-network.hostsfile create mode 100644 tests/networkxml2confdata/dhcp6host-routed-network.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-hours.conf create mode 100644 tests/networkxml2confdata/leasetime-hours.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-hours.xml create mode 100644 tests/networkxml2confdata/leasetime-infinite.conf create mode 100644 tests/networkxml2confdata/leasetime-infinite.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-infinite.xml create mode 100644 tests/networkxml2confdata/leasetime-minutes.conf create mode 100644 tests/networkxml2confdata/leasetime-minutes.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-minutes.xml create mode 100644 tests/networkxml2confdata/leasetime-seconds.conf create mode 100644 tests/networkxml2confdata/leasetime-seconds.hostsfile create mode 100644 tests/networkxml2confdata/leasetime-seconds.xml create mode 100644 tests/networkxml2confdata/nat-network-dns-srv-record-minimal.hostsfile create mode 100644 tests/networkxml2confdata/nat-network-dns-srv-record.hostsfile create mode 100644 tests/networkxml2confdata/nat-network-dns-txt-record.hostsfile create mode 100644 tests/networkxml2confdata/nat-network-mtu.hostsfile create mode 100644 tests/networkxml2confdata/nat-network-name-with-quotes.hostsfile create mode 100644 tests/networkxml2confdata/nat-network.hostsfile create mode 100644 tests/networkxml2confdata/ptr-domains-auto.hostsfile create mode 100644 tests/networkxml2xmlin/leasetime-hours.xml create mode 100644 tests/networkxml2xmlin/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlin/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlin/leasetime-seconds.xml create mode 100644 tests/networkxml2xmlout/leasetime-hours.xml create mode 100644 tests/networkxml2xmlout/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlout/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlout/leasetime-seconds.xml I've turned some of these files into symlinks since they are the same. Michal
Re: [PATCH v3 1/2] conf: Add option for settings
On 4/22/20 10:05 PM, Julio Faracco wrote: If an user is trying to configure a dhcp neetwork settings, it is not possible to change the leasetime of a range or a host entry. This is available using dnsmasq extra options, but they are associated with dhcp-range or dhcp-hosts fields. This patch implements a leasetime for range and hosts tags. They can be defined under that settings: Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=913446 Signed-off-by: Julio Faracco --- docs/schemas/basictypes.rng | 8 ++ docs/schemas/network.rng| 20 + src/conf/network_conf.c | 159 +++- src/conf/network_conf.h | 27 +- src/libvirt_private.syms| 3 + src/network/bridge_driver.c | 56 +++-- src/network/bridge_driver.h | 1 + src/test/test_driver.c | 2 +- src/util/virdnsmasq.c | 60 +- src/util/virdnsmasq.h | 3 + src/vbox/vbox_network.c | 16 ++-- tests/networkxml2conftest.c | 15 ++-- 12 files changed, 306 insertions(+), 64 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 81465273c8..271ed18afb 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -607,4 +607,12 @@ + + + seconds + mins + hours Since seconds and hours are specified fully, I think "mins" should be too. It's not any longer than "seconds" anyway :-) + + + diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 60453225d6..88b6f4dfdd 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -371,6 +371,16 @@ + + + + + + + + + + @@ -388,6 +398,16 @@ + + + + + + + + + + diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 819b645df7..286a0edb7c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -70,6 +70,13 @@ VIR_ENUM_IMPL(virNetworkTaint, "hook-script", ); +VIR_ENUM_IMPL(virNetworkDHCPLeaseTimeUnit, + VIR_NETWORK_DHCP_LEASETIME_UNIT_LAST, + "seconds", + "mins", + "hours", +); + static virClassPtr virNetworkXMLOptionClass; static void @@ -132,12 +139,20 @@ virNetworkForwardPfDefClear(virNetworkForwardPfDefPtr def) } +static void +virNetworkDHCPLeaseTimeDefClear(virNetworkDHCPLeaseTimeDefPtr lease) +{ +VIR_FREE(lease); +} + + static void virNetworkDHCPHostDefClear(virNetworkDHCPHostDefPtr def) { VIR_FREE(def->mac); VIR_FREE(def->id); VIR_FREE(def->name); +VIR_FREE(def->lease); } @@ -145,6 +160,9 @@ static void virNetworkIPDefClear(virNetworkIPDefPtr def) { VIR_FREE(def->family); + +while (def->nranges) +virNetworkDHCPLeaseTimeDefClear(def->ranges[--def->nranges].lease); VIR_FREE(def->ranges); while (def->nhosts) @@ -391,11 +409,55 @@ int virNetworkIPDefNetmask(const virNetworkIPDef *def, static int -virSocketAddrRangeParseXML(const char *networkName, - virNetworkIPDefPtr ipdef, - xmlNodePtr node, - virSocketAddrRangePtr range) +virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr *lease, + xmlNodePtr node) +{ +virNetworkDHCPLeaseTimeDefPtr new_lease = *lease; +g_autofree char *expiry = NULL, *unit = NULL; + +if (!(expiry = virXMLPropString(node, "expiry"))) +return 0; + +if (VIR_ALLOC(new_lease) < 0) +return -1; + +if (virStrToLong_ul(expiry, NULL, 10, &new_lease->expiry) < 0) +return -1; + +if (!(unit = virXMLPropString(node, "unit"))) +new_lease->unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES; +else +new_lease->unit = virNetworkDHCPLeaseTimeUnitTypeFromString(unit); Ouch. This is unsafe. Firstly, if an uses specifies say unit="microfornight" TypeFromString() will return -1 because the string is not from the enum. Then, assigning -1 to the virNetworkDHCPLeaseTimeUnitType enu
Re: [all PATCH v2] gitlab: add CI job for validating DCO signoff
On Wed, 2020-04-22 at 18:31 +0100, Daniel P. Berrangé wrote: > +# Check that all commits are signed-off for the DCO. > +# Skip on "libvirt" namespace, since we only need to run > +# this test on developer's personal forks from which > +# merge requests are submitted > +check-dco: > + stage: prebuild > + image: registry.gitlab.com/libvirt/libvirt-ci/check-dco:master > + script: > +- /check-dco > + except: > +variables: > + - $CI_PROJECT_NAMESPACE == 'libvirt' I still don't understand why we can't keep things simple and just run the check everywhere, but I think I'm done arguing :) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] news: Document new Xen hypervisor features
On Wed, 2020-04-22 at 11:55 -0600, Jim Fehlig wrote: > On 4/22/20 11:06 AM, Andrea Bolognani wrote: > > [1] Although it will never be a full rename, because it still need to > > handle libxl:// connection URLs. > > libxl:// was never supported. DV rightly pointed out that introducing > libxl:// > as another way to connect to Xen violated one of libvirt's core principles of > "minimize the change on the application stack as the lower layers of > virtualization evolves". It took me a while to find that bit of history :-) > > https://www.redhat.com/archives/libvir-list/2011-March/msg00449.html > > You'll also notice libxl:// is not mentioned on the Connect URI page > > https://libvirt.org/uri.html I stand corrected! I clearly don't know much about Xen O:-) -- Andrea Bolognani / Red Hat / Virtualization