- Devel - libvirt List Archives

[Question] sg_persist failed when using qemu-pr-helper
by Zihao Chang 17 Nov '20

17 Nov '20

Hi all, When using qemu-pr-helper，"sg_persist" in guest return Aborted Command after iscsiadm logout & login. My confusion: Is this problem caused by bugs of libvirt/qemu-pr-helper? Or iscsiadm logout & login should be forbidden when using qemu-pr-helper? Or There is something wrong with my config. Steps to Reproduce: 1. setup multipath env 1) set /etc/iscsi/initiatorname.iscsi 2) iscsiadm discovery & login 3) start multipathd 2. add the following xml to guest <disk type='block' device='lun'> <driver name='qemu' type='raw' cache='none' io='native'/> <source dev='/dev/mapper/XXXXXXXXXXXXXXXXX'> **<reservations managed='yes'/>** </source> <target dev='sdc' bus='scsi'/> </disk> 3. start the guest 1) "sg_persist -v -i -n -k -d /dev/sdc" works well in the guest 2) here /dev/sdc(guest) -> /dev/sdht(host) 4. iscsiadm logout & login on host 1) iscsiadm -m node -u (host) 2) iscsiadm -m node -l (host) 3) here /dev/sdc(guest) -(change)-> /dev/sdfo(host) 5. exec "sg_persist -v -i -n -k -d /dev/sdc" in the guest 1) command return: Persistent Reservation In cmd: 5e 00 00 00 00 00 00 20 00 00 persistent reservation in: Fixed format, current; Sense key: Aborted Command Additional sense: Logical unit communication failure PR in (Read keys): Aborted command sense key, other than protection related (asc=0x10) 2) strace qemu-pr-helper show: openat(AT_FDCWD, "/dev/sdht", O_RDONLY) = -1 ENOENT (No such file or directory) Additional info: 1. strace qemu-pr-helper show: openat(AT_FDCWD, "/dev/sdht", O_RDONLY) = -1 ENOENT (No such file or directory) 2. This problem may be related to mnt(namespace). I try to set mnt of qemu-pr-helper as pid 1, sg_persist works well after logout & login.

2 3

[libvirt PATCH 0/3] Define and enable Snowridge model
by Tim Wiederhake 17 Nov '20

17 Nov '20

This series adds a CPU model for Snowridge to libvirt. Snowridge was added to QEMU in v4.1.0. Tim Wiederhake (3): cpu_map: Add support for core-capability CPU feature cpu_map: Add support for split-lock-detect CPU feature cpu_map: Define and enable Snowridge model src/cpu_map/index.xml | 1 + src/cpu_map/meson.build | 1 + src/cpu_map/x86_Snowridge.xml | 71 +++++++++++++++++++ src/cpu_map/x86_features.xml | 8 +++ .../domaincapsdata/qemu_4.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.x86_64.xml | 1 + .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.x86_64.xml | 1 + .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_5.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_5.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.x86_64.xml | 1 + 19 files changed, 96 insertions(+) create mode 100644 src/cpu_map/x86_Snowridge.xml -- 2.26.2

1 3

[libvirt PATCH v3] Do not disable incompatible-pointer-types-discards-qualifiers
by Martin Kletzander 17 Nov '20

17 Nov '20

This reverts commit b3710e9a2af402a2b620de570b062294e11190eb. That check is very valuable for our code, but it causes issue with glib >= 2.67.0 when building with clang. The reason is a combination of two commits in glib, firstly fdda405b6b1b which adds a g_atomic_pointer_{set,get} variants that enforce stricter type checking (by removing an extra cast) for compilers that support __typeof__, and commit dce24dc4492d which effectively enabled the new variant of glib's atomic code for clang. This will not be necessary when glib's issue #600 [0] (8 years old) is fixed. Thankfully, MR #1719 [1], which is supposed to deal with this issue was opened 3 weeks ago, so there is a slight sliver of hope. [0] https://gitlab.gnome.org/GNOME/glib/-/issues/600 [1] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- v3: - added check for glib version v2: - override _G_DEFINE_TYPE_EXTENDED_BEGIN in gcompat.h meson.build | 3 --- src/util/glibcompat.h | 24 ++++++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index cecaad199d4c..04646e3a078c 100644 --- a/meson.build +++ b/meson.build @@ -405,9 +405,6 @@ cc_flags += [ # so use this Clang-specific arg to keep it quiet '-Wno-typedef-redefinition', - # Clang complains about casts in G_DEFINE_TYPE(...) - '-Wno-incompatible-pointer-types-discards-qualifiers', - # We don't use -Wc++-compat so we have to enable it explicitly '-Wjump-misses-init', diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h index 6f50a76f3c2b..457f6ba797f3 100644 --- a/src/util/glibcompat.h +++ b/src/util/glibcompat.h @@ -20,6 +20,30 @@ #include <glib.h> #include <glib/gstdio.h> +#include <glib-object.h> + +#if defined(__clang__) && GLIB_CHECK_VERSION(2, 67, 0) +/* + * Clang detects (valid) issue in G_DEFINE_TYPE and derivatives starting with + * glib >= 2.67.0. See https://gitlab.gnome.org/GNOME/glib/-/issues/600 + * + * For that we need to disable the one check that produces an error in our + * builds when using any G_DEFINE_TYPE* macro. Thankfully all those macros end + * up using _G_DEFINE_TYPE_EXTENDED_BEGIN. Because with that we can redefine + * this one macro to cover all use cases. The macro is defined the same way it + * is defined in glib (with a very low probability of being changed thanks to a + * comment above it). + */ +# undef _G_DEFINE_TYPE_EXTENDED_BEGIN + +# define _G_DEFINE_TYPE_EXTENDED_BEGIN(TypeName, type_name, TYPE_PARENT, flags) \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wincompatible-pointer-types-discards-qualifiers\"") \ + _G_DEFINE_TYPE_EXTENDED_BEGIN_PRE(TypeName, type_name, TYPE_PARENT) \ + _G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER(TypeName, type_name, TYPE_PARENT, flags) \ + _Pragma("GCC diagnostic pop") + +#endif /* __clang__ */ gchar * vir_g_canonicalize_filename(const gchar *filename, const gchar *relative_to); -- 2.29.2

