Re: [libvirt-ci RFC PATCH 00/13] Introduce a new global TOML config file for lcitool

2020-04-23 Thread Erik Skultety
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

2020-04-23 Thread Laine Stump

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

2020-04-23 Thread Laine Stump

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

2020-04-23 Thread Julio Faracco
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

2020-04-23 Thread Fernando Casas Schössow

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

2020-04-23 Thread Pavel Mores
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

2020-04-23 Thread Andrea Bolognani
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

2020-04-23 Thread Eric Blake

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

2020-04-23 Thread Eric Blake

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

2020-04-23 Thread Eric Blake

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

2020-04-23 Thread Eric Blake

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

2020-04-23 Thread Peter Krempa
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'

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Pavel Mores
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

2020-04-23 Thread Julio Faracco
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

2020-04-23 Thread Michal Privoznik

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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Julio Faracco
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

2020-04-23 Thread Bjoern Walk
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

2020-04-23 Thread Bjoern Walk
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

2020-04-23 Thread Bjoern Walk
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

2020-04-23 Thread Bjoern Walk
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

2020-04-23 Thread Bjoern Walk
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

2020-04-23 Thread Julio Faracco
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

2020-04-23 Thread Daniel P . Berrangé
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

2020-04-23 Thread Daniel P . Berrangé
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

2020-04-23 Thread Daniel P . Berrangé
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

2020-04-23 Thread Daniel P . Berrangé
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

2020-04-23 Thread Michal Privoznik
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

2020-04-23 Thread Daniel Henrique Barboza

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

2020-04-23 Thread Michal Privoznik

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'

2020-04-23 Thread Peter Krempa
'@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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Peter Krempa
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

2020-04-23 Thread Michal Privoznik

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

2020-04-23 Thread Michal Privoznik

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

2020-04-23 Thread Michal Privoznik

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

2020-04-23 Thread Michal Privoznik

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

2020-04-23 Thread Michal Privoznik

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

2020-04-23 Thread Andrea Bolognani
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

2020-04-23 Thread Andrea Bolognani
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