[PATCH v2] NEWS: Announcing Network Metadata APIs
Ref to patchset implementing the above: https://listman.redhat.com/archives/libvir-list/2023-August/241250.html Signed-off-by: K Shiva Kiran --- This is a v2 of: https://listman.redhat.com/archives/libvir-list/2023-August/241469.html Diff to v1: - Shortened the text and put all text under one section. NEWS.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index e40c8ac259..5275a8299a 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -28,6 +28,17 @@ v9.7.0 (unreleased) 2) pre-binding the variant driver using the ``--driver`` option of ``virsh nodedev-detach``. + * network: Support for and fields in Network XML + +The network object adds two more user defined metadata fields +and . +Two new APIs ``virNetworkGetMetadata()`` and ``virNetworkSetMetadata()`` can be +used to view and modify the above including the existing field. + +virsh adds two new commands ``net-desc`` and ``net-metadata`` to view/modify the same. +``net-list`` adds a new option ``--title`` that prints the content of +in an extra column within the default ``--table`` output. + * **Improvements** * **Bug fixes** -- 2.42.0
Error : virHostCPUGetKVMMaxVCPUs:1228 : KVM is not supported on this platform: Function not implemented.
Hello. I'm running Debian bookworm on my ARM Chromebook,model "xe303c12" and I've recompiled the kernel (5.4) to enable KVM,so now my system sounds like this : $ lsb_release -a No LSB modules are available. Distributor ID: Debian Description:Debian GNU/Linux 12 (bookworm) Release:12 Codename: bookworm $ uname -a Linux chromarietto 5.4.244-stb-cbe #8 SMP PREEMPT Sat Aug 19 22:19:32 UTC 2023 armv7l GNU/Linux $ uname -r 5.4.244-stb-cbe $ kvm-ok INFO: /dev/kvm exists KVM acceleration can be used $ qemu-system-arm --version QEMU emulator version 5.1.0 (v5.1.0-dirty) Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers $ python3 --version Python 3.11.2 I have installed libvirt 9.7.0,qemu 5.1 and virt-manager from source code with the final goal to be able to connect qemu,kvm and libvirt together to virtualize FreeBSD 13.2 for arm 32 bit. Some useful informations about my platform : root@chromarietto:/home/marietto/Desktop# virt-host-validate QEMU: Checking if device /dev/kvm exists : PASS QEMU: Checking if device /dev/kvm is accessible : PASS QEMU: Checking if device /dev/vhost-net exists: PASS QEMU: Checking if device /dev/net/tun exists : PASS QEMU: Checking for cgroup 'cpu' controller support: PASS QEMU: Checking for cgroup 'cpuacct' controller support: PASS QEMU: Checking for cgroup 'cpuset' controller support : PASS QEMU: Checking for cgroup 'memory' controller support : PASS QEMU: Checking for cgroup 'devices' controller support: PASS QEMU: Checking for cgroup 'blkio' controller support : PASS QEMU: Checking for device assignment IOMMU support : WARN (No ACPI IORT table found, IOMMU not supported by this hardware platform) QEMU: Checking for secure guest support : WARN (Unknown if this platform has Secure Guest support) LXC: Checking for Linux >= 2.6.26 : PASS LXC: Checking for namespace ipc : PASS LXC: Checking for namespace mnt : PASS LXC: Checking for namespace pid : PASS LXC: Checking for namespace uts : PASS LXC: Checking for namespace net : PASS LXC: Checking for namespace user : PASS LXC: Checking for cgroup 'cpu' controller support : PASS LXC: Checking for cgroup 'cpuacct' controller support : PASS LXC: Checking for cgroup 'cpuset' controller support : PASS LXC: Checking for cgroup 'memory' controller support : PASS LXC: Checking for cgroup 'devices' controller support : PASS LXC: Checking for cgroup 'freezer' controller support : FAIL (Enable 'freezer' in kernel Kconfig file or mount/enable cgroup controller in your system) LXC: Checking for cgroup 'blkio' controller support : PASS LXC: Checking if device /sys/fs/fuse/connections exists : PASS # lsmod | grep kvm no errors (I have embedded the options needed to enable KVM inside the kernel) # virsh --connect qemu:///system capabilities | grep baselabel +1002:+1002 +1002:+1002 The error that I'm not able to fix is the following one : root@chromarietto:~# virsh domcapabilities --machine virt --emulatorbin /usr/local/bin/qemu-system-arm 2023-08-29 10:17:59.110+: 1763: error : virHostCPUGetKVMMaxVCPUs:1228 : KVM is not supported on this platform: Function not implemented ; error: failed to get emulator capabilities error: KVM is not supported on this platform: Function not implemented and this is the log that I've got when I ran libvirtd with the debug option enabled root@chromarietto:~# libvirtd --debug [Tue, 29 Aug 2023 10:10:11 virt-manager 2141] DEBUG (createvm:494) UEFI found, setting it as default. [Tue, 29 Aug 2023 10:10:11 virt-manager 2141] DEBUG (createvm:728) Guest type set to os_type=hvm, arch=armv7l, dom_type=kvm [Tue, 29 Aug 2023 10:10:11 virt-manager 2141] DEBUG (guest:546) Prefer EFI => True 2023-08-29 10:10:12.972+: 1765: error : virHostCPUGetKVMMaxVCPUs:1228 : KVM is not supported on this platform: Function not implemented [Tue, 29 Aug 2023 10:10:12 virt-manager 2141] DEBUG (domcapabilities:250) Error fetching domcapabilities XML Traceback (most recent call last): File "/usr/local/share/virt-manager/virtinst/domcapabilities.py", line 245, in build_from_params xml = conn.getDomainCapabilities(emulator, arch, ^^ File "/usr/lib/python3/dist-packages/libvirt.py", line 4612, in getDomainCapabilities raise
Re: [PATCH 0/3] bhyve: Feed hook scripts with domain XML
On a Tuesday in 2023, Michal Privoznik wrote: *** BLURB HERE *** Michal Prívozník (3): docs: Document that libxl hooks are also given full domain XML docs: Document bhyve hook scripts bhyve: Feed hook scripts with domain XML docs/hooks.rst| 56 +++ src/bhyve/bhyve_process.c | 35 2 files changed, 75 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 3/4] vircommand: Make sysconf(_SC_OPEN_MAX) failure non-fatal
On Tue, Aug 29, 2023 at 05:13:08PM +0200, Michal Privoznik wrote:
> The point of calling sysconf(_SC_OPEN_MAX) is to allocate big
> enough bitmap so that subsequent call to
> virCommandMassCloseGetFDsDir() can just set the bit instead of
> expanding memory (this code runs in a forked off child and thus
> using async-signal-unsafe functions like malloc() is a bit
> tricky).
>
> But on some systems the limit for opened FDs is virtually
> non-existent (typically macOS Ventura started reporting EINVAL).
>
> But with both glibc and musl using malloc() after fork() is safe.
> And with sufficiently new glib too, as it's using malloc() with
> newer releases instead of their own allocator.
>
> Therefore, pick a sufficiently large value (glibc falls back to
> 256, [1], so 1024 should be good enough) to fall back to and make
> the error non-fatal.
That's not suitable for macOS. THey have a constant OPEN_MAX we
should be using that is way bigger than 1024.
>
> 1:
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getdtsz.c;h=4c5a6208067d2f9eaaac6dba652702fb4af9b7e3;hb=HEAD
> Signed-off-by: Michal Privoznik
> ---
> src/util/vircommand.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 822b9487f9..8c06e19723 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -500,7 +500,7 @@ virCommandMassCloseGetFDsDir(virBitmap *fds,
> return -1;
> }
>
> -ignore_value(virBitmapSetBit(fds, fd));
> +virBitmapSetBitExpand(fds, fd);
> }
>
> if (rc < 0)
> @@ -544,10 +544,8 @@ virCommandMassCloseFrom(virCommand *cmd,
> * Therefore we can safely allocate memory here (and transitively call
> * opendir/readdir) without a deadlock. */
>
> -if (openmax < 0) {
> -virReportSystemError(errno, "%s", _("sysconf(_SC_OPEN_MAX) failed"));
> -return -1;
> -}
> +if (openmax <= 0)
> +openmax = 1024;
>
> fds = virBitmapNew(openmax);
>
> --
> 2.41.0
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH 2/4] vircommand: Isolate FD dir parsing into a separate function
So far, virCommandMassCloseGetFDsLinux() opens "/proc/self/fd",
iterates over it marking opened FDs in @fds bitmap. Well, we can
do the same on other systems (with altered path), like MacOS or
FreeBSD. Therefore, isolate dir iteration into a separate
function that accepts dir path as an argument.
Unfortunately, this function might be unused on some systems
(e.g. mingw), therefore mark it as such.
Signed-off-by: Michal Privoznik
---
src/util/vircommand.c | 31 +--
1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 60d419a695..822b9487f9 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -479,17 +479,12 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
return 0;
}
-# ifdef __linux__
-/* On Linux, we can utilize procfs and read the table of opened
- * FDs and selectively close only those FDs we don't want to pass
- * onto child process (well, the one we will exec soon since this
- * is called from the child). */
-static int
-virCommandMassCloseGetFDsLinux(virBitmap *fds)
+static int G_GNUC_UNUSED
+virCommandMassCloseGetFDsDir(virBitmap *fds,
+ const char *dirName)
{
g_autoptr(DIR) dp = NULL;
struct dirent *entry;
-const char *dirName = "/proc/self/fd";
int rc;
if (virDirOpen(, dirName) < 0)
@@ -514,15 +509,20 @@ virCommandMassCloseGetFDsLinux(virBitmap *fds)
return 0;
}
-# else /* !__linux__ */
-
static int
-virCommandMassCloseGetFDsGeneric(virBitmap *fds)
+virCommandMassCloseGetFDs(virBitmap *fds)
{
+# ifdef __linux__
+/* On Linux, we can utilize procfs and read the table of opened
+ * FDs and selectively close only those FDs we don't want to pass
+ * onto child process (well, the one we will exec soon since this
+ * is called from the child). */
+return virCommandMassCloseGetFDsDir(fds, "/proc/self/fd");
+# else
virBitmapSetAll(fds);
return 0;
+# endif
}
-# endif /* !__linux__ */
static int
virCommandMassCloseFrom(virCommand *cmd,
@@ -551,13 +551,8 @@ virCommandMassCloseFrom(virCommand *cmd,
fds = virBitmapNew(openmax);
-# ifdef __linux__
-if (virCommandMassCloseGetFDsLinux(fds) < 0)
+if (virCommandMassCloseGetFDs(fds) < 0)
return -1;
-# else
-if (virCommandMassCloseGetFDsGeneric(fds) < 0)
-return -1;
-# endif
lastfd = MAX(lastfd, childin);
lastfd = MAX(lastfd, childout);
--
2.41.0
[PATCH 1/4] vircommand: Drop unused arguments from virCommandMassCloseGetFDs*()
Both virCommandMassCloseGetFDsLinux() and
virCommandMassCloseGetFDsGeneric() take @cmd argument only to
mark it as unused. Drop it from both.
Signed-off-by: Michal Privoznik
---
src/util/vircommand.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 5f094c625a..60d419a695 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -485,8 +485,7 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
* onto child process (well, the one we will exec soon since this
* is called from the child). */
static int
-virCommandMassCloseGetFDsLinux(virCommand *cmd G_GNUC_UNUSED,
- virBitmap *fds)
+virCommandMassCloseGetFDsLinux(virBitmap *fds)
{
g_autoptr(DIR) dp = NULL;
struct dirent *entry;
@@ -518,8 +517,7 @@ virCommandMassCloseGetFDsLinux(virCommand *cmd
G_GNUC_UNUSED,
# else /* !__linux__ */
static int
-virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED,
- virBitmap *fds)
+virCommandMassCloseGetFDsGeneric(virBitmap *fds)
{
virBitmapSetAll(fds);
return 0;
@@ -554,10 +552,10 @@ virCommandMassCloseFrom(virCommand *cmd,
fds = virBitmapNew(openmax);
# ifdef __linux__
-if (virCommandMassCloseGetFDsLinux(cmd, fds) < 0)
+if (virCommandMassCloseGetFDsLinux(fds) < 0)
return -1;
# else
-if (virCommandMassCloseGetFDsGeneric(cmd, fds) < 0)
+if (virCommandMassCloseGetFDsGeneric(fds) < 0)
return -1;
# endif
--
2.41.0
[PATCH 0/4] vircommand: Various mass close improvements
*** BLURB HERE *** Michal Prívozník (4): vircommand: Drop unused arguments from virCommandMassCloseGetFDs*() vircommand: Isolate FD dir parsing into a separate function vircommand: Make sysconf(_SC_OPEN_MAX) failure non-fatal vircommand: Parse /dev/fd on *BSD-like systems when looking for opened FDs src/util/vircommand.c | 43 ++- 1 file changed, 18 insertions(+), 25 deletions(-) -- 2.41.0
[PATCH 3/4] vircommand: Make sysconf(_SC_OPEN_MAX) failure non-fatal
The point of calling sysconf(_SC_OPEN_MAX) is to allocate big
enough bitmap so that subsequent call to
virCommandMassCloseGetFDsDir() can just set the bit instead of
expanding memory (this code runs in a forked off child and thus
using async-signal-unsafe functions like malloc() is a bit
tricky).
But on some systems the limit for opened FDs is virtually
non-existent (typically macOS Ventura started reporting EINVAL).
But with both glibc and musl using malloc() after fork() is safe.
And with sufficiently new glib too, as it's using malloc() with
newer releases instead of their own allocator.
Therefore, pick a sufficiently large value (glibc falls back to
256, [1], so 1024 should be good enough) to fall back to and make
the error non-fatal.
1:
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getdtsz.c;h=4c5a6208067d2f9eaaac6dba652702fb4af9b7e3;hb=HEAD
Signed-off-by: Michal Privoznik
---
src/util/vircommand.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 822b9487f9..8c06e19723 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -500,7 +500,7 @@ virCommandMassCloseGetFDsDir(virBitmap *fds,
return -1;
}
-ignore_value(virBitmapSetBit(fds, fd));
+virBitmapSetBitExpand(fds, fd);
}
if (rc < 0)
@@ -544,10 +544,8 @@ virCommandMassCloseFrom(virCommand *cmd,
* Therefore we can safely allocate memory here (and transitively call
* opendir/readdir) without a deadlock. */
-if (openmax < 0) {
-virReportSystemError(errno, "%s", _("sysconf(_SC_OPEN_MAX) failed"));
-return -1;
-}
+if (openmax <= 0)
+openmax = 1024;
fds = virBitmapNew(openmax);
--
2.41.0
[PATCH 4/4] vircommand: Parse /dev/fd on *BSD-like systems when looking for opened FDs
On BSD-like systems "/dev/fd" serves the same purpose as "/proc/self/fd". And since procfs is usually not mounted, on such systems we can use "/dev/fd" instead. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/518 Signed-off-by: Michal Privoznik --- src/util/vircommand.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 8c06e19723..2b3b125e5f 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -518,6 +518,8 @@ virCommandMassCloseGetFDs(virBitmap *fds) * onto child process (well, the one we will exec soon since this * is called from the child). */ return virCommandMassCloseGetFDsDir(fds, "/proc/self/fd"); +# elif defined(__APPLE__) || defined(__FreeBSD__) +return virCommandMassCloseGetFDsDir(fds, "/dev/fd"); # else virBitmapSetAll(fds); return 0; -- 2.41.0
[PATCH 1/3] docs: Document that libxl hooks are also given full domain XML
Our hooks.rst document existence of libxl hook scripts, but mentions only qemu and lxc as receivers of full domain XML. But since their introduction in v2.2.0-rc1~201 they are also given full domain XML. Fix our wording. Signed-off-by: Michal Privoznik --- docs/hooks.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/hooks.rst b/docs/hooks.rst index 9387676a55..b127ec4202 100644 --- a/docs/hooks.rst +++ b/docs/hooks.rst @@ -89,7 +89,7 @@ Script arguments The hook scripts are called with specific command line arguments, depending upon the script, and the operation being performed. -The guest hook scripts, qemu and lxc, are also given the **full** XML +The guest hook scripts, qemu, lxc and libxl are also given the **full** XML description for the domain on their stdin. This includes items such the UUID of the domain and its storage information, and is intended to provide all the libvirt information the script needs. @@ -126,8 +126,8 @@ followed with the full XML description of the port: Please note that this approach is different from other cases such as ``daemon``, -``qemu`` or ``lxc`` hook scripts, because two XMLs may be passed here, while in -the other cases only a single XML is passed. +``qemu``, ``lxc`` or ``libxl`` hook scripts, because two XMLs may be passed +here, while in the other cases only a single XML is passed. The command line arguments take this approach: -- 2.41.0
[PATCH 3/3] bhyve: Feed hook scripts with domain XML
Domain related hook scripts are all fed with domain XML on their
stdin, except for bhyve. Fix this.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/528
Signed-off-by: Michal Privoznik
---
docs/hooks.rst| 10 +-
src/bhyve/bhyve_process.c | 35 ---
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/docs/hooks.rst b/docs/hooks.rst
index 4e02ba0f8f..bd197c0d6e 100644
--- a/docs/hooks.rst
+++ b/docs/hooks.rst
@@ -92,9 +92,9 @@ Script arguments
The hook scripts are called with specific command line arguments, depending
upon
the script, and the operation being performed.
-The guest hook scripts, qemu, lxc and libxl are also given the **full** XML
-description for the domain on their stdin. This includes items such the UUID of
-the domain and its storage information, and is intended to provide all the
+The guest hook scripts, qemu, lxc, libxl and bhyve are also given the **full**
+XML description for the domain on their stdin. This includes items such the
UUID
+of the domain and its storage information, and is intended to provide all the
libvirt information the script needs.
For all cases, stdin of the network hook script is provided with the full XML
@@ -129,8 +129,8 @@ followed with the full XML description of the port:
Please note that this approach is different from other cases such as
``daemon``,
-``qemu``, ``lxc`` or ``libxl`` hook scripts, because two XMLs may be passed
-here, while in the other cases only a single XML is passed.
+``qemu``, ``lxc``, ``libxl`` or ``bhyve`` hook scripts, because two XMLs may be
+passed here, while in the other cases only a single XML is passed.
The command line arguments take this approach:
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 80d5a8804f..d476ff401f 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -94,21 +94,34 @@ virBhyveFormatDevMapFile(const char *vm_name, char **fn_out)
}
static int
-bhyveProcessStartHook(virDomainObj *vm, virHookBhyveOpType op)
+bhyveProcessStartHook(struct _bhyveConn *driver,
+ virDomainObj *vm,
+ virHookBhyveOpType op)
{
+g_autofree char *xml = NULL;
+
if (!virHookPresent(VIR_HOOK_DRIVER_BHYVE))
return 0;
+xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
+
return virHookCall(VIR_HOOK_DRIVER_BHYVE, vm->def->name, op,
- VIR_HOOK_SUBOP_BEGIN, NULL, NULL, NULL);
+ VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL);
}
static void
-bhyveProcessStopHook(virDomainObj *vm, virHookBhyveOpType op)
+bhyveProcessStopHook(struct _bhyveConn *driver,
+ virDomainObj *vm,
+ virHookBhyveOpType op)
{
-if (virHookPresent(VIR_HOOK_DRIVER_BHYVE))
-virHookCall(VIR_HOOK_DRIVER_BHYVE, vm->def->name, op,
-VIR_HOOK_SUBOP_END, NULL, NULL, NULL);
+g_autofree char *xml = NULL;
+if (!virHookPresent(VIR_HOOK_DRIVER_BHYVE))
+return;
+
+xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
+
+virHookCall(VIR_HOOK_DRIVER_BHYVE, vm->def->name, op,
+VIR_HOOK_SUBOP_END, NULL, xml, NULL);
}
static int
@@ -194,7 +207,7 @@ virBhyveProcessStartImpl(struct _bhyveConn *driver,
goto cleanup;
}
-if (bhyveProcessStartHook(vm, VIR_HOOK_BHYVE_OP_START) < 0)
+if (bhyveProcessStartHook(driver, vm, VIR_HOOK_BHYVE_OP_START) < 0)
goto cleanup;
/* Now we can start the domain */
@@ -216,7 +229,7 @@ virBhyveProcessStartImpl(struct _bhyveConn *driver,
BHYVE_STATE_DIR) < 0)
goto cleanup;
-if (bhyveProcessStartHook(vm, VIR_HOOK_BHYVE_OP_STARTED) < 0)
+if (bhyveProcessStartHook(driver, vm, VIR_HOOK_BHYVE_OP_STARTED) < 0)
goto cleanup;
ret = 0;
@@ -265,7 +278,7 @@ virBhyveProcessStart(virConnectPtr conn,
struct _bhyveConn *driver = conn->privateData;
/* Run an early hook to setup missing devices. */
-if (bhyveProcessStartHook(vm, VIR_HOOK_BHYVE_OP_PREPARE) < 0)
+if (bhyveProcessStartHook(driver, vm, VIR_HOOK_BHYVE_OP_PREPARE) < 0)
return -1;
if (flags & VIR_BHYVE_PROCESS_START_AUTODESTROY)
@@ -307,7 +320,7 @@ virBhyveProcessStop(struct _bhyveConn *driver,
if ((priv != NULL) && (priv->mon != NULL))
bhyveMonitorClose(priv->mon);
-bhyveProcessStopHook(vm, VIR_HOOK_BHYVE_OP_STOPPED);
+bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_STOPPED);
/* Cleanup network interfaces */
bhyveNetCleanup(vm);
@@ -329,7 +342,7 @@ virBhyveProcessStop(struct _bhyveConn *driver,
vm->pid = 0;
vm->def->id = -1;
-bhyveProcessStopHook(vm, VIR_HOOK_BHYVE_OP_RELEASE);
+bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_RELEASE);
cleanup:
virPidFileDelete(BHYVE_STATE_DIR, vm->def->name);
--
2.41.0
[PATCH 2/3] docs: Document bhyve hook scripts
We have bhyve hook scripts since v6.1.0-rc1~42 but never mention them in hooks.rst. Fill the blanks. Signed-off-by: Michal Privoznik --- docs/hooks.rst | 46 ++ 1 file changed, 46 insertions(+) diff --git a/docs/hooks.rst b/docs/hooks.rst index b127ec4202..4e02ba0f8f 100644 --- a/docs/hooks.rst +++ b/docs/hooks.rst @@ -20,6 +20,7 @@ occur: - A QEMU guest is started or stopped ( :since:`since 0.8.0` ) - An LXC guest is started or stopped ( :since:`since 0.8.0` ) - A libxl-handled Xen guest is started or stopped ( :since:`since 2.1.0` ) +- An bhyve guest is started or stopped ( :since:`since 6.1.0` ) - A network is started or stopped or an interface is plugged/unplugged to/from the network ( :since:`since 1.2.2` ) @@ -53,6 +54,8 @@ At present, there are five hook scripts that can be called: Executed when an LXC guest is started or stopped - ``/etc/libvirt/hooks/libxl`` Executed when a libxl-handled Xen guest is started, stopped, or migrated +- ``/etc/libvirt/hooks/bhyve`` + Executed when an bhyve guest is started or stopped - ``/etc/libvirt/hooks/network`` Executed when a network is started or stopped or an interface is plugged/unplugged to/from the network @@ -393,6 +396,49 @@ operation. There is no specific operation to indicate a "restart" is occurring. /etc/libvirt/hooks/libxl guest_name reconnect begin - +/etc/libvirt/hooks/bhyve + + +- | Before an bhyve guest is started, the bhyve hook script is called in three + locations; if any location fails, the guest is not started. The first + location, :since:`since 6.1.0` , is before libvirt performs any resource + labeling, and the hook can allocate resources not managed by libvirt. This is + called as: + + :: + + /etc/libvirt/hooks/bhyve guest_name prepare begin - + + | The second location, available :since:`Since 6.1.0` , occurs after libvirt + has finished labeling all resources, but has not yet started the guest, + called as: + + :: + + /etc/libvirt/hooks/bhyve guest_name start begin - + + | The third location, :since:`6.1.0` , occurs after the bhyve process has + successfully started up: + + :: + + /etc/libvirt/hooks/bhyve guest_name started begin - + +- | When an bhyve guest is stopped, the bhyve hook script is called in two + locations, to match the startup. First, :since:`since 6.1.0` , the hook is + called before libvirt restores any labels: + + :: + + /etc/libvirt/hooks/bhyve guest_name stopped end - + + | Then, after libvirt has released all resources, the hook is called again, + :since:`since 6.1.0` , to allow any additional resource cleanup: + + :: + + /etc/libvirt/hooks/bhyve guest_name release end - + /etc/libvirt/hooks/network ^^ -- 2.41.0
[PATCH 0/3] bhyve: Feed hook scripts with domain XML
*** BLURB HERE *** Michal Prívozník (3): docs: Document that libxl hooks are also given full domain XML docs: Document bhyve hook scripts bhyve: Feed hook scripts with domain XML docs/hooks.rst| 56 +++ src/bhyve/bhyve_process.c | 35 2 files changed, 75 insertions(+), 16 deletions(-) -- 2.41.0
Re: [RFC PATCH-for-8.1] accel: Remove HAX accelerator
On 24/06/2023 01.12, Philippe Mathieu-Daudé wrote:
On 24/6/23 01:08, Philippe Mathieu-Daudé wrote:
HAX is deprecated since commits 73741fda6c ("MAINTAINERS: Abort
HAXM maintenance") and 90c167a1da ("docs/about/deprecated: Mark
HAXM in QEMU as deprecated"), released in v8.0.0.
Per the QEMU deprecation policy, we shouldn't remove it before
QEMU release v8.2.0. However per the latest HAXM release (v7.8),
the latest QEMU supported is v7.2:
Note: Up to this release, HAXM supports QEMU from 2.9.0 to 7.2.0.
(https://github.com/intel/haxm/releases/tag/v7.8.0)
The next commit (https://github.com/intel/haxm/commit/da1b8ec072)
added:
HAXM v7.8.0 is our last release and we will not accept
pull requests or respond to issues after this.
As of commit b455ce4c2f, it became very hard to build and test
HAXM. Its previous maintainers made it clear they won't help.
It doesn't seem to be a very good use of QEMU maintainers to
spend their time in a dead project. Save our time by removing
this orphan zombie code before the QEMU v8.2 release.
Signed-off-by: Philippe Mathieu-Daudé
---
diff --git a/docs/about/removed-features.rst
b/docs/about/removed-features.rst
index 5b258b446b..cc8a1e38a9 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -659,15 +659,18 @@ Use ``Icelake-Server`` instead.
System accelerators
---
-Userspace local APIC with KVM (x86, removed 8.0)
+Userspace local APIC with KVM (x86, removed in 8.0)
Oops I didn't mean to commit this line. The doc won't build with padding.
Hi Philippe,
now that 8.1 has been released, could you please update your patch by
removing this bad hunk and by updating the commit description?
Thanks,
Thomas
Re: [PATCH 0/5] Refactor few old-style XML node lookup loops
On 8/28/23 15:57, Peter Krempa wrote: > Peter Krempa (5): > virNetworkDNSHostDefParseXML: Refactor parsing > virsh: domain: Refactor XML handling for disk changes > virDomainFeaturesCapabilitiesDefParse: Use virXMLNodeGetSubelementList > virDomainFeaturesKVMDefParse: Use virXMLNodeGetSubelementList > virDomainFeaturesXENDefParse: Use virXMLNodeGetSubelementList > > src/conf/domain_conf.c | 50 +++--- > src/conf/network_conf.c | 94 - > tools/virsh-domain.c| 63 +++ > 3 files changed, 78 insertions(+), 129 deletions(-) > Reviewed-by: Michal Privoznik Michal
Re: [PATCH 0/9] qemu: Rework probing of vCPU hotplug support
On 8/28/23 15:11, Peter Krempa wrote:
> Nowadays all qemu's support the command which was used as witness, but
> was gated on machine type's support of vCPU hotplug. Directly probe the
> machine type.
>
>
> Peter Krempa (9):
> qemu: Rename qemuDomainSupportsNewVcpuHotplug to
> qemuDomainSupportsVcpuHotplug
> qemu: capabilities: Export functions necessary for probing machine
> types
> qemu: process: Probe machine type data on reconnect to qemu
> qemuxml2argvtest: Modernize 'cpu-hotplug-startup' case
> tests: qemuhotplugtest: Fix arch-specific parts of 'ppc64' test XMLs
> qemuhotplugtest: Remove 'modern' field for cpu hotplug tests
> qemuDomainSupportsVcpuHotplug: Base return value on
> virQEMUCapsGetMachineHotplugCpus
> qemu: capabilities: Retire QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS
> qemu: capabilities: Remove unused 'virQEMUCapsFilterByMachineType'
>
> src/qemu/qemu_capabilities.c | 53 ++-
> src/qemu/qemu_capabilities.h | 18 ---
> src/qemu/qemu_domain.c| 12 ++---
> src/qemu/qemu_domain.h| 2 +-
> src/qemu/qemu_hotplug.c | 4 +-
> src/qemu/qemu_process.c | 41 --
> .../caps_4.2.0_aarch64.xml| 1 -
> .../qemucapabilitiesdata/caps_4.2.0_ppc64.xml | 1 -
> .../qemucapabilitiesdata/caps_4.2.0_s390x.xml | 1 -
> .../caps_4.2.0_x86_64.xml | 1 -
> .../caps_5.0.0_aarch64.xml| 1 -
> .../qemucapabilitiesdata/caps_5.0.0_ppc64.xml | 1 -
> .../caps_5.0.0_riscv64.xml| 1 -
> .../caps_5.0.0_x86_64.xml | 1 -
> .../qemucapabilitiesdata/caps_5.1.0_sparc.xml | 1 -
> .../caps_5.1.0_x86_64.xml | 1 -
> .../caps_5.2.0_aarch64.xml| 1 -
> .../qemucapabilitiesdata/caps_5.2.0_ppc64.xml | 1 -
> .../caps_5.2.0_riscv64.xml| 1 -
> .../qemucapabilitiesdata/caps_5.2.0_s390x.xml | 1 -
> .../caps_5.2.0_x86_64.xml | 1 -
> .../caps_6.0.0_aarch64.xml| 1 -
> .../qemucapabilitiesdata/caps_6.0.0_s390x.xml | 1 -
> .../caps_6.0.0_x86_64.xml | 1 -
> .../caps_6.1.0_x86_64.xml | 1 -
> .../caps_6.2.0_aarch64.xml| 1 -
> .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 1 -
> .../caps_6.2.0_x86_64.xml | 1 -
> .../caps_7.0.0_aarch64+hvf.xml| 1 -
> .../caps_7.0.0_aarch64.xml| 1 -
> .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 -
> .../caps_7.0.0_x86_64.xml | 1 -
> .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 -
> .../caps_7.1.0_x86_64.xml | 1 -
> tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 -
> .../caps_7.2.0_x86_64+hvf.xml | 1 -
> .../caps_7.2.0_x86_64.xml | 1 -
> .../caps_8.0.0_riscv64.xml| 1 -
> .../caps_8.0.0_x86_64.xml | 1 -
> .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 -
> .../caps_8.1.0_x86_64.xml | 1 -
> tests/qemuhotplugtest.c | 40 +-
> .../ppc64-modern-bulk-domain.xml | 4 +-
> .../ppc64-modern-bulk-result-conf.xml | 18 ---
> .../ppc64-modern-bulk-result-live.xml | 19 +++
> .../ppc64-modern-individual-domain.xml| 4 +-
> .../ppc64-modern-individual-result-conf.xml | 18 ---
> .../ppc64-modern-individual-result-live.xml | 19 +++
> ...=> cpu-hotplug-startup.x86_64-latest.args} | 14 ++---
> tests/qemuxml2argvtest.c | 2 +-
> 50 files changed, 126 insertions(+), 177 deletions(-)
> rename tests/qemuxml2argvdata/{cpu-hotplug-startup.args =>
> cpu-hotplug-startup.x86_64-latest.args} (55%)
>
Reviewed-by: Michal Privoznik
Michal
Re: [libvirt PATCH v7 25/35] qemu: Taint domain if nbdkit restart fails
On Mon, Aug 28, 2023 at 16:45:00 -0500, Jonathon Jongsma wrote:
> Since the restart handler will trigger at an arbitrary time (when the
> nbdkit process crashes, for instance), it's difficult to provide
> feedback to the user if the restart is unsuccessful. Rather than just
> relying on a warning in the log, taint the domain so that there will be
> a slightly more user-visible notification.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/conf/domain_conf.c | 2 ++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_nbdkit.c | 4 +++-
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index bb4f1fdb94..8feaf5d055 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -87,6 +87,7 @@ VIR_ENUM_IMPL(virDomainTaint,
>"custom-hypervisor-feature",
>"deprecated-config",
>"custom-device",
> + "nbdkit-restart",
> );
>
> VIR_ENUM_IMPL(virDomainTaintMessage,
> @@ -105,6 +106,7 @@ VIR_ENUM_IMPL(virDomainTaintMessage,
>N_("hypervisor feature autodetection override"),
>N_("use of deprecated configuration settings"),
>N_("custom device configuration"),
> + N_("nbdkit restart failed"),
> );
>
> VIR_ENUM_IMPL(virDomainVirt,
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ca195a52d2..c0729905a8 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3194,6 +3194,7 @@ typedef enum {
> VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, /* custom hypervisor feature
> control */
> VIR_DOMAIN_TAINT_DEPRECATED_CONFIG, /* Configuration that is marked
> deprecated */
> VIR_DOMAIN_TAINT_CUSTOM_DEVICE, /* hypervisor device config customized */
> +VIR_DOMAIN_TAINT_NBDKIT_RESTART,/* nbdkit could not be restarted */
>
> VIR_DOMAIN_TAINT_LAST
> } virDomainTaintFlags;
As mentioned in review to 24/35, these two patches should be swapped, so
that this can be used directly.
Reviewed-by: Peter Krempa
> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index c3fa349922..ff5f1c0d05 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c
> @@ -623,8 +623,10 @@ qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
> /* clean up resources associated with process */
> qemuNbdkitProcessStop(proc);
>
> -if (qemuNbdkitProcessStart(proc, vm, driver) < 0)
> +if (qemuNbdkitProcessStart(proc, vm, driver) < 0) {
> VIR_WARN("Unable to restart nbkdit process");
> +virDomainObjTaint(vm, VIR_DOMAIN_TAINT_NBDKIT_RESTART);
> +}
This then obviously belongs to the patch adding the watching impl
Re: [libvirt PATCH v7 24/35] qemu: Monitor nbdkit process for exit
On Mon, Aug 28, 2023 at 16:44:59 -0500, Jonathon Jongsma wrote:
> Adds the ability to monitor the nbdkit process so that we can take
> action in case the child exits unexpectedly.
>
> When the nbdkit process exits, we pause the vm, restart nbdkit, and then
> resume the vm. This allows the vm to continue working in the event of a
> nbdkit failure.
>
> Eventually we may want to generalize this functionality since we may
> need something similar for e.g. qemu-storage-daemon, etc.
>
> The process is monitored with the pidfd_open() syscall if it exists
> (since linux 5.3). Otherwise it resorts to checking whether the process
> is alive once a second. The one-second time period was chosen somewhat
> arbitrarily.
>
> Signed-off-by: Jonathon Jongsma
> ---
> meson.build | 7 ++
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_driver.c | 18 ++
> src/qemu/qemu_nbdkit.c | 137 ++--
> src/qemu/qemu_nbdkit.h | 8 ++-
> src/qemu/qemu_process.c | 13 +++-
> src/qemu/qemu_process.h | 3 +
> 8 files changed, 180 insertions(+), 8 deletions(-)
[...]
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ad8428948b..46e089fe0f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4040,6 +4040,21 @@ processResetEvent(virQEMUDriver *driver,
> }
>
>
> +
> +
Too much whitespace before function.
> +static void
> +processNbdkitExitedEvent(virDomainObj *vm,
> + qemuNbdkitProcess *nbdkit)
> +{
> +if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
> +return;
> +
> +qemuNbdkitProcessRestart(nbdkit, vm);
> +
> +virDomainObjEndJob(vm);
> +}
> +
> +
> static void qemuProcessEventHandler(void *data, void *opaque)
> {
> struct qemuProcessEvent *processEvent = data;
[...]
> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index df638e99c0..c3fa349922 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c
[...]
> @@ -611,6 +613,106 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
> }
>
>
> +void
> +qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
> + virDomainObj *vm)
> +{
> +qemuDomainObjPrivate *vmpriv = vm->privateData;
> +virQEMUDriver *driver = vmpriv->driver;
> +
> +/* clean up resources associated with process */
> +qemuNbdkitProcessStop(proc);
> +
> +if (qemuNbdkitProcessStart(proc, vm, driver) < 0)
> +VIR_WARN("Unable to restart nbkdit process");
As noted before VIR_WARN is not acceptable. Move patch 25 before this
one and do the taint right away. And ditch the VIR_WARN.
> +}
> +
> +
> +typedef struct {
> +qemuNbdkitProcess *proc;
> +virDomainObj *vm;
> +} qemuNbdkitProcessEventData;
> +
> +
> +static qemuNbdkitProcessEventData*
> +qemuNbdkitProcessEventDataNew(qemuNbdkitProcess *proc,
> + virDomainObj *vm)
> +{
> +qemuNbdkitProcessEventData *d = g_new(qemuNbdkitProcessEventData, 1);
> +d->proc = proc;
> +d->vm = virObjectRef(vm);
> +return d;
> +}
> +
> +
> +static void
> +qemuNbdkitProcessEventDataFree(qemuNbdkitProcessEventData *d)
> +{
> +virObjectUnref(d->vm);
> +g_free(d);
> +}
> +
> +
> +#if WITH_DECL_SYS_PIDFD_OPEN
> +static void
> +qemuNbdkitProcessPidfdCb(int watch G_GNUC_UNUSED,
> + int fd,
> + int events G_GNUC_UNUSED,
> + void *opaque)
> +{
> +qemuNbdkitProcessEventData *d = opaque;
> +
> +VIR_FORCE_CLOSE(fd);
> +VIR_DEBUG("nbdkit process %i died", d->proc->pid);
I presume that the 'qemuNbdkitProcess' struct is protected by belonging
into the virDomainObj and thus using it's lock. In such chase this
violates locking as 'vm' is not locked at this point, but 'proc' is
accessed.
> +/* submit an event so that it is handled in the per-vm event thread */
> +qemuProcessHandleNbdkitExit(d->proc, d->vm);
> +}
> +#endif /* WITH_DECL_SYS_PIDFD_OPEN */
> +
> +
> +static int
> +qemuNbdkitProcessStartMonitor(qemuNbdkitProcess *proc,
> + virDomainObj *vm)
> +{
> +#if WITH_DECL_SYS_PIDFD_OPEN
> +int pidfd;
> +
> +pidfd = syscall(SYS_pidfd_open, proc->pid, 0);
> +if (pidfd < 0) {
> +virReportSystemError(errno, _("pidfd_open failed for %1$i"),
> proc->pid);
> +return -1;
> +}
> +
> +proc->eventwatch = virEventAddHandle(pidfd,
> + VIR_EVENT_HANDLE_READABLE,
> + qemuNbdkitProcessPidfdCb,
> + qemuNbdkitProcessEventDataNew(proc,
> vm),
> +
> (virFreeCallback)qemuNbdkitProcessEventDataFree);
'virEventAddHandle' can fail and return -1, if the corresponding
callback is not registered. All other callers check the return value and
also prevent the memory leak from the opaque data
Re: [PATCH] NEWS: Announcing Network Metadata APIs
On Fri, Aug 25, 2023 at 21:09:27 +0530, K Shiva Kiran wrote: > Ref to patchset implementing the above: > https://listman.redhat.com/archives/libvir-list/2023-August/241250.html > > Signed-off-by: K Shiva Kiran > --- > NEWS.rst | 26 ++ > 1 file changed, 26 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 73571f0019..01df22ef03 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -17,6 +17,32 @@ v9.7.0 (unreleased) > > * **New features** > > + * network: Support for `` and `` fields in Network XML > + > +The `` attribute can hold a short title defined by the user > +and cannot contain newlines. > +The `` attribute holds any documentation that the user > +wants to store. > + > + * network: Support to view and modify User defined metadata > + > +Two new APIs `virNetworkGetMetadata()` and `virNetworkSetMetadata()` > +have been introduced. These allow users to view and modify the > +contents of ``, `` and ``. > + > + * virsh: Introduces new commands `net-desc` and `net-metadata` > + > +`net-desc` can be used to view and modify the Network's title and > +description attributes. > +`net-metadata` can be used to define or modify the custom metadata > +attribute of the Network object. > + > + * virsh: Adds `--title` option to the `net-list` command > + > +This option prints the attribute of the network in > +an extra column. This option is usable only with the default > +`--table` output. All of the above are really about a single feature. I don't have a problem mentioning all the changed things although it can be shortened a bit, but you don't need 4 setions in the NEWS file for all of it.
Re: [PATCH] REFACTOR: Eliminate newlines in messages and fixes a typo
On 8/25/23 17:29, K Shiva Kiran wrote:
> Removes newlines of error message strings in metadata unit tests,
> libvirt-domain.c and libvirt-network.c
> Fixes a minor typo in libvirt-domain.c
>
> Signed-off-by: K Shiva Kiran
> ---
> include/libvirt/libvirt-domain.h | 2 +-
> src/libvirt-domain.c | 3 +--
> src/libvirt-network.c| 3 +--
> tests/metadatatest.c | 9 +++--
> tests/networkmetadatatest.c | 9 +++--
> 5 files changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h
> b/include/libvirt/libvirt-domain.h
> index a1902546bb..ea36805aa3 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -5184,7 +5184,7 @@ typedef void
> (*virConnectDomainEventDeviceRemovalFailedCallback)(virConnectPtr c
> * virConnectDomainEventMetadataChangeCallback:
> * @conn: connection object
> * @dom: domain on which the event occurred
> - * @type: a value from virDomainMetadataTypea
> + * @type: a value from virDomainMetadataType
> * @nsuri: XML namespace URI
> * @opaque: application specified data
> *
ACK to this hunk.
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index ec42bb9a53..80a554a530 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -8585,8 +8585,7 @@ virDomainSetMetadata(virDomainPtr domain,
> case VIR_DOMAIN_METADATA_TITLE:
> if (metadata && strchr(metadata, '\n')) {
> virReportInvalidArg(metadata, "%s",
> -_("metadata title can't contain "
> - "newlines"));
> +_("metadata title can't contain newlines"));
> goto error;
> }
> G_GNUC_FALLTHROUGH;
> diff --git a/src/libvirt-network.c b/src/libvirt-network.c
> index c0daea3a60..ef17a8a04d 100644
> --- a/src/libvirt-network.c
> +++ b/src/libvirt-network.c
> @@ -1974,8 +1974,7 @@ virNetworkSetMetadata(virNetworkPtr network,
> case VIR_NETWORK_METADATA_TITLE:
> if (metadata && strchr(metadata, '\n')) {
> virReportInvalidArg(metadata, "%s",
> -_("metadata title can't contain "
> - "newlines"));
> +_("metadata title can't contain newlines"));
> goto error;
> }
> G_GNUC_FALLTHROUGH;
These two - I've already covered in my other series, that's just waiting
for the release to be pushed:
https://listman.redhat.com/archives/libvir-list/2023-August/241416.html
> diff --git a/tests/metadatatest.c b/tests/metadatatest.c
> index b56428fde5..7136730e6a 100644
> --- a/tests/metadatatest.c
> +++ b/tests/metadatatest.c
> @@ -113,8 +113,7 @@ verifyMetadata(virDomainPtr dom,
>
> if (STRNEQ(metadataAPI, expectAPI)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - "XML metadata in API doesn't match expected
> metadata: "
> - "expected:\n[%s]\ngot:\n[%s]",
> + "XML metadata in API doesn't match expected
> metadata: expected:\n[%s]\ngot:\n[%s]",
> expectAPI, metadataAPI);
> return false;
> }
NACK to this one and the rest. If message contains '\n' character then
it's okay if it is on multiple lines. The sole reason we want error
messages on one line is: whenever I see a bug report I can select
"random" substring of the error message and pass it to 'git grep'. If a
message is split on multiple lines though I have to play guessing game
and try to find the exact point where the message was split.
For instance, in the previous two hunks, I can do (after my series is
merged):
git grep "metadata title can't contain newlines"
git grep "metadata title can't contain"
git grep "can't contain newlines"
and all would match the same lines. But if I would do that now, the last
one would match nothing.
But with error messages that contain '\n' it's obviously wrong if I'd
pick a substring containing '\n' and pass it to git grep.
Hence, this hunk and the rest is wrong and make code worse. Sorry.
Michal
Re: [libvirt PATCH] ci: lcitool: Add libvirt-tck+runtime deps list
On Mon, Aug 28, 2023 at 10:50:16AM +0200, Erik Skultety wrote: > This change was supposed to be part of commit 120a674f , but was > proposed against the libvirt TCK project instead. Since we're running > the TCK test suite as part of this project, this is the right place for > the TCK runtime deps list config. Well, at least it can live here until we have a better option, such as ability to reference a remotely defined set of deps. > Signed-off-by: Erik Skultety > --- > ci/lcitool/projects/libvirt-tck+runtime.yml | 32 + > 1 file changed, 32 insertions(+) > create mode 100644 ci/lcitool/projects/libvirt-tck+runtime.yml Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC] Adding timestamp to guest's serial console log
On Tue, Aug 29, 2023 at 11:11:09 +0200, Michal Prívozník wrote: > On 8/29/23 09:04, Shaleen Bathla wrote: > > ping > > > > -Original Message- > > From: Shaleen Bathla > > Sent: Tuesday, June 20, 2023 2:44 PM > > To: libvir-list@redhat.com > > Subject: [RFC] Adding timestamp to guest's serial console log > > > > Hi, > > > > Need some comments regarding the following feature : > > Addition of timestamp support for serial console logs of a guest. > > > > We can implement it as a configurable attribute in xml. > > For example : > > > > > > > > > > > > > > > > We can add a timestamp after every '\n' character received from qemu. > > Can I have some comments regarding this change like what I should keep > > in mind while implementing, whether it is a welcome addition or not, > > issues I might face, any qemu changes required. > > > > Yeah, this might be useful. We definitely need additional attribute to > control this behavior, otherwise we might break some scenarios (e.g. > where users transfer binary data via this channel. > > What I worry though, is that with stdio_handler="file", i.e. when the > file is passed to QEMU directly, then this knob won't work. So we would > need some changes to QEMU. But then, taking this idea further, if QEMU > is changed, then does it make sense to have the same code in libvirt > (for stdio_handler='logd' case)? Nowadays I don't think that anybody disables logd. You can always report an error if the configuration is not supported. Note that '' is a separate output from qemu, so adding timestamps to that only (via logd) will not break anything trying to communicate via the real PTY side. Just the log file will be binary data intermixed with timestamps at certain points.
Re: [RFC] Adding timestamp to guest's serial console log
On Tue, Aug 29, 2023 at 11:11:09AM +0200, Michal Prívozník wrote: > On 8/29/23 09:04, Shaleen Bathla wrote: > > ping > > > > -Original Message- > > From: Shaleen Bathla > > Sent: Tuesday, June 20, 2023 2:44 PM > > To: libvir-list@redhat.com > > Subject: [RFC] Adding timestamp to guest's serial console log > > > > Hi, > > > > Need some comments regarding the following feature : > > Addition of timestamp support for serial console logs of a guest. > > > > We can implement it as a configurable attribute in xml. > > For example : > > > > > > > > > > > > > > > > We can add a timestamp after every '\n' character received from qemu. > > Can I have some comments regarding this change like what I should keep > > in mind while implementing, whether it is a welcome addition or not, > > issues I might face, any qemu changes required. > > > > Yeah, this might be useful. We definitely need additional attribute to > control this behavior, otherwise we might break some scenarios (e.g. > where users transfer binary data via this channel. > > What I worry though, is that with stdio_handler="file", i.e. when the > file is passed to QEMU directly, then this knob won't work. So we would > need some changes to QEMU. But then, taking this idea further, if QEMU > is changed, then does it make sense to have the same code in libvirt > (for stdio_handler='logd' case)? We could declare handler=file to be unsupported for timestamps instead of changing QEMU. I'm not too fussed whether it lives in libvirt or QEMU though With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC] Adding timestamp to guest's serial console log
On Tue, Aug 29, 2023 at 07:04:22AM +, Shaleen Bathla wrote: > ping > > -Original Message- > From: Shaleen Bathla > Sent: Tuesday, June 20, 2023 2:44 PM > To: libvir-list@redhat.com > Subject: [RFC] Adding timestamp to guest's serial console log > > Hi, > > Need some comments regarding the following feature : > Addition of timestamp support for serial console logs of a guest. > > We can implement it as a configurable attribute in xml. > For example : > > > > > > > > We can add a timestamp after every '\n' character received from qemu. > Can I have some comments regarding this change like what I should keep > in mind while implementing, whether it is a welcome addition or not, > issues I might face, any qemu changes required. QEMU does not have ability to add timestamps currently. If it was to be added by libvirt, it will need logic added to virtlogd and would obviously be unavailable in any deployment that disables virtlogd. We recommend using virtlogd to prevent guest denial of service by consuming all disk space, so not a big problem. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC] Adding timestamp to guest's serial console log
On 8/29/23 09:04, Shaleen Bathla wrote: > ping > > -Original Message- > From: Shaleen Bathla > Sent: Tuesday, June 20, 2023 2:44 PM > To: libvir-list@redhat.com > Subject: [RFC] Adding timestamp to guest's serial console log > > Hi, > > Need some comments regarding the following feature : > Addition of timestamp support for serial console logs of a guest. > > We can implement it as a configurable attribute in xml. > For example : > > > > > > > > We can add a timestamp after every '\n' character received from qemu. > Can I have some comments regarding this change like what I should keep > in mind while implementing, whether it is a welcome addition or not, > issues I might face, any qemu changes required. > Yeah, this might be useful. We definitely need additional attribute to control this behavior, otherwise we might break some scenarios (e.g. where users transfer binary data via this channel. What I worry though, is that with stdio_handler="file", i.e. when the file is passed to QEMU directly, then this knob won't work. So we would need some changes to QEMU. But then, taking this idea further, if QEMU is changed, then does it make sense to have the same code in libvirt (for stdio_handler='logd' case)? QEMU already has -msg timestamp=on, which controls timestamps for log messages (/var/log/libvirt/qemu/$dom.log). But that may be orthogonal to what you are trying to achieve. Michal
Re: [libvirt PATCH 2/2] tools: fix VMSA construction with explicit CPU family/model/stepping
On Fri, Aug 25, 2023 at 01:52:58PM +0100, Daniel P. Berrangé wrote: > If the CPU family/model/stepping are provided on the command line, but > the firmware is being automatically extracted from the libvirt guest, > we try to build the VMSA too early. This leads to an exception trying > to parse the firmware that has not been loaded yet. We must delay > building the VMSA in that scenario. > > Signed-off-by: Daniel P. Berrangé Reviewed-by: Erik Skultety
Re: [libvirt PATCH 1/2] tools: fix handling of CPU family/model/stepping in SEV validation
On Fri, Aug 25, 2023 at 01:52:57PM +0100, Daniel P. Berrangé wrote:
> The SEV-ES boot measurement includes the initial CPU register state
> (VMSA) and one of the fields includes the CPU identification. When
> building a VMSA blob we get the CPU family/model/stepping from the
> host capabilities, however, the VMSA must reflect the guest CPU not
> host CPU. Thus using host capabilities is only when whe the guest
> has the 'host-passthrough' CPU mode active. With 'host-model' it is
> cannot be assumed host and guest match, because QEMU may not (yet)
> have a named CPU model for a given host CPU.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> tools/virt-qemu-sev-validate | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/tools/virt-qemu-sev-validate b/tools/virt-qemu-sev-validate
> index 209f19a4a8..c279741004 100755
> --- a/tools/virt-qemu-sev-validate
> +++ b/tools/virt-qemu-sev-validate
> @@ -1054,6 +1054,11 @@ class LibvirtConfidentialVM(ConfidentialVM):
> raise InsecureUsageException(
> "Using CPU SKU from capabilities is not secure")
>
> +mode = doc.xpath("/domain/cpu/@mode")
> +if mode != "host-passthrough":
> +raise UnsupportedUsageException(
> +"Using CPU family/model/stepping from host not possible
> unless 'host-passthrough' is used")
> +
> sig = capsdoc.xpath("/capabilities/host/cpu/signature")
> if len(sig) != 1:
> raise UnsupportedUsageException(
> --
> 2.41.0
>
Reviewed-by: Erik Skultety
Re: [libvirt PATCH 2/2] tools: fix VMSA construction with explicit CPU family/model/stepping
On Fri, Aug 25, 2023 at 13:52:58 +0100, Daniel P. Berrangé wrote: > If the CPU family/model/stepping are provided on the command line, but > the firmware is being automatically extracted from the libvirt guest, > we try to build the VMSA too early. This leads to an exception trying > to parse the firmware that has not been loaded yet. We must delay > building the VMSA in that scenario. > > Signed-off-by: Daniel P. Berrangé > --- > tools/virt-qemu-sev-validate | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) Reviewed-by: Peter Krempa
Re: [libvirt PATCH 1/2] tools: fix handling of CPU family/model/stepping in SEV validation
On Fri, Aug 25, 2023 at 13:52:57 +0100, Daniel P. Berrangé wrote: > The SEV-ES boot measurement includes the initial CPU register state > (VMSA) and one of the fields includes the CPU identification. When > building a VMSA blob we get the CPU family/model/stepping from the > host capabilities, however, the VMSA must reflect the guest CPU not > host CPU. Thus using host capabilities is only when whe the guest > has the 'host-passthrough' CPU mode active. With 'host-model' it is > cannot be assumed host and guest match, because QEMU may not (yet) > have a named CPU model for a given host CPU. > > Signed-off-by: Daniel P. Berrangé > --- > tools/virt-qemu-sev-validate | 5 + > 1 file changed, 5 insertions(+) Reviewed-by: Peter Krempa
Re: [libvirt PATCH 0/2] tools: some fixes to VMSA construction when validating SEV boot
ping, I'd like these bug fixes in the forthcoming release this week. On Fri, Aug 25, 2023 at 01:52:56PM +0100, Daniel P. Berrangé wrote: > > > Daniel P. Berrangé (2): > tools: fix handling of CPU family/model/stepping in SEV validation > tools: fix VMSA construction with explicit CPU family/model/stepping > > tools/virt-qemu-sev-validate | 21 ++--- > 1 file changed, 14 insertions(+), 7 deletions(-) > > -- > 2.41.0 > With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
RE: [RFC] Adding timestamp to guest's serial console log
ping -Original Message- From: Shaleen Bathla Sent: Tuesday, June 20, 2023 2:44 PM To: libvir-list@redhat.com Subject: [RFC] Adding timestamp to guest's serial console log Hi, Need some comments regarding the following feature : Addition of timestamp support for serial console logs of a guest. We can implement it as a configurable attribute in xml. For example : We can add a timestamp after every '\n' character received from qemu. Can I have some comments regarding this change like what I should keep in mind while implementing, whether it is a welcome addition or not, issues I might face, any qemu changes required. Thanks and Regards, Shaleen Bathla
Re: [libvirt PATCH v7 00/35] Use nbdkit for http/ftp/ssh network drives in libvirt
On Mon, Aug 28, 2023 at 04:44:35PM -0500, Jonathon Jongsma wrote: > This is the seventh version of this patch series. See > https://bugzilla.redhat.com/show_bug.cgi?id=2016527 for more information. > > Note that testing this requires selinux policy changes which are not fully > done, but there is a new policy in development that has allowed me to run with > selinux in enforcing mode for the common cases. See > https://bugzilla.redhat.com/show_bug.cgi?id=2182505 for more information. The > following scenarios should work now with selinux enabled using the selinux > policy from that bug: > - http/https disks > - ssh disks with password authentication > - ssh disks with passwordless keyfile > > The one major thing that doesn't work and is difficult to get working with > selinux enabled is the ssh-agent. This is because there doesn't seem to be any > selinux policy for ssh-agent, so by default the ssh-agent socket is labeled > unconfined_t. We cannot allow access from the libvirt/qemu to unconfined_t > because that would open up access to just about anything on the host. So > additional work will likely be necessary for ssh-agent/libvirt interaction in > the future. Fortunately ssh-agent is something that never was really supported > with the old qemu block driver either, so I think we could potentially merge > this patchset either without the ssh-agent patches or with a note that > ssh-agent won't work with selinux enabled. > > Note also that gitlab CI will not work for this series without changes to the > ci definitions due to the addition of libnbd dependency. As for dependencies in CI, since commit 120a674f25aa6e9e1ff7c2e9527f890f48f0340e you can now add dependencies as part of the patch series as long as the dependency exists in lcitool (which in this case it does). If it doesn't, ideally it should be added directly to upstream lcitool, but there's also the option of using mapping overrides using ci/lcitool/mappings.yml. So, before this series gets merged a standalone commit tweaking ci/lcitool/projects/libvirt.yml should be added - it's a trivial change for which you can assume my R-b. Erik