2 1

[libvirt] improve security by adjusting the privileges of libvirtd processes
by yebiaoxiang 17 Nov '20

17 Nov '20

Hi Team The daemon libvirtd runs as root user, which against the least privilege security model. root 567642 1.2 0.0 2856020 47576 ? Ssl 15:49 0:02 /usr/sbin/libvirtd --listen In addition, the "--listen" parameter exposes TCP or TLS ports on the network, it increasing the attack surface. tcp 0 0 0.0.0.0:16509 0.0.0.0:* LISTEN 647824/libvirtd tcp 0 0 0.0.0.0:16514 0.0.0.0:* LISTEN 647824/libvirtd I have the following puzzles: 1. Whether root is the least privilege required for libvirtd to manage virtualization platforms, it's possible to run libvirtd as a non-root user? 2. Is there any plan to resolve this security weaknesses? (like move the function of "--listen" to an independent non-root process, or other good idea) Regards, BiaoXiang

2 2

nwfilter issue with new ebtables
by Christian Ehrhardt 17 Nov '20

17 Nov '20

Hi, I have last week discussed breakage in nwfilter usage on IRC <filterref filter='clean-traffic'> <parameter name='CTRL_IP_LEARNING' value='dhcp'/> </filterref> virsh start <guest> error: Failed to start domain <guest> error: internal error: applyDHCPOnlyRules failed - spoofing not protect With debug in the logs enabled I got confirmation by Daniel (thanks!) that the command sequence libvirt issued looked kind of "normal". Hereby I wanted to let you know that some further debugging identified a part of the sequence that libvirt issues as being broken in recent ebtables versions. # ebtables --concurrent -t nat -N testrule3 # ebtables --concurrent -t nat -E testrule3 testrule3-renamed ebtables v1.8.6 (nf_tables): Chain 'testrule3' doesn't exists This led to upstream ebtables bug [1] - for now just FYI in case you want/need to subscribe for your own tracking. [1]: https://bugzilla.netfilter.org/show_bug.cgi?id=1481 -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

3 4

[libvirt PATCH v2] qemu_command: fix FD usage in qemuBuildInterfaceCommandLine
by Pavel Hrdina 17 Nov '20

17 Nov '20

If virCommandPassFD() is called with VIR_COMMAND_PASS_FD_CLOSE_PARENT the passed FD is closed. If qemuVirCommandGetFDSet() returns NULL we would jump to cleanup where the vdpafd is closed again if not set to -1. In order to fix it use a different variable where we will copy FD number. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/qemu/qemu_command.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0eec35da16..2a786e34dd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8141,18 +8141,20 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, if (vdpafd > 0) { g_autofree char *fdset = NULL; g_autofree char *addfdarg = NULL; + int vdpafd_copy = vdpafd; virCommandPassFD(cmd, vdpafd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); - fdset = qemuVirCommandGetFDSet(cmd, vdpafd); + vdpafd = -1; + + fdset = qemuVirCommandGetFDSet(cmd, vdpafd_copy); if (!fdset) goto cleanup; - vdpafdName = qemuVirCommandGetDevSet(cmd, vdpafd); + vdpafdName = qemuVirCommandGetDevSet(cmd, vdpafd_copy); /* set opaque to the devicepath so that we can look up the fdset later * if necessary */ addfdarg = g_strdup_printf("%s,opaque=%s", fdset, net->data.vdpa.devicepath); virCommandAddArgList(cmd, "-add-fd", addfdarg, NULL); - vdpafd = -1; } if (chardev) -- 2.26.2

1 0

