March 2019 - Devel - libvirt List Archives

[libvirt] [PATCH] tests: domaincapstest: Fix build on mingw
by Cole Robinson 18 Mar '19

18 Mar '19

fillStringValues is only used if WITH_QEMU || WITH_BHYVE Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- Pushed under build break rule tests/domaincapstest.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 1aa8c023a2..2cccfbc8e2 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -24,6 +24,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#if WITH_QEMU || WITH_BHYVE static int ATTRIBUTE_SENTINEL fillStringValues(virDomainCapsStringValuesPtr values, ...) { @@ -44,6 +45,7 @@ fillStringValues(virDomainCapsStringValuesPtr values, ...) return ret; } +#endif /* WITH_QEMU || WITH_BHYVE */ #if WITH_QEMU # include "testutilsqemu.h" -- 2.21.0

1 0

[libvirt] [PATCH] iptablesSetupPrivateChains: Be forgiving if a table does not exist
by Michal Privoznik 18 Mar '19

18 Mar '19

The way this function works is that for both iptables and ip6tables (or their firewalld friends) and for every table ("filter", "nat", "mangle") it lists chains defined for the table and then calls iptablesPrivateChainCreate() over the list. The callback is then supposed to find libvirt private chains and if not found create rules to add them. So far so good. Problem is if one of the tables doesn't exist (e.g. due to a module missing). For instance, on my system I don't have CONFIG_IP6_NF_MANGLE enabled therefore I'm lacking "mangle" table for ip6tables. This means that the whole operation of setting up private chains fails because the whole transaction is run as "do not ignore errors". The solution is to have two transactions, the first one which just lists chains can run ignoring errors, and the second one which then installs the private chains will run normally. In the code, this approach is pushed to another level - every table for which private chains are created is run as a separate transaction. The reason is that it saves us one more variable where we would track if the second transaction was started already or not; and also, it doesn't matter. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/viriptables.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/util/viriptables.c b/src/util/viriptables.c index d67b640a3b..ea24b03ec8 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -101,6 +101,16 @@ iptablesPrivateChainCreate(virFirewallPtr fw, tmp++; } + /* This function is running in the context of the very first transaction, + * which does nothing more than just lists current tables and chains. But + * since some tables might not be there (e.g. because of a module missing), + * the transaction is run with IGNORE_ERRORS flag. But obviously, we don't + * want to ignore errors here, where we are constructing our own chains and + * rules. The only way to resolve this is to start a new transaction so + * that all those AddRule() calls below add rules to new transaction/group. + */ + virFirewallStartTransaction(fw, 0); + for (i = 0; i < data->nchains; i++) { const char *from; if (!virHashLookup(chains, data->chains[i].child)) { @@ -160,7 +170,7 @@ iptablesSetupPrivateChains(void) fw = virFirewallNew(); - virFirewallStartTransaction(fw, 0); + virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); for (i = 0; i < ARRAY_CARDINALITY(data); i++) virFirewallAddRuleFull(fw, data[i].layer, -- 2.19.2

3 6

[libvirt] [PATCH 0/2] virsh: man: Document quirks of detaching devices
by Peter Krempa 18 Mar '19

18 Mar '19

Peter Krempa (2): virsh: man: Document quirks of device-detach and friends virsh: man: Document asynchronous behaviour of detach-device-alias tools/virsh.pod | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) -- 2.20.1

4 6

[libvirt] [PATCH] tests: Document how to add new replies files
by Andrea Bolognani 18 Mar '19

18 Mar '19

We already document how to generate them, so might as well go the extra mile and document the remaining steps. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- tests/qemucapabilitiestest.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index 16c2832ffb..4c9b7c5322 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -230,6 +230,14 @@ mymain(void) * * If you manually edit replies files you can run * "tests/qemucapsfixreplies foo.replies" to fix the replies ids. + * + * Once a replies file has been generated and tweaked if necessary, + * you can drop it into tests/qemucapabilitiesdata/ (with a sensible + * name - look at what's already there for inspiration) and test + * programs will automatically pick it up. + * + * To generate the corresponding output files after a new replies + * file has been added, run "VIR_TEST_REGENERATE_OUTPUT=1 make check". */ testQemuDataReset(&data); -- 2.20.1

2 1

[libvirt] [PATCH v2 00/16] domcaps: use virTristateBool
by Cole Robinson 18 Mar '19

18 Mar '19

v1 posting: https://www.redhat.com/archives/libvir-list/2019-February/msg01088.html v2 changes: - Rebase to master - Remove the full.xml test in patch #3 - Add virCapsEnum 'format' and use it - Extend docs to explain optional XML v1 cover letter: Extending domaincapabilities with new XML schema is currently a bit of a maintenance pain. Consider the case of adding a new enum for listing <sound> models. I want to output this info for the qemu driver. Internally in the domaincapabilities plumbing, whether a <device> is supported= is tracked with boolean true/false. If I extend that pattern for <sound> devices and fill in data for the qemu driver, the other domcaps implementations will now automatically output a new XML element: <sound supported='no'> Now, for bhyve I can 'git grep' confirm that it doesn't have any <sound> support, but for xen/libxl it _is_ supported. So if I don't fill in accurate support in the xen driver, I've just made their domcaps report blatantly incorrect info. Ideally I would make these <sound> changes and the other drivers output would _not_ change. xen output would now be incomplete, but not obviously wrong, which is easier on me the developer, and safer for the API consumer. This moves domcaps plumbing in that direction. It switches most internal 'supported' fields to virTristateBool so we can track an ABSENT state and map that to outputting no XML. Explicit supported='no' values are filled in where needed to ensure existing driver XML doesn't change. cpu and sev supported= values are left unconverted, but they require semi-special handling anyways so aren't really affected by the problem I laid out above. In v2, I additionally added a mechanism to make <enum> values optionally formatted. Right now whenever a new <enum> is added, if the parent bit is supported (like <disk supported='yes'/>), the new <enum> is automatically formatted as well. This has the same problem described above with the @supported bit. Now drivers are required to set a virCapsPtr.report = true if they want the <enum> to be formatted. Existing drives have this value filled in to maintain back compat. Again, bhyve changes are untested. If someone can give them a spin that would be appreciated, otherwise I will try to get a freebsd build setup. Cole Robinson (16): tests: domcaps: Add a default 'empty' test tests: domcaps: Remove unused typedef tests: domcaps: Remove 'full' test conf: domcaps: Add single line formatting macro conf: domcaps: use virTristateBool for 'supported' qemu: domcaps: fill in explicit supported BOOL_NO libxl: domcaps: fill in explicit supported BOOL_NO bhyve: domcaps: fill in explicit supported BOOL_NO schemas: domcaps: Make more elements optional conf: domcaps: Don't output XML on tristate ABSENT conf: domcaps: Add virCapsEnum 'report' qemu: fill in virCapsEnum 'report' libxl: fill in virCapsEnum 'report' bhyve: fill in virCapsEnum 'report' conf: domcaps: Don't format XML on report=false docs: formatdomaincaps: Describe optional XML changes docs/formatdomaincaps.html.in | 11 +++ docs/schemas/domaincaps.rng | 20 ++++- src/bhyve/bhyve_capabilities.c | 27 ++++-- src/conf/domain_capabilities.c | 31 ++++--- src/conf/domain_capabilities.h | 21 ++--- src/libxl/libxl_capabilities.c | 31 +++++-- src/qemu/qemu_capabilities.c | 41 ++++++--- tests/domaincapsschemadata/empty.xml | 16 ++++ tests/domaincapsschemadata/full.xml | 123 --------------------------- tests/domaincapstest.c | 79 +---------------- 10 files changed, 155 insertions(+), 245 deletions(-) create mode 100644 tests/domaincapsschemadata/empty.xml delete mode 100644 tests/domaincapsschemadata/full.xml -- 2.20.1

2 20

[libvirt] [PATCH] node_device_hal.c: Follow _class -> klass rename
by Michal Privoznik 18 Mar '19

18 Mar '19

In 0eca80e60 _class was renamed to klass for variety of struct members. However, gather_usb_cap() was missed out in this rename leaving FreeBSD build broken. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed under trivial and build-breker rules. src/node_device/node_device_hal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index b77c743305..d1eb6c7851 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -171,7 +171,7 @@ gather_usb_cap(LibHalContext *ctx, const char *udi, (void)get_int_prop(ctx, udi, "usb.interface.number", (int *)&d->usb_if.number); (void)get_int_prop(ctx, udi, "usb.interface.class", - (int *)&d->usb_if._class); + (int *)&d->usb_if.klass); (void)get_int_prop(ctx, udi, "usb.interface.subclass", (int *)&d->usb_if.subclass); (void)get_int_prop(ctx, udi, "usb.interface.protocol", -- 2.19.2

1 0

[libvirt] [PATCH 0/2] storage: fix rbd deprecations in ceph 14
by Daniel P. Berrangé 18 Mar '19

18 Mar '19

Latest ceph has deprecated the rbd_list API and this is present in Fedora 30/31-rawhide Technically this is a build breaker fix, but the patches are complex enough that they would benefit from review before pushing. Daniel P. Berrangé (2): storage: split off code for calling rbd_list storage: add support for new rbd_list2 method m4/virt-storage-rbd.m4 | 1 + src/storage/storage_backend_rbd.c | 116 +++++++++++++++++++++++------- 2 files changed, 92 insertions(+), 25 deletions(-) -- 2.20.1

2 4

[libvirt] [PATCH] Drop needless virtType validation
by Cole Robinson 18 Mar '19

18 Mar '19

This code originates from: commit d0aa10fdd6c108ad442886e4451b2629a3dc8b86 Author: Daniel P. Berrange <berrange(a)redhat.com> Date: Tue Mar 3 12:03:44 2009 +0000 QEMU security driver usage for sVirt support (James Morris, Dan Walsh, Daniel Berrange) Originally in the qemudDomainGetSecurityLabel function. It doesn't appear to have done anything useful back then either. The other two instances look like copy+paste Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/lxc/lxc_driver.c | 7 ------- src/qemu/qemu_driver.c | 14 -------------- 2 files changed, 21 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 760f9f8bdf..e981f8e901 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1268,13 +1268,6 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla if (virDomainGetSecurityLabelEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (!virDomainVirtTypeToString(vm->def->virtType)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown virt type in domain definition '%d'"), - vm->def->virtType); - goto cleanup; - } - /* * Theoretically, the pid can be replaced during this operation and * return the label of a different process. If atomicity is needed, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3615270650..a16eab5467 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6445,13 +6445,6 @@ static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secl if (virDomainGetSecurityLabelEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (!virDomainVirtTypeToString(vm->def->virtType)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown virt type in domain definition '%d'"), - vm->def->virtType); - goto cleanup; - } - /* * Theoretically, the pid can be replaced during this operation and * return the label of a different process. If atomicity is needed, @@ -6493,13 +6486,6 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom, if (virDomainGetSecurityLabelListEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (!virDomainVirtTypeToString(vm->def->virtType)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown virt type in domain definition '%d'"), - vm->def->virtType); - goto cleanup; - } - /* * Check the comment in qemuDomainGetSecurityLabel function. */ -- 2.21.0

2 1

[libvirt] [PATCH] qemu: Compare group_names by STRNEQ not CHECK_EQ
by Han Han 18 Mar '19

18 Mar '19

Fix issue introduced by 047cfb05ee. Since group_name is str, use STRNEQ instead of CHECK_EQ to do comparition. Signed-off-by: Han Han <hhan(a)redhat.com> --- src/qemu/qemu_domain.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86e80391e1..e6d0fbef04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9387,9 +9387,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, CHECK_EQ(blkdeviotune.size_iops_sec, "blkdeviotune size_iops_sec", true); - CHECK_EQ(blkdeviotune.group_name, - "blkdeviotune group_name", - true); + if (disk->blkdeviotune.group_name) { + if (STRNEQ(disk->blkdeviotune.group_name, orig_disk->blkdeviotune.group_name)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot modify field '%s' of the disk"), + "target"); + return false; + } + } if (disk->serial && STRNEQ_NULLABLE(disk->serial, orig_disk->serial)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, -- 2.20.1

3 2

[libvirt] [PATCH v6 0/8] Incremental backups: virDomainSnapshot class
by Eric Blake 18 Mar '19

18 Mar '19

This is a respin of patch 4/20 in my larger series: https://www.redhat.com/archives/libvir-list/2019-March/msg00386.html As John pointed out, my v4/v5 posting had a LOT of duplicate code, including lots of poorly commented code managing a potential tree of checkpoints (even if the more immediate use tends to be a linear chain than a full-blown tree), all because I had copied-and-pasted from snapshot code. Copy-and-paste is technical debt; better is to get rid of the debt by refactoring the code for easy reuse. Here's as far as I got today, but hopefully reviewers agree that this glimpse of where things are headed is worthwhile, and I can start pushing these patches while continuing my cleanup work on the rest of the incremental backup stuff. Pre-series, virDomainSnapshotList() code in snapshot_conf.h can only compute a list of virDomainSnapshotPtrs; once this series is applied, that function will instead be changed to compute a list of virDomainMomentPtrs (which is then trivially cast to virDomainSnapshotPtr or virDomainCheckpointPtr as needed), so that all the code related to maintaining relationships between points in time can be shared rather than duplicated between snapshots and checkpoints. Eric Blake (8): snapshot: Split domain forward typedefs into new file snapshot: Sort virconftypes.h snapshot: Break out virDomainSnapshotObj into its own file snapshot: Export two functions prior to file split snapshot: Break out virDomainSnapshotObjList into its own file snapshot: Use accessors for virDomainSnapshot members snapshot: Create virDomainMoment base class backup: Introduce virDomainCheckpointPtr include/libvirt/virterror.h | 6 +- src/util/virerror.c | 12 +- include/libvirt/libvirt.h | 6 +- src/conf/domain_conf.h | 220 +--------- src/conf/snapshot_conf.h | 74 +--- src/conf/virconftypes.h | 281 +++++++++++++ src/conf/virdomainsnapshotobj.h | 50 +++ src/conf/virdomainsnapshotobjlist.h | 74 ++++ src/datatypes.h | 79 +++- src/conf/Makefile.inc.am | 5 + src/conf/domain_conf.c | 1 + src/conf/snapshot_conf.c | 608 +--------------------------- src/conf/virdomainobjlist.c | 1 + src/conf/virdomainsnapshotobj.c | 123 ++++++ src/conf/virdomainsnapshotobjlist.c | 553 +++++++++++++++++++++++++ src/datatypes.c | 132 ++++-- src/esx/esx_driver.c | 66 +-- src/libvirt-domain-snapshot.c | 26 +- src/libvirt_private.syms | 38 +- src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_driver.c | 41 +- src/qemu/qemu_migration.c | 1 + src/remote/remote_daemon_dispatch.c | 4 +- src/remote/remote_driver.c | 4 +- src/rpc/gendispatch.pl | 2 +- src/test/test_driver.c | 21 +- src/vbox/vbox_common.c | 56 +-- src/vz/vz_driver.c | 52 +-- 29 files changed, 1450 insertions(+), 1088 deletions(-) create mode 100644 src/conf/virconftypes.h create mode 100644 src/conf/virdomainsnapshotobj.h create mode 100644 src/conf/virdomainsnapshotobjlist.h create mode 100644 src/conf/virdomainsnapshotobj.c create mode 100644 src/conf/virdomainsnapshotobjlist.c -- 2.20.1

6 35