[libvirt PATCH v2] Do not disable incompatible-pointer-types-discards-qualifiers
by Martin Kletzander 17 Nov '20

17 Nov '20

This reverts commit b3710e9a2af402a2b620de570b062294e11190eb. That check is very valuable for our code, but it causes issue with glib >= 2.67.0 when building with clang. The reason is a combination of two commits in glib, firstly fdda405b6b1b which adds a g_atomic_pointer_{set,get} variants that enforce stricter type checking (by removing an extra cast) for compilers that support __typeof__, and commit dce24dc4492d which effectively enabled the new variant of glib's atomic code for clang. This will not be necessary when glib's issue #600 [0] (8 years old) is fixed. Thankfully, MR #1719 [1], which is supposed to deal with this issue was opened 3 weeks ago, so there is a slight sliver of hope. [0] https://gitlab.gnome.org/GNOME/glib/-/issues/600 [1] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- meson.build | 3 --- src/util/glibcompat.h | 24 ++++++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index cecaad199d4c..04646e3a078c 100644 --- a/meson.build +++ b/meson.build @@ -405,9 +405,6 @@ cc_flags += [ # so use this Clang-specific arg to keep it quiet '-Wno-typedef-redefinition', - # Clang complains about casts in G_DEFINE_TYPE(...) - '-Wno-incompatible-pointer-types-discards-qualifiers', - # We don't use -Wc++-compat so we have to enable it explicitly '-Wjump-misses-init', diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h index 6f50a76f3c2b..f4867f3eadbc 100644 --- a/src/util/glibcompat.h +++ b/src/util/glibcompat.h @@ -20,6 +20,30 @@ #include <glib.h> #include <glib/gstdio.h> +#include <glib-object.h> + +#ifdef __clang__ +/* + * Clang detects (valid) issue in G_DEFINE_TYPE and derivatives starting with + * glib >= 2.67.0. See https://gitlab.gnome.org/GNOME/glib/-/issues/600 + * + * For that we need to disable the one check that produces an error in our + * builds when using any G_DEFINE_TYPE* macro. Thankfully all those macros end + * up using _G_DEFINE_TYPE_EXTENDED_BEGIN. Because with that we can redefine + * this one macro to cover all use cases. The macro is defined the same way it + * is defined in glib (with a very low probability of being changed thanks to a + * comment above it). + */ +# undef _G_DEFINE_TYPE_EXTENDED_BEGIN + +# define _G_DEFINE_TYPE_EXTENDED_BEGIN(TypeName, type_name, TYPE_PARENT, flags) \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wincompatible-pointer-types-discards-qualifiers\"") \ + _G_DEFINE_TYPE_EXTENDED_BEGIN_PRE(TypeName, type_name, TYPE_PARENT) \ + _G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER(TypeName, type_name, TYPE_PARENT, flags) \ + _Pragma("GCC diagnostic pop") + +#endif /* __clang__ */ gchar * vir_g_canonicalize_filename(const gchar *filename, const gchar *relative_to); -- 2.29.2

2 1

[PATCH 1/1] qemu_tmp.c: fix 'shortName' leak
by Daniel Henrique Barboza 17 Nov '20

17 Nov '20

This is a Coverity fix pointed out by John in IRC. This code was introduced in 19d74fdf0eb, when the TPM Proxy device for for ppc64 was introduced. This will leak in case we have 2 TPMs in the same domain, a possible scenario with the protected Ultravisor execution in PowerPC guests. Fixes: 19d74fdf0eb5d2e89e80ceedea736425160ffccb Reported-by: John Ferlan <jferlan(a)redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com> --- src/qemu/qemu_tpm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 8adb0e42b8..872be16570 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -841,10 +841,11 @@ qemuExtTPMStop(virQEMUDriverPtr driver, virDomainObjPtr vm) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - g_autofree char *shortName = NULL; size_t i; for (i = 0; i < vm->def->ntpms; i++) { + g_autofree char *shortName = NULL; + if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; @@ -866,12 +867,13 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver, virCgroupPtr cgroup) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - g_autofree char *shortName = NULL; int rc; pid_t pid; size_t i; for (i = 0; i < def->ntpms; i++) { + g_autofree char *shortName = NULL; + if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; -- 2.26.2

2 1

[libvirt PATCH 0/4] Misc version updates
by Daniel P. Berrangé 16 Nov '20

16 Nov '20

In both yajl and wireshark we can bump our min version per our platform support matrix and thus eliminate some historical cruft. Daniel P. Berrangé (4): meson: assume pkg-config support for yajl meson: bump min wireshark to 2.6.0 rpm: remove version checks for wireshark rpm: enable wireshark build for RHEL-8 and later libvirt.spec.in | 8 +++---- meson.build | 63 ++++++++++++++----------------------------------- 2 files changed, 22 insertions(+), 49 deletions(-) -- 2.28.0

2 5

[libvirt PATCH] Do not disable incompatible-pointer-types-discards-qualifiers
by Martin Kletzander 16 Nov '20

16 Nov '20

This reverts commit b3710e9a2af402a2b620de570b062294e11190eb. That check is very valuable for our code, but it causes issue with glib >= 2.67.0 when building with clang. The reason is a combination of two commits in glib, firstly fdda405b6b1b which adds a g_atomic_pointer_{set,get} variants that enforce stricter type checking (by removing an extra cast) for compilers that support __typeof__, and commit dce24dc4492d which effectively enabled the new variant of glib's atomic code for clang. This will not be necessary when glib's issue #600 [0] (8 years old) is fixed. Thankfully, MR #1719 [1], which is supposed to deal with this issue was opened 3 weeks ago, so there is a slight sliver of hope. [0] https://gitlab.gnome.org/GNOME/glib/-/issues/600 [1] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- meson.build | 3 --- src/qemu/qemu_domain.c | 7 +++++++ src/util/vireventthread.c | 6 ++++++ src/util/viridentity.c | 6 ++++++ src/util/virobject.c | 6 ++++++ 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index cecaad199d4c..04646e3a078c 100644 --- a/meson.build +++ b/meson.build @@ -405,9 +405,6 @@ cc_flags += [ # so use this Clang-specific arg to keep it quiet '-Wno-typedef-redefinition', - # Clang complains about casts in G_DEFINE_TYPE(...) - '-Wno-incompatible-pointer-types-discards-qualifiers', - # We don't use -Wc++-compat so we have to enable it explicitly '-Wjump-misses-init', diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2158080a56ad..3349476cceae 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -330,7 +330,14 @@ struct _qemuDomainLogContext { virLogManagerPtr manager; }; +#pragma clang diagnostic push +/* Workaround for glib's issue #600 on clang */ +#pragma clang diagnostic ignored "-Wincompatible-pointer-types-discards-qualifiers" + G_DEFINE_TYPE(qemuDomainLogContext, qemu_domain_log_context, G_TYPE_OBJECT); + +#pragma clang diagnostic pop + static virClassPtr qemuDomainSaveCookieClass; static void qemuDomainLogContextFinalize(GObject *obj); diff --git a/src/util/vireventthread.c b/src/util/vireventthread.c index 8342f420f666..4e202c94d0f0 100644 --- a/src/util/vireventthread.c +++ b/src/util/vireventthread.c @@ -32,8 +32,14 @@ struct _virEventThread { GMainLoop *loop; }; +#pragma clang diagnostic push +/* Workaround for glib's issue #600 on clang */ +#pragma clang diagnostic ignored "-Wincompatible-pointer-types-discards-qualifiers" + G_DEFINE_TYPE(virEventThread, vir_event_thread, G_TYPE_OBJECT) +#pragma clang diagnostic pop + #define VIR_FROM_THIS VIR_FROM_EVENT static void diff --git a/src/util/viridentity.c b/src/util/viridentity.c index 2cb9042a846a..43f447f629fe 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -50,8 +50,14 @@ struct _virIdentity { virTypedParameterPtr params; }; +#pragma clang diagnostic push +/* Workaround for glib's issue #600 on clang */ +#pragma clang diagnostic ignored "-Wincompatible-pointer-types-discards-qualifiers" + G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT) +#pragma clang diagnostic pop + static virThreadLocal virIdentityCurrent; static void virIdentityFinalize(GObject *obj); diff --git a/src/util/virobject.c b/src/util/virobject.c index a6305354c35e..8d1f26618334 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -53,8 +53,14 @@ struct _virObjectPrivate { }; +#pragma clang diagnostic push +/* Workaround for glib's issue #600 on clang */ +#pragma clang diagnostic ignored "-Wincompatible-pointer-types-discards-qualifiers" + G_DEFINE_TYPE_WITH_PRIVATE(virObject, vir_object, G_TYPE_OBJECT) +#pragma clang diagnostic pop + #define VIR_OBJECT_NOTVALID(obj) (!obj || !VIR_IS_OBJECT(obj)) #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass) \ -- 2.29.2

4 10

[libvirt PATCH] news: Fix vertical and horizontal spacing
by Andrea Bolognani 16 Nov '20

16 Nov '20

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Pushed as trivial. NEWS.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/NEWS.rst b/NEWS.rst index f4fc407b77..98303432f0 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -33,6 +33,7 @@ v6.10.0 (unreleased) This drops support for Windows Server 2008R2 and 2012. The earliest supported version is now Windows 2012R2. + v6.9.0 (2020-11-02) =================== @@ -165,17 +166,17 @@ v6.8.0 (2020-10-01) * qemu: Preserve qcow2 cluster size after external snapshots - The new overlay image which is installed on top of the current chain when - taking an external snapshot now preserves the cluser size of the original - top image to preserve any performance tuning done on the original image. + The new overlay image which is installed on top of the current chain when + taking an external snapshot now preserves the cluser size of the original + top image to preserve any performance tuning done on the original image. * **Bug fixes** * qemu: Various (i)SCSI backed hostdev fixes - (i)SCSI backed hostdevs now work again with an arbitrarily long - user-specified device alias and also honor the 'readonly' property after a - recent rewrite. + (i)SCSI backed hostdevs now work again with an arbitrarily long + user-specified device alias and also honor the 'readonly' property after a + recent rewrite. * **Removed features** -- 2.26.2

1 0

[PATCH] News: Several apparmor improvements at v6.7.0[v3]
by jgao 16 Nov '20

16 Nov '20

From: Jianan Gao <jgao(a)redhat.com> Add news about apparmor about the improvements. Signed-off-by: Jianan Gao <jgao(a)redhat.com> --- Clarified news content and revised commit information --- NEWS.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 905deba5a8..fd3254d47f 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -224,6 +224,11 @@ v6.7.0 (2020-09-01) forbidden and no size auto-alignment will be made. Instead, libvirt will suggest an aligned round up size for the user. + * apparmor: Several improvements + + Add support for virtiofs filesystem and allow qemu load old shared objects after + upgrade. + * **Bug fixes** * virdevmapper: Deal with kernels without DM support -- 2.21.3

2 1

Issues 14 and 90 Contribution Details
by Dustan B Helm 16 Nov '20

16 Nov '20

Hey Libvirt team. We are looking into issue 14 <https://gitlab.com/libvirt/libvirt/-/issues/14> and issue 90 <https://gitlab.com/libvirt/libvirt/-/issues/90> to make possible contributions to one or both of them. However, before we start preparing patches for either of these issues, we have some clarifying questions about the scope of the necessary changes. Issue 14: Mostly we wanted to know what was wrong with Guannan's previous attempt at patching this. Was it incorrect, incomplete, or otherwise lacking? We also were confused on how Peter's usb-audio commit actually addressed an issue. It seems like most of the changes were documentation and constant and only a few changes changed significant amounts of code. We understand that QEMU is the program actually handling the sound card, but how do Peter's changes actually hook up the sound card with QEMU? Would the changes needed for the usb-net device be the same as Peter's usb-audio changes except in different sections of each file, or would the changes need to be made elsewhere? We saw that Peter and Guannan both had changes to tests/qemuhelptest.c and src/qemu/qemu_capabilities.c (to name a few). Issue 90:In order to implement a new network disk protocol, we believe there should be changes to the parameters in virsh/attach-disk, but we also know that there will likely be changes elsewhere in the codebase to allow support for a new protocol type. Ex: diskSourceNetworkProtocol definitions in docs/schemas/domaincommon.rng. Is there documentation available on what files/sections of the codebase handle supporting network disk protocols, so we can find more reference examples and begin to make changes in the appropriate locations? Thanks in advance, Dustan Helm, Ryan Gahagan, Barrett Schonefeld

1 0

[PATCH] News: Several apparmor improvements at v6.7.0
by jgao 14 Nov '20

14 Nov '20

From: jgao <jgao(a)redhat.com> Add news about apparmor about the improvements. Signed-off-by: jgao <jgao(a)redhat.com> --- NEWS.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 905deba5a8..30a4f1065a 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -224,6 +224,11 @@ v6.7.0 (2020-09-01) forbidden and no size auto-alignment will be made. Instead, libvirt will suggest an aligned round up size for the user. + * apparmor: Several improvements + + Add support for virtiofs filesystem, allowing qemu load old shared objects after + upgrades. + * **Bug fixes** * virdevmapper: Deal with kernels without DM support -- 2.21.3

4 3

[PATCH] util: remove ATTRIBUTE_NONNULL from virDirClose declaration
by Laine Stump 14 Nov '20

14 Nov '20

Before commit 24d8968c, virDirClose took a DIR**, and that was never NULL, so its declaration included ATTRIBUTE_NONNULL(1). Since that commit, virDirClose takes a DIR*, and it may be NULL (e.g. if the DIR* is initialized to NULL and was never closed). Even though virDirClose() is currently only called implicitly (as the cleanup for a g_autoptr(DIR)), and (as I've just newly learned) the autocleanup function g_autoptr will only be called if the pointer in question is non-null (see the definition of _GLIB_AUTOPTR_CLEAR_FUNC_NAME in /usr/include/glib-2.0/glib/gmacros.h), it does still cause Coverity to complain that it *could* be called with a NULL, and it's also possible that in the future someone might add code that explicitly calls virDirClose. To eliminate the Coverity complaints, and protect against the hypothetical future where someone both explicitly calls virDirClose() with a potentially NULL value, *and* re-enables the nonnull directive when not building with Coverity (disabled by commit eefb881) this patch removes the ATTRIBUTE_NONNULL(1) from the declaration of virDirClose(). Fixes: 24d8968cd0a718af4badbbc858b1b449fea7205a Details-Research-by: Dan Berrange <berrange(a)redhat.com> Signed-off-by: Laine Stump <laine(a)redhat.com> --- src/util/virfile.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virfile.h b/src/util/virfile.h index ba31a78e58..32505826ee 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -269,8 +269,7 @@ int virDirOpenQuiet(DIR **dirp, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; -void virDirClose(DIR *dirp) - ATTRIBUTE_NONNULL(1); +void virDirClose(DIR *dirp); G_DEFINE_AUTOPTR_CLEANUP_FUNC(DIR, virDirClose); int virFileMakePath(const char *path) G_GNUC_WARN_UNUSED_RESULT; -- 2.28.0

1 1

[PATCH 1/1] qemu_driver.c: do not redefine 'event' in qemuDomainDefineXMLFlags()
by Daniel Henrique Barboza 14 Nov '20

14 Nov '20

A bad merge while rebasing 74b2834333a caused the @event variable to be defined twice, inside the 'cleanup' label, causing coverity errors. This code was originally moved outside of the label by commit 773c7c43611a. Delete the unintended code in the 'cleanup' label. Fixes: 74b2834333ab3bf500f870e0a6d4e8309379d96a Reported-by: John Ferlan <jferlan(a)redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com> --- src/qemu/qemu_driver.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78ccf96df5..ac38edf009 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6748,15 +6748,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, } } - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_DEFINED, - !oldDef ? - VIR_DOMAIN_EVENT_DEFINED_ADDED : - VIR_DOMAIN_EVENT_DEFINED_UPDATED); - - VIR_INFO("Creating domain '%s'", vm->def->name); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); - virDomainObjEndAPI(&vm); virObjectEventStateQueue(driver->domainEventState, event); return dom; -- 2.26.2

2 1

[PATCH v1 00/10] more simple g_autoptr() cleanups
by Daniel Henrique Barboza 14 Nov '20

14 Nov '20

I intended to clean up the virDomainDefFree() calls that I kept seeing in qemu_driver.c, ended up doing a bit more than that. Daniel Henrique Barboza (10): qemu_driver.c: use g_autoptr() with virDomainDef pointers qemu_driver.c: use g_autoptr() with qemuMigrationParams pointers qemu_driver.c: use g_autoptr() with virDomainDeviceDefPtr qemu_driver.c: remove unneeded 'cleanup' labels qemu_snapshot.c: remove uneeded 'cleanup' label in qemuSnapshotDelete() qemu_domain.c: use g_autoptr() with virDomainDef pointers qemu_domain.c: remove unneeded cleanup labels qemu_domain.c: modernize qemuDomainFixupCPUs() qemu_domain.c: modernize qemuDomainWriteMasterKeyFile() qemu_domain.c: modernize qemuMonitorGetCpuHalted() src/qemu/qemu_domain.c | 100 ++++++------------ src/qemu/qemu_driver.c | 219 +++++++++++++++------------------------ src/qemu/qemu_snapshot.c | 3 +- 3 files changed, 119 insertions(+), 203 deletions(-) -- 2.26.2

3 14

[PATCH] virnetdevopenvswitch: Fix ATTRIBUTE_NONNULL() tag for virNetDevOpenvswitchGetVhostuserIfname()
by Michal Privoznik 14 Nov '20

14 Nov '20

After e4c29e2904 the function has one argument more and the argument that can't be NULL moved from second to third position. Reported-by: John Ferlan <jferlan(a)redhat.com> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed as trivial. src/util/virnetdevopenvswitch.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 5cd1d22ae3..8409aa92ac 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -63,7 +63,7 @@ int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) int virNetDevOpenvswitchGetVhostuserIfname(const char *path, bool server, char **ifname) - ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT G_GNUC_NO_INLINE; + ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT G_GNUC_NO_INLINE; int virNetDevOpenvswitchUpdateVlan(const char *ifname, const virNetDevVlan *virtVlan) -- 2.26.2

1 0

[libvirt PATCH] kbase: Shorten "less verbose QEMU logging" example
by Andrea Bolognani 14 Nov '20

14 Nov '20

Rationale for the changes: * util.dbus produces very little logging, so it doesn't really make sense to filter it out; * util.udev doesn't exist; * access can be filtered out entirely, as nothing very interesting is produced by the only other component in the same package (access.accessdriverpolkit). Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- docs/kbase/debuglogs.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/kbase/debuglogs.rst b/docs/kbase/debuglogs.rst index 7945cf8bed..fed998efd8 100644 --- a/docs/kbase/debuglogs.rst +++ b/docs/kbase/debuglogs.rst @@ -178,13 +178,12 @@ Less verbose logging for QEMU VMs Some subsystems are very noisy and usually not the culprit of the problems. They can be silenced individually for a less verbose log while still logging -everything else. Usual suspects are the JSON code, udev, authentication and such. +everything else. Usual suspects are the JSON code, RPC, authentication and such. A permissive filter is good for development use cases. :: - 3:remote 4:event 3:util.json 3:util.object 3:util.dbus 3:util.udev 3:node_device 3:rpc 3:access.accessmanager 3:util.netlink 1:* - + 3:remote 4:event 3:util.json 3:util.object 3:util.netlink 3:node_device 3:rpc 3:access 1:* Minimalistic QEMU QMP monitor logging ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -- 2.26.2

3 3

[PATCH 0/2] virnetdevopenvswitch: Get names for dpdkvhostuserclient too
by Michal Privoznik 14 Nov '20

14 Nov '20

The first patch fixes the actual problem, the second is just a cleanup I've came up with whilst looking at the code. Michal Prívozník (2): virnetdevopenvswitch: Get names for dpdkvhostuserclient too virnetdevopenvswitch: Simplify OVS_VSCTL cmd creation src/qemu/qemu_command.c | 1 + src/qemu/qemu_hotplug.c | 1 + src/util/virnetdevopenvswitch.c | 92 +++++++++++++++++++-------------- src/util/virnetdevopenvswitch.h | 1 + tests/qemuxml2argvmock.c | 1 + 5 files changed, 56 insertions(+), 40 deletions(-) -- 2.26.2

3 5

[libvirt PATCH] docs: compiling.html: pass -d to xz to decompress
by Daniel P. Berrangé 14 Nov '20

14 Nov '20

From: Jonathan Watt <jwatt(a)jwatt.org> tar on macOS recognizes XZ compression automatically, but that is not the case for GNU tar (1.32 at least). On Fedora 33 the current instructions result in the following error: $ xz -c libvirt-6.9.0.tar.xz | tar xvf - tar: Archive is compressed. Use -J option tar: Error is not recoverable: exiting now Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com> Signed-off-by: Jonathan Watt <jwatt(a)jwatt.org> --- Pushed as a trivial patch from gitlab docs/compiling.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/compiling.html.in b/docs/compiling.html.in index c898d16313..c2c9d9f0ed 100644 --- a/docs/compiling.html.in +++ b/docs/compiling.html.in @@ -14,7 +14,7 @@ </p> <pre> -$ xz -c libvirt-x.x.x.tar.xz | tar xvf - +$ xz -dc libvirt-x.x.x.tar.xz | tar xvf - $ cd libvirt-x.x.x $ meson build</pre> -- 2.28.0

1 0

[PATCH 0/6] qemu: Enable client TLS certificate validation by default for chardev, migration, and backup servers
by Peter Krempa 14 Nov '20

14 Nov '20

See patches 6 for a explanation. Peter Krempa (6): qemu: conf: Allow individual control of default value for *_tls_x509_verify qemu: conf: Clarify default of "vnc_tls_x509_verify" qemu: conf: Enable 'chardev_tls_x509_verify' by default qemu: conf: Enable 'migrate_tls_x509_verify' by default qemu: conf: Enable 'backup_tls_x509_verify' by default NEWS: Mention change of default for TLS certificate verification NEWS.rst | 11 +++++++++++ src/qemu/qemu.conf | 18 ++++++++++++++---- src/qemu/qemu_conf.c | 22 ++++++++++++++-------- src/qemu/qemu_conf.h | 1 + 4 files changed, 40 insertions(+), 12 deletions(-) -- 2.28.0

5 14

Re: [PATCH] Added attach-disk parameters for network disk support
by Peter Krempa 13 Nov '20

13 Nov '20

On Thu, Nov 12, 2020 at 15:15:27 -0600, Ryan Gahagan wrote: > Hey Peter, Hi Ryan, firstly I'd like to ask you to keep the conversation on-list. (use reply-all all the time). This ensures that the response is public and also other people can jump in and respont. At the very least it can be used for future reference. I'll be re-adding the mailing list in this response. Also please avoid top-posting in techincal lists. It's always best to respond inline at the proper context that is being discussed. > Sorry about all the issues with our commit, we're new to this and trying to > make our first contribution to this repo. We had a couple questions over > your feedback and wanted to run them by you before making another request. Sure thing, you can always ask on the mailing list even without patches. > In the original code, the --source parameter is marked as required but okay > to be empty. If the string is empty, then we think the parameter read in by > vshCommandOptStringReq would be NULL, in which case no <source> tag would > be generated at all. If this is the case, then why is --source required in > the first place? We see that --source-name is mutually exclusive witha That's a quirk of it being a positional argument (VSH_OT_DATA). A positional argument is mandatory. > --source, but is it also okay to be empty? For example, in the issue > <https://gitlab.com/libvirt/libvirt/-/issues/16> this patch was based on, > there's an example XML which has a <source> with neither a file nor a name. > Is this example incorrect, or is it fine to have neither and only have a > protocol? That is correct for example with the NBD protocol where a missing name actually means the default unnamed export. The thing is that we can't really change --source to be something else than VSH_OT_DATA since it would change the semantics of the virsh command. You probably will need to use it also for the 'name' attribute and switch the name according to the presence of the 'protocol' attribute/option. > When it comes to making these mutually exclusive, we also were confused on > what flags to give them. Would it be proper behavior to give them both no > flags or only the VSH_OFLAG_EMPTY_OK flag and then check that one is > defined in the cmdAttachDisk method (using VSH_EXCLUSIVE_OPTIONS_VAR) and > error there instead? Those are two separate things. If it makes sense for a flag to be specified but empty and it impacts behaviour then you need to use VSH_OFLAG_EMPTY_OK. Any non-positional arguments (those which need --) can be made optional. On the other hand that does not say anything about how they can or can't be combined with other options and thus you need to declare that in the code. > If the user provides an empty source without any protocol, but provides > --source-host-* parameters, what should be generated instead? Would this This should be an error. --source-host must require the use of --source-protocol > cause an error, would it generate an in-line <host> tag, or would it > generate a completely empty <source> tag followed by the host information? The protocol option flag should be mandatory if you want to use the host/socket. > > -Ryan Gahagan > > On Thu, Nov 12, 2020 at 2:57 AM Peter Krempa <pkrempa(a)redhat.com> wrote: > > > On Wed, Nov 11, 2020 at 17:00:51 -0600, Ryan Gahagan wrote: > > > Related issue: https://gitlab.com/libvirt/libvirt/-/issues/16 > > > Added in support for the following parameters in attach-disk: > > > --source-name > > > --source-protocol > > > --source-host-name > > > --source-host-socket > > > --source-host-transport Please trim parts of the message that are no longer relevant. [...]

1 0

[PATCH] viridentitytest: Run more frequently
by Michal Privoznik 13 Nov '20

13 Nov '20

The viridentitytest tests our viridentity module which is compiled on all platforms and OSes. There is no need to have SELinux secdriver as individual test cases are skipped if SELinux is missing. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Successfully build here: https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/213586713 While technically this is a v2 of: https://www.redhat.com/archives/libvir-list/2020-November/msg00308.html it's implementing completely different approach and thus I did not mark it as v2. tests/meson.build | 2 +- tests/viridentitytest.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/meson.build b/tests/meson.build index 8decdfa20c..9f0051303d 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -312,6 +312,7 @@ tests += [ { 'name': 'virfirewalltest' }, { 'name': 'virhostcputest', 'link_whole': [ test_file_wrapper_lib ] }, { 'name': 'virhostdevtest' }, + { 'name': 'viridentitytest' }, { 'name': 'viriscsitest' }, { 'name': 'virkeycodetest' }, { 'name': 'virkmodtest' }, @@ -480,7 +481,6 @@ if conf.has('WITH_SECDRIVER_SELINUX') if conf.has('WITH_LIBATTR') tests += [ { 'name': 'securityselinuxtest' }, - { 'name': 'viridentitytest' }, ] if conf.has('WITH_QEMU') diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c index fd01eeaa80..afb9fdaec4 100644 --- a/tests/viridentitytest.c +++ b/tests/viridentitytest.c @@ -88,8 +88,7 @@ static int testIdentityGetSystem(const void *data) #if !WITH_SELINUX if (context) { VIR_DEBUG("libvirt not compiled with SELinux, skipping this test"); - ret = EXIT_AM_SKIP; - return -1; + return EXIT_AM_SKIP; } #endif -- 2.26.2

3 2

[PATCH 0/9] docs: kbase style change and cleanups
by Peter Krempa 13 Nov '20

13 Nov '20

Some patches are taken from an older series where we've discussed that the kbase page should look more like 'docs.html' as more articles appear, this series delivers that: https://www.redhat.com/archives/libvir-list/2020-August/msg00172.html ... and a few other changes. Final output: https://pipo.sk.gitlab.io/-/libvirt/-/jobs/830387801/artifacts/website/docs… Peter Krempa (9): docs: Fix title of 'docs' page docs: xslt: Use 'Link' rather than 'Permalink' in header links docs: css: Add a gray box around table of contents of RST based docs docs: kbase: Move index page to docs/kbase docs: kbase: Remove extra container from index page docs: kbase: Split articles into sections docs: xsl: Unify stylability of main container element docs: css: Modify appearance of the kbase directory page docs: kbase: Reorder some articles in the 'Usage' section docs/docs.html.in | 5 ++-- docs/index.html.in | 4 +-- docs/kbase.rst | 53 --------------------------------- docs/kbase/index.rst | 57 +++++++++++++++++++++++++++++++++++ docs/kbase/meson.build | 1 + docs/libvirt.css | 67 +++++++++++++++++++++++++++--------------- docs/meson.build | 1 - docs/page.xsl | 27 ++++++++++------- scripts/hvsupport.py | 2 +- 9 files changed, 124 insertions(+), 93 deletions(-) delete mode 100644 docs/kbase.rst create mode 100644 docs/kbase/index.rst -- 2.26.2

2 11