[PATCH V2 3/4] Apparmor: Allow reading libnl's classid file
I noticed the following denial messages from apparmor in audit.log when
starting confined VMs via the QEMU driver
type=AVC msg=audit(1623864006.370:837): apparmor="DENIED" operation="open" \
profile="virt-aa-helper" name="/etc/libnl/classid" pid=11265 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
type=AVC msg=audit(1623864006.582:849): apparmor="DENIED" operation="open" \
profile="libvirt-0ca2720d-6cff-48bb-86c2-61ab9a79b6e9" \
name="/etc/libnl/classid" pid=11270 comm="qemu-system-x86" \
requested_mask="r" denied_mask="r" fsuid=107 ouid=0
It is possible for site admins to assign names to classids in this file,
which are then used by all libnl tools, possibly those used by libvirt.
To be on the safe side, allow read access to the file in the virt-aa-helper
profile and the libvirt-qemu abstraction.
Signed-off-by: Jim Fehlig
---
src/security/apparmor/libvirt-qemu | 2 ++
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/security/apparmor/libvirt-qemu
b/src/security/apparmor/libvirt-qemu
index 3e31ed4981..4156428163 100644
--- a/src/security/apparmor/libvirt-qemu
+++ b/src/security/apparmor/libvirt-qemu
@@ -37,6 +37,8 @@
@{PROC}/sys/vm/overcommit_memory r,
# detect hardware capabilities via qemu_getauxval
owner @{PROC}/*/auxv r,
+ # allow reading libnl's classid file
+ /etc/libnl{,-3}/classid r,
# For hostdev access. The actual devices will be added dynamically
/sys/bus/usb/devices/ r,
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index dd18c8ab89..8ebb47596a 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -19,7 +19,8 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
# Used when internally running another command (namely apparmor_parser)
@{PROC}/@{pid}/fd/ r,
- @sysconfdir@/libnl-3/classid r,
+ # allow reading libnl's classid file
+ @sysconfdir@/libnl{,-3}/classid r,
# for gl enabled graphics
/dev/dri/{,*} r,
--
2.31.1
[PATCH V2 2/4] Apparmor: Add profile for virtxend
A new apparmor profile initially derived from the libvirtd profile.
All rules were prefixed with the 'audit' qualifier to verify they
are actually used by virtxend. It turns out that several, beyond
the obvious ones, can be dropped in the resulting virtxend profile.
Signed-off-by: Jim Fehlig
---
src/security/apparmor/meson.build | 1 +
src/security/apparmor/usr.sbin.virtxend.in | 53 ++
2 files changed, 54 insertions(+)
diff --git a/src/security/apparmor/meson.build
b/src/security/apparmor/meson.build
index 56f308bf3a..990f00b4f3 100644
--- a/src/security/apparmor/meson.build
+++ b/src/security/apparmor/meson.build
@@ -2,6 +2,7 @@ apparmor_gen_profiles = [
'usr.lib.libvirt.virt-aa-helper',
'usr.sbin.libvirtd',
'usr.sbin.virtqemud',
+ 'usr.sbin.virtxend',
]
apparmor_gen_profiles_conf = configuration_data()
diff --git a/src/security/apparmor/usr.sbin.virtxend.in
b/src/security/apparmor/usr.sbin.virtxend.in
new file mode 100644
index 00..37c31bb104
--- /dev/null
+++ b/src/security/apparmor/usr.sbin.virtxend.in
@@ -0,0 +1,53 @@
+#include
+
+profile virtxend @sbindir@/virtxend flags=(attach_disconnected) {
+ #include
+ #include
+
+ capability kill,
+ capability setgid,
+ capability setuid,
+
+ network inet stream,
+ network inet dgram,
+ network inet6 stream,
+ network inet6 dgram,
+ network netlink raw,
+ network packet dgram,
+ network packet raw,
+
+ # for --p2p migrations
+ unix (send, receive) type=stream addr=none peer=(label=unconfined addr=none),
+
+ ptrace (read,trace) peer=unconfined,
+
+ signal (send) set=("kill", "term", "hup") peer=unconfined,
+
+ # Very lenient profile for virtxend
+ / r,
+ /** rwmkl,
+
+ /bin/* PUx,
+ /sbin/* PUx,
+ /usr/bin/* PUx,
+ @sbindir@/virtlogd pix,
+ @sbindir@/* PUx,
+ /{usr/,}lib/udev/scsi_id PUx,
+ /usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
+ /usr/{lib,lib64}/xen/bin/* Ux,
+ /usr/{lib,libexec}/xen-*/bin/libxl-save-helper PUx,
+ /usr/{lib,libexec}/xen-*/bin/pygrub PUx,
+
+ # force the use of virt-aa-helper
+ audit deny /{usr/,}sbin/apparmor_parser rwxl,
+ audit deny /etc/apparmor.d/libvirt/** wxl,
+ audit deny /sys/kernel/security/apparmor/features rwxl,
+ audit deny /sys/kernel/security/apparmor/matching rwxl,
+ audit deny /sys/kernel/security/apparmor/.* rwxl,
+ /sys/kernel/security/apparmor/profiles r,
+ @libexecdir@/* PUxr,
+ @libexecdir@/libvirt_parthelper ix,
+ @libexecdir@/libvirt_iohelper ix,
+ /etc/libvirt/hooks/** rmix,
+ /etc/xen/scripts/** rmix,
+}
--
2.31.1
[PATCH V2 0/4] Apparmor: Add profiles for hypervisor daemons
and other improvements. V2 of https://listman.redhat.com/archives/libvir-list/2021-June/msg00456.html Changes since V1: Removed many unneeded capabilities. I used the 'audit' qualifier as suggested by cboltz to verify which capabilities were actually used. It's a difficult task though, as it is nearly impossible for one person to exercise a driver in all the ways thousands of users will push it :-). I was able to whittle the virtxend profile quite a bit since xen doesn't need a lot in the way of host capabilities. Removed patch containing the virtlxcd profile since I'm unable to start any lxc domains with virtlxcd. Added patches to squelch denial messages from the virt-aa-helper profile. Jim Fehlig (4): Apparmor: Add profile for virtqemud Apparmor: Add profile for virtxend Apparmor: Allow reading libnl's classid file Apparmor: Allow reading /etc/ssl/openssl.cnf src/security/apparmor/libvirt-qemu| 5 + src/security/apparmor/meson.build | 2 + .../usr.lib.libvirt.virt-aa-helper.in | 4 +- src/security/apparmor/usr.sbin.virtqemud.in | 135 ++ src/security/apparmor/usr.sbin.virtxend.in| 53 +++ 5 files changed, 198 insertions(+), 1 deletion(-) create mode 100644 src/security/apparmor/usr.sbin.virtqemud.in create mode 100644 src/security/apparmor/usr.sbin.virtxend.in -- 2.31.1
[PATCH V2 4/4] Apparmor: Allow reading /etc/ssl/openssl.cnf
I noticed the following denial when running confined VMs with the QEMU
driver
type=AVC msg=audit(1623865089.263:865): apparmor="DENIED" operation="open" \
profile="virt-aa-helper" name="/etc/ssl/openssl.cnf" pid=12503 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
Allow reading the file by including the openssl abstraction in the
virt-aa-helper profile.
Signed-off-by: Jim Fehlig
---
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index 8ebb47596a..ff1d46bebe 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -2,6 +2,7 @@
profile virt-aa-helper @libexecdir@/virt-aa-helper {
#include
+ #include
# needed for searching directories
capability dac_override,
--
2.31.1
[PATCH V2 1/4] Apparmor: Add profile for virtqemud
A new apparmor profile derived from the libvirtd profile, with non-QEMU
related rules removed. Adopt the libvirt-qemu abstraction to work with
the new profile.
Signed-off-by: Jim Fehlig
---
src/security/apparmor/libvirt-qemu | 3 +
src/security/apparmor/meson.build | 1 +
src/security/apparmor/usr.sbin.virtqemud.in | 135
3 files changed, 139 insertions(+)
diff --git a/src/security/apparmor/libvirt-qemu
b/src/security/apparmor/libvirt-qemu
index 85c9e61d6c..3e31ed4981 100644
--- a/src/security/apparmor/libvirt-qemu
+++ b/src/security/apparmor/libvirt-qemu
@@ -16,9 +16,11 @@
ptrace (readby, tracedby) peer=libvirtd,
ptrace (readby, tracedby) peer=/usr/sbin/libvirtd,
+ ptrace (readby, tracedby) peer=virtqemud,
signal (receive) peer=libvirtd,
signal (receive) peer=/usr/sbin/libvirtd,
+ signal (receive) peer=virtqemud,
/dev/kvm rw,
/dev/net/tun rw,
@@ -221,6 +223,7 @@
# allow connect with openGraphicsFD to work
unix (send, receive) type=stream addr=none peer=(label=libvirtd),
unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd),
+ unix (send, receive) type=stream addr=none peer=(label=virtqemud),
# for gathering information about available host resources
/sys/devices/system/cpu/ r,
diff --git a/src/security/apparmor/meson.build
b/src/security/apparmor/meson.build
index af43780211..56f308bf3a 100644
--- a/src/security/apparmor/meson.build
+++ b/src/security/apparmor/meson.build
@@ -1,6 +1,7 @@
apparmor_gen_profiles = [
'usr.lib.libvirt.virt-aa-helper',
'usr.sbin.libvirtd',
+ 'usr.sbin.virtqemud',
]
apparmor_gen_profiles_conf = configuration_data()
diff --git a/src/security/apparmor/usr.sbin.virtqemud.in
b/src/security/apparmor/usr.sbin.virtqemud.in
new file mode 100644
index 00..b986241c74
--- /dev/null
+++ b/src/security/apparmor/usr.sbin.virtqemud.in
@@ -0,0 +1,135 @@
+#include
+@{LIBVIRT}="libvirt"
+
+profile virtqemud @sbindir@/virtqemud flags=(attach_disconnected) {
+ #include
+ #include
+
+ capability kill,
+ capability net_admin,
+ capability net_raw,
+ capability setgid,
+ capability sys_admin,
+ capability sys_module,
+ capability sys_ptrace,
+ capability sys_pacct,
+ capability sys_nice,
+ capability sys_chroot,
+ capability setuid,
+ capability dac_override,
+ capability dac_read_search,
+ capability fowner,
+ capability chown,
+ capability setpcap,
+ capability mknod,
+ capability fsetid,
+ capability audit_write,
+ capability ipc_lock,
+ capability sys_rawio,
+ capability bpf,
+ capability perfmon,
+
+ # Needed for vfio
+ capability sys_resource,
+
+ mount options=(rw,rslave) -> /,
+ mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/,
+ umount /{var/,}run/libvirt/qemu/*.dev/,
+
+ # libvirt provides any mounts under /dev to qemu namespaces
+ mount options=(rw, move) /dev/ -> /{,var/}run/libvirt/qemu/*.dev/,
+ mount options=(rw, move) /dev/** -> /{,var/}run/libvirt/qemu/*{,/},
+ mount options=(rw, move) /{,var/}run/libvirt/qemu/*.dev/ -> /dev/,
+ mount options=(rw, move) /{,var/}run/libvirt/qemu/*{,/} -> /dev/**,
+
+ network inet stream,
+ network inet dgram,
+ network inet6 stream,
+ network inet6 dgram,
+ network netlink raw,
+ network packet dgram,
+ network packet raw,
+
+ # for --p2p migrations
+ unix (send, receive) type=stream addr=none peer=(label=unconfined addr=none),
+
+ ptrace (read,trace) peer=unconfined,
+ ptrace (read,trace) peer=@{profile_name},
+ ptrace (read,trace) peer=dnsmasq,
+ ptrace (read,trace) peer=/usr/sbin/dnsmasq,
+ ptrace (read,trace) peer=libvirt-*,
+
+ signal (send) peer=dnsmasq,
+ signal (send) peer=/usr/sbin/dnsmasq,
+ signal (read, send) peer=libvirt-*,
+ signal (send) set=("kill", "term") peer=unconfined,
+
+ # For communication/control to qemu-bridge-helper
+ unix (send, receive) type=stream addr=none
peer=(label=libvirtd//qemu_bridge_helper),
+ signal (send) set=("term") peer=libvirtd//qemu_bridge_helper,
+
+ # allow connect with openGraphicsFD, direction reversed in newer versions
+ unix (send, receive) type=stream addr=none
peer=(label=libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*),
+ # unconfined also required if guests run without security module
+ unix (send, receive) type=stream addr=none peer=(label=unconfined),
+
+ # required if guests run unconfined seclabel type='none' but libvirtd is
confined
+ signal (read, send) peer=unconfined,
+
+ # Very lenient profile for libvirtd since we want to first focus on confining
+ # the guests. Guests will have a very restricted profile.
+ / r,
+ /** rwmkl,
+
+ /bin/* PUx,
+ /sbin/* PUx,
+ /usr/bin/* PUx,
+ @sbindir@/virtlogd pix,
+ @sbindir@/* PUx,
+ /{usr/,}lib/udev/scsi_id PUx,
+ /usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx,
+ /usr/{lib,lib64,lib/qemu,libexec}/virtiofsd PUx,
+
+ # Required by nwfilter_ebiptables_driver.c:ebiptablesWriteToTempFile() to
+ # read and run an
Re: [PATCH] test_driver: Implement virDomainGetMessages
[Another one of those lost e-mails] On Wed, Jun 16, 2021 at 05:23:36PM +0800, Luke Yue wrote: On Tue, 2021-06-15 at 10:09 +0200, Martin Kletzander wrote: On Mon, Jun 14, 2021 at 09:13:17PM +0800, Luke Yue wrote: > Signed-off-by: Luke Yue > --- > src/test/test_driver.c | 53 > ++ > 1 file changed, 53 insertions(+) > Similarly to the other patch with virDomainGetSecurityLabelList() this one should also test the added code. Thanks, Martin I will try to add test in v2, thanks! You might need to add an extra VM to the default test driver setup which has some taint or something so that it is nicer to test. We will need to add more in the future in any case, not all new APIs will be testable with the current limited set. signature.asc Description: PGP signature
Re: [PATCH] test_driver: Implement virDomainGetSecurityLabelList
[Just found out I got couple of mails lost, so resending even though it was sent a week ago] On Wed, Jun 16, 2021 at 05:21:17PM +0800, Luke Yue wrote: On Tue, 2021-06-15 at 10:08 +0200, Martin Kletzander wrote: On Mon, Jun 14, 2021 at 09:12:57PM +0800, Luke Yue wrote: > Signed-off-by: Luke Yue > --- > src/test/test_driver.c | 41 > + > 1 file changed, 41 insertions(+) > This patch looks fine, but it would be good to have tests for it also. The good thing is that thanks to the fact that this is a test driver API the check can be done using just virsh, no need to write tests and mock syscalls. The previous patches added at least some checks, because it was already in some other test codepath, but this (and later ones as well) are going to need to have some new ones added. Thanks for the review! It seems that there is no command in virsh uses virDomainGetSecurityLabelList, should we add one? Or is there any other testing methods? You can add a command, or you can just write a small program that checks it. The former approach would require a round of design so that it is sensible for virsh to have such command, however the latter approach would add a whole extra binary to the build just to call one API. LLet's see what others think. I think we should definitely test it, especially when it can share most of its codepath with qemu and others. Thanks, Luke signature.asc Description: PGP signature
[libvirt PATCH v2 0/5] mdev tweaks
A few minor fixes to mdev support in the nodedev driver Changes in v2: - split out the error-reporting macro into a separate commit as recommended by Peter - Since virCommandRun() may report an error, ensure that the virMdevctl$COMMAND() functions always set an error to make error-handling consistent. v1 tried to return an error message and have the caller report the error. - Added a new patch (destroying inactive device) Jonathon Jongsma (5): nodedev: Remove useless device name from error message nodedev: Handle NULL command variable nodedev: add macro to handle command errors nodedev: handle mdevctl errors consistently nodedev: improve error message when destroying an inactive device src/node_device/node_device_driver.c | 147 +-- 1 file changed, 94 insertions(+), 53 deletions(-) -- 2.31.1
[libvirt PATCH v2 5/5] nodedev: improve error message when destroying an inactive device
When trying to destroy a node device that is not active, we end up with
a confusing error message:
# nodedev-destroy mdev_88a6b868_46bd_4015_8e5b_26107f82da38
error: Failed to destroy node device
'mdev_88a6b868_46bd_4015_8e5b_26107f82da38'
error: failed to access
'/sys/bus/mdev/devices/88a6b868-46bd-4015-8e5b-26107f82da38/iommu_group': No
such file or directory
With this patch, the error is more clear:
# nodedev-destroy mdev_88a6b868_46bd_4015_8e5b_26107f82da38
error: Failed to destroy node device
'mdev_88a6b868_46bd_4015_8e5b_26107f82da38'
error: Requested operation is not valid: Device
'mdev_88a6b868_46bd_4015_8e5b_26107f82da38' is not active
Signed-off-by: Jonathon Jongsma
---
src/node_device/node_device_driver.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 497db0006a..721ba96203 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1227,6 +1227,15 @@ nodeDeviceDestroy(virNodeDevicePtr device)
ret = 0;
} else if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
+g_autofree char *vfiogroup = NULL;
+VIR_AUTOCLOSE fd = -1;
+
+if (!virNodeDeviceObjIsActive(obj)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _("Device '%s' is not active"), def->name);
+goto cleanup;
+}
+
/* If this mediated device is in use by a vm, attempting to stop it
* will block until the vm closes the device. The nodedev driver
* cannot query the hypervisor driver to determine whether the device
@@ -1236,10 +1245,7 @@ nodeDeviceDestroy(virNodeDevicePtr device)
* to be opened by one user at a time. So if we get EBUSY when opening
* the group, we infer that the device is in use and therefore we
* shouldn't try to remove the device. */
-g_autofree char *vfiogroup =
-virMediatedDeviceGetIOMMUGroupDev(def->caps->data.mdev.uuid);
-VIR_AUTOCLOSE fd = -1;
-
+vfiogroup =
virMediatedDeviceGetIOMMUGroupDev(def->caps->data.mdev.uuid);
if (!vfiogroup)
goto cleanup;
--
2.31.1
[libvirt PATCH v2 2/5] nodedev: Handle NULL command variable
In commit 68580a51, I removed the checks for NULL cmd variables because virCommandRun() already handles the case where it is called with a NULL cmd. Unfortunately, it handles this case by raising a generic error which is both unhelpful and overwrites our existing error message. So for example, when I attempt to create a mediated device with an invalid parent, I get the following output: virsh # nodedev-create mdev-test.xml error: Failed to create node device from mdev-test.xml error: internal error: invalid use of command API With this patch, I now get a useful error message again: virsh # nodedev-create mdev-test.xml error: Failed to create node device from mdev-test.xml error: internal error: unable to find parent device 'pci__00_03_0' Signed-off-by: Jonathon Jongsma --- src/node_device/node_device_driver.c | 16 1 file changed, 16 insertions(+) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 0f13cb4849..43a8c1bf60 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -799,6 +799,10 @@ virMdevctlCreate(virNodeDeviceDef *def, char **uuid, char **errmsg) MDEVCTL_CMD_CREATE, uuid, errmsg); + +if (!cmd) +return -1; + /* an auto-generated uuid is returned via stdout if no uuid is specified in * the mdevctl args */ if (virCommandRun(cmd, ) < 0 || status != 0) @@ -819,6 +823,9 @@ virMdevctlDefine(virNodeDeviceDef *def, char **uuid, char **errmsg) MDEVCTL_CMD_DEFINE, uuid, errmsg); +if (!cmd) +return -1; + /* an auto-generated uuid is returned via stdout if no uuid is specified in * the mdevctl args */ if (virCommandRun(cmd, ) < 0 || status != 0) @@ -925,6 +932,9 @@ virMdevctlStop(virNodeDeviceDef *def, char **errmsg) cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_STOP, NULL, errmsg); +if (!cmd) +return -1; + if (virCommandRun(cmd, ) < 0 || status != 0) return -1; @@ -940,6 +950,9 @@ virMdevctlUndefine(virNodeDeviceDef *def, char **errmsg) cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_UNDEFINE, NULL, errmsg); +if (!cmd) +return -1; + if (virCommandRun(cmd, ) < 0 || status != 0) return -1; @@ -955,6 +968,9 @@ virMdevctlStart(virNodeDeviceDef *def, char **errmsg) cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_START, NULL, errmsg); +if (!cmd) +return -1; + if (virCommandRun(cmd, ) < 0 || status != 0) return -1; -- 2.31.1
[libvirt PATCH v2 4/5] nodedev: handle mdevctl errors consistently
Currently, we have three different types of mdevctl errors:
1. the command cannot be constructed ecause of unsatisfied
preconditions
2. the command cannot be executed due to some error
3. the command is executed, but returns an error status
These different failures are handled differently. Some cases set an
error and return and error status, and some return a error message but
do not set an error.
This means that the caller has to check both whether the return value is
negative and whether the errmsg parameter is non-NULL before deciding
whether to report the error or not. The situation is further complicated
by the fact that there are occasional instances where mdevctl exits with
an error status but does not print an error message. This results in
errmsg being an empty string "" (i.e. non-NULL).
Simplify the situation by ensuring that virReportError() is called for
all error conditions rather than returning an error message back to the
calling function.
Signed-off-by: Jonathon Jongsma
---
src/node_device/node_device_driver.c | 114 +++
1 file changed, 65 insertions(+), 49 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index eb85cc0439..497db0006a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -746,6 +746,8 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
case MDEVCTL_CMD_LAST:
default:
/* SHOULD NEVER HAPPEN */
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown Command '%i'"), cmd_type);
return NULL;
}
@@ -795,48 +797,62 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
static int
-virMdevctlCreate(virNodeDeviceDef *def, char **uuid, char **errmsg)
+virMdevctlCreate(virNodeDeviceDef *def, char **uuid)
{
int status;
+g_autofree char *errmsg = NULL;
g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlCommand(def,
MDEVCTL_CMD_CREATE,
uuid,
-errmsg);
+);
if (!cmd)
return -1;
/* an auto-generated uuid is returned via stdout if no uuid is specified in
* the mdevctl args */
-if (virCommandRun(cmd, ) < 0 || status != 0)
+if (virCommandRun(cmd, ) < 0)
+return -1;
+
+if (status != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to start mediated device: %s"),
+ MDEVCTL_ERROR(errmsg));
return -1;
+}
/* remove newline */
*uuid = g_strstrip(*uuid);
-
return 0;
}
static int
-virMdevctlDefine(virNodeDeviceDef *def, char **uuid, char **errmsg)
+virMdevctlDefine(virNodeDeviceDef *def, char **uuid)
{
int status;
+g_autofree char *errmsg = NULL;
g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlCommand(def,
MDEVCTL_CMD_DEFINE,
-uuid, errmsg);
+uuid, );
if (!cmd)
return -1;
/* an auto-generated uuid is returned via stdout if no uuid is specified in
* the mdevctl args */
-if (virCommandRun(cmd, ) < 0 || status != 0)
+if (virCommandRun(cmd, ) < 0)
return -1;
+if (status != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to define mediated device: %s"),
+ MDEVCTL_ERROR(errmsg));
+return -1;
+}
+
/* remove newline */
*uuid = g_strstrip(*uuid);
-
return 0;
}
@@ -846,7 +862,6 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
virNodeDeviceDef *def)
{
g_autofree char *uuid = NULL;
-g_autofree char *errmsg = NULL;
if (!def->parent) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -854,11 +869,7 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
return NULL;
}
-if (virMdevctlCreate(def, , ) < 0) {
-if (errmsg)
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to start mediated device: %s"),
- errmsg);
+if (virMdevctlCreate(def, ) < 0) {
return NULL;
}
@@ -928,55 +939,79 @@ nodeDeviceCreateXML(virConnectPtr conn,
static int
-virMdevctlStop(virNodeDeviceDef *def, char **errmsg)
+virMdevctlStop(virNodeDeviceDef *def)
{
int status;
g_autoptr(virCommand) cmd = NULL;
+g_autofree char *errmsg = NULL;
-cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_STOP, NULL, errmsg);
+cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_STOP, NULL, );
if (!cmd)
return -1;
-if (virCommandRun(cmd, ) < 0 ||
[libvirt PATCH v2 3/5] nodedev: add macro to handle command errors
This macro will be utilized in the following patch. Since mdevctl
commands can fail with or without an error message, this macro makes it
easy to print a fallback error in the case that the error message is not
set.
Signed-off-by: Jonathon Jongsma
---
src/node_device/node_device_driver.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 43a8c1bf60..eb85cc0439 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -57,6 +57,9 @@ VIR_ENUM_IMPL(virMdevctlCommand,
);
+#define MDEVCTL_ERROR(msg) (msg && msg[0] != '\0' ? msg : _("Unknown error"))
+
+
virDrvOpenStatus
nodeConnectOpen(virConnectPtr conn,
virConnectAuthPtr auth G_GNUC_UNUSED,
@@ -1387,7 +1390,7 @@ nodeDeviceUndefine(virNodeDevice *device,
if (virMdevctlUndefine(def, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to undefine mediated device: %s"),
- errmsg && errmsg[0] ? errmsg : "Unknown Error");
+ MDEVCTL_ERROR(errmsg));
goto cleanup;
}
ret = 0;
@@ -1434,7 +1437,7 @@ nodeDeviceCreate(virNodeDevice *device,
if (virMdevctlStart(def, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to create mediated device: %s"),
- errmsg && errmsg[0] ? errmsg : "Unknown Error");
+ MDEVCTL_ERROR(errmsg));
goto cleanup;
}
ret = 0;
--
2.31.1
[libvirt PATCH v2 1/5] nodedev: Remove useless device name from error message
At the point where the error message is emitted, the field def->name is
still set to "new device", so the error message becomes:
Unable to start mediated device 'new device': ...
Since the name doesn't contain anything useful, just omit it from the
error message altogether.
Signed-off-by: Jonathon Jongsma
Reviewed-by: Peter Krempa
---
src/node_device/node_device_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 8a0a2c3847..0f13cb4849 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -847,8 +847,8 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
if (virMdevctlCreate(def, , ) < 0) {
if (errmsg)
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to start mediated device '%s': %s"),
- def->name, errmsg);
+ _("Unable to start mediated device: %s"),
+ errmsg);
return NULL;
}
--
2.31.1
Re: [PATCH] ci: Also perform `brew upgrade` on MacOS
On Wed, Jun 16, 2021 at 06:21:00AM -0700, Andrea Bolognani wrote: On Tue, Jun 15, 2021 at 12:43:39PM +0200, Martin Kletzander wrote: ci: Also perform `brew upgrade` on MacOS s/MacOS/macOS/ But see below for why we might have to change the subject even further. @Andrea: if you have a good explanation you'd like to put in the commit message, I'd me glad to add it (or you can do that as well). Thanks I think something like The base OS image might include outdated contents, and we don't want to get spurious failures caused by bugs that have already been fixed in the respective packages. This is particularly important on macOS, because 'brew install foo' will fail if 'foo' is already installed but outdated: upgrading all packages first ensures we never run into this scenario. would about sum it up. @@ -443,6 +444,7 @@ x64-macos-11-build: CIRRUS_VM_IMAGE_SELECTOR: image CIRRUS_VM_IMAGE_NAME: big-sur-base UPDATE_COMMAND: brew update +UPGRADE_COMMAND: brew upgrade I believe you also need to add UPGRADE_COMMAND: pkg upgrade -y to the FreeBSD jobs: I don't think Cirrus CI would appreciate having a completely empty string in the list of commands it's supposed to run. It does not cause any issues on libnbd setup where the upgrade is run only on macOS. With that squashed in, Reviewed-by: Andrea Bolognani Are you suggesting that I add the `pkg upgrade -y` to FreeBSDs as well here? Because then the commit message would not fit the patch. and thanks for taking care of this :) and sorry for forgetting about this =) -- Andrea Bolognani / Red Hat / Virtualization signature.asc Description: PGP signature
Re: [PATCH v3 1/6] schemas: Make SEV policy on launch security optional
On 6/22/21 10:10 AM, Boris Fiuczynski wrote:
Change launch security policy of type SEV from required to
optional and add a test to ensure the required launch security
policy remains required when launch security type is SEV.
Signed-off-by: Boris Fiuczynski
---
Reviewed-by: Daniel Henrique Barboza
docs/schemas/domaincommon.rng | 12 ---
src/conf/domain_conf.c| 3 +-
...urity-sev-missing-policy.x86_64-2.12.0.err | 1 +
.../launch-security-sev-missing-policy.xml| 34 +++
tests/qemuxml2argvtest.c | 1 +
5 files changed, 46 insertions(+), 5 deletions(-)
create mode 100644
tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
create mode 100644
tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5ea14b6dbf..8c1b6c3a09 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -483,7 +483,9 @@
-sev
+
+ sev
+
@@ -496,9 +498,11 @@
-
-
-
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f65509d8ec..af2fd03d3c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14749,7 +14749,8 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("failed to get launch security policy"));
+ _("failed to get launch security policy for "
+ "launch security type SEV"));
goto error;
}
diff --git a/tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
new file mode 100644
index 00..2019c8bb13
--- /dev/null
+++
b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
@@ -0,0 +1 @@
+XML error: failed to get launch security policy for launch security type SEV
diff --git a/tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
new file mode 100644
index 00..5461b06c9d
--- /dev/null
+++ b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
@@ -0,0 +1,34 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+AQAOQAOQAOQAOQAOAAA
+IHAVENOIDEABUTJUSTPROVIDINGASTRING
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 9df28658b9..ef6afae586 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3459,6 +3459,7 @@ mymain(void)
DO_TEST_CAPS_VER("launch-security-sev", "2.12.0");
DO_TEST_CAPS_VER("launch-security-sev", "6.0.0");
DO_TEST_CAPS_VER("launch-security-sev-missing-platform-info", "2.12.0");
+DO_TEST_CAPS_VER_PARSE_ERROR("launch-security-sev-missing-policy",
"2.12.0");
DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory");
DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages");
Re: [PATCH v3 3/6] conf: refactor launch security to allow more types
On 6/22/21 10:10 AM, Boris Fiuczynski wrote:
Adding virDomainSecDef for general launch security data
and moving virDomainSEVDef as an element for SEV data.
Signed-off-by: Boris Fiuczynski
---
Reviewed-by: Daniel Henrique Barboza
src/conf/domain_conf.c | 127 +++-
src/conf/domain_conf.h | 11 +++-
src/conf/virconftypes.h | 2 +
src/qemu/qemu_cgroup.c | 4 +-
src/qemu/qemu_command.c | 44 +++--
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_firmware.c| 33 ++
src/qemu/qemu_namespace.c | 20 --
src/qemu/qemu_process.c | 33 --
src/qemu/qemu_validate.c| 22 +--
src/security/security_dac.c | 6 +-
11 files changed, 218 insertions(+), 87 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93ec50ff5d..2bd5210a16 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3502,6 +3502,19 @@ virDomainSEVDefFree(virDomainSEVDef *def)
g_free(def);
}
+
+void
+virDomainSecDefFree(virDomainSecDef *def)
+{
+if (!def)
+return;
+
+virDomainSEVDefFree(def->sev);
+
+g_free(def);
+}
+
+
static void
virDomainOSDefClear(virDomainOSDef *os)
{
@@ -3703,7 +3716,7 @@ void virDomainDefFree(virDomainDef *def)
if (def->namespaceData && def->ns.free)
(def->ns.free)(def->namespaceData);
-virDomainSEVDefFree(def->sev);
+virDomainSecDefFree(def->sec);
xmlFreeNode(def->metadata);
@@ -14720,57 +14733,72 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
unsigned long policy;
-g_autofree char *type = NULL;
int rc = -1;
g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1);
ctxt->node = sevNode;
-if (!(type = virXMLPropString(sevNode, "type"))) {
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing launch security type"));
+ _("failed to get launch security policy for "
+ "launch security type SEV"));
return NULL;
}
-def->sectype = virDomainLaunchSecurityTypeFromString(type);
-switch ((virDomainLaunchSecurity) def->sectype) {
-case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("failed to get launch security policy for "
- "launch security type SEV"));
-return NULL;
-}
+/* the following attributes are platform dependent and if missing,
+ * we can autofill them from domain capabilities later
+ */
+rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
+if (rc == 0) {
+def->haveCbitpos = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security cbitpos"));
+return NULL;
+}
-/* the following attributes are platform dependent and if missing,
- * we can autofill them from domain capabilities later
- */
-rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
-if (rc == 0) {
-def->haveCbitpos = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security cbitpos"));
-return NULL;
-}
+rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
+ >reduced_phys_bits);
+if (rc == 0) {
+def->haveReducedPhysBits = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security "
+ "reduced-phys-bits"));
+return NULL;
+}
-rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
- >reduced_phys_bits);
-if (rc == 0) {
-def->haveReducedPhysBits = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security "
- "reduced-phys-bits"));
-return NULL;
-}
+def->policy = policy;
+def->dh_cert = virXPathString("string(./dhCert)", ctxt);
+def->session = virXPathString("string(./session)", ctxt);
+
+return g_steal_pointer();
+}
+
+
+static virDomainSecDef *
+virDomainSecDefParseXML(xmlNodePtr lsecNode,
+xmlXPathContextPtr ctxt)
+{
+g_autoptr(virDomainSecDef) sec = g_new0(virDomainSecDef, 1);
+g_autofree char *type = NULL;
-def->policy = policy;
-def->dh_cert = virXPathString("string(./dhCert)", ctxt);
-def->session = virXPathString("string(./session)", ctxt);
+ctxt->node
Re: [PATCH v3 2/6] conf: modernize SEV XML parse and format methods
On 6/22/21 10:10 AM, Boris Fiuczynski wrote:
Make use of virDomainLaunchSecurity enum and automatic memory freeing.
Signed-off-by: Boris Fiuczynski
---
Reviewed-by: Daniel Henrique Barboza
src/conf/domain_conf.c | 123 +
src/conf/domain_conf.h | 2 +
2 files changed, 64 insertions(+), 61 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index af2fd03d3c..93ec50ff5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
}
-static void
+void
virDomainSEVDefFree(virDomainSEVDef *def)
{
if (!def)
@@ -14719,73 +14719,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
xmlXPathContextPtr ctxt)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-virDomainSEVDef *def;
unsigned long policy;
g_autofree char *type = NULL;
int rc = -1;
-def = g_new0(virDomainSEVDef, 1);
+g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1);
ctxt->node = sevNode;
if (!(type = virXMLPropString(sevNode, "type"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing launch security type"));
-goto error;
+return NULL;
}
def->sectype = virDomainLaunchSecurityTypeFromString(type);
switch ((virDomainLaunchSecurity) def->sectype) {
case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-break;
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("failed to get launch security policy for "
+ "launch security type SEV"));
+return NULL;
+}
+
+/* the following attributes are platform dependent and if missing,
+ * we can autofill them from domain capabilities later
+ */
+rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
+if (rc == 0) {
+def->haveCbitpos = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security cbitpos"));
+return NULL;
+}
+
+rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
+ >reduced_phys_bits);
+if (rc == 0) {
+def->haveReducedPhysBits = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security "
+ "reduced-phys-bits"));
+return NULL;
+}
+
+def->policy = policy;
+def->dh_cert = virXPathString("string(./dhCert)", ctxt);
+def->session = virXPathString("string(./session)", ctxt);
+
+return g_steal_pointer();
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
default:
virReportError(VIR_ERR_XML_ERROR,
_("unsupported launch security type '%s'"),
type);
-goto error;
-}
-
-if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("failed to get launch security policy for "
- "launch security type SEV"));
-goto error;
-}
-
-/* the following attributes are platform dependent and if missing, we can
- * autofill them from domain capabilities later
- */
-rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
-if (rc == 0) {
-def->haveCbitpos = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security cbitpos"));
-goto error;
-}
-
-rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
- >reduced_phys_bits);
-if (rc == 0) {
-def->haveReducedPhysBits = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security "
- "reduced-phys-bits"));
-goto error;
+return NULL;
}
-
-def->policy = policy;
-def->dh_cert = virXPathString("string(./dhCert)", ctxt);
-def->session = virXPathString("string(./session)", ctxt);
-
-return def;
-
- error:
-virDomainSEVDefFree(def);
-return NULL;
}
@@ -26844,25 +26837,33 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev)
if (!sev)
return;
-virBufferAsprintf(buf, "\n",
- virDomainLaunchSecurityTypeToString(sev->sectype));
-virBufferAdjustIndent(buf, 2);
+switch ((virDomainLaunchSecurity) sev->sectype) {
+case VIR_DOMAIN_LAUNCH_SECURITY_SEV: {
+virBufferAsprintf(buf, "\n",
+
Re: [libvirt PATCH 6/7] nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
On 6/22/21 5:08 PM, Jonathon Jongsma wrote: On Tue, Jun 22, 2021 at 10:03 AM Boris Fiuczynski wrote: On 6/22/21 4:33 PM, Jonathon Jongsma wrote: So it appears that there is a bug where an mdev is still marked as autostart even after it's undefined. Was there anything else you were trying to demonstrate? Jonathon Don't you need to resync with mdevctl on nodedev-info? If you would resync and fail to find the definition for the mdev your bugfix would be to set the autostart to "no". The shortcut would be to set autostart to "no" when undefine is called but that would leave out direct usage of mdectl. Yes, it appears that there's a bug in my patch that needs to be fixed. But your email said "The default "yes" seems to be not really a good choice even for mdevs. That makes it sound like you think there's something more fundamentally wrong than just a simple bug where a value didn't get updated properly. Jonathon Is the current code resyncing with mdevctl on nodedev-info calls persisted and autostart? Is using mdevctl directly something libvirt needs to worry about with regard to data consistency? -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [libvirt PATCH 6/7] nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
On Tue, Jun 22, 2021 at 10:03 AM Boris Fiuczynski wrote: > > On 6/22/21 4:33 PM, Jonathon Jongsma wrote: > > So it appears that there is a bug where an mdev is still marked as > > autostart even after it's undefined. Was there anything else you were > > trying to demonstrate? > > > > Jonathon > > Don't you need to resync with mdevctl on nodedev-info? > If you would resync and fail to find the definition for the mdev your > bugfix would be to set the autostart to "no". The shortcut would be to > set autostart to "no" when undefine is called but that would leave out > direct usage of mdectl. Yes, it appears that there's a bug in my patch that needs to be fixed. But your email said > "The default "yes" seems to be not really a good choice even for mdevs. That makes it sound like you think there's something more fundamentally wrong than just a simple bug where a value didn't get updated properly. Jonathon
Re: [libvirt PATCH] nodedev: handle mdevs from multiple parents
ping
On Thu, Jun 10, 2021 at 1:18 PM Jonathon Jongsma wrote:
>
> Due to a rather unfortunate misunderstanding, we were parsing the list
> of defined devices from mdevctl incorrectly. Since my primary
> development machine only has a single device capable of mdevs, I
> apparently neglected to test multiple parent devices and made some
> assumptions based on reading the mdevctl code. These assumptions turned
> out to be incorrect, so the parsing failed when devices from more than
> one parent device were returned.
>
> The details: mdevctl returns an array of objects representing the
> defined devices. But instead of an array of multiple objects (with each
> object representing a parent device), the array always contains only a
> single object. That object has a separate property for each parent
> device.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/node_device/node_device_driver.c | 41 ++-
> .../mdevctl-list-multiple.json| 4 +-
> 2 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index 8a0a2c3847..cb2c3ceaa4 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1056,6 +1056,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
> size_t noutdevs = 0;
> size_t i;
> size_t j;
> +virJSONValue *obj;
>
> json_devicelist = virJSONValueFromString(jsonstring);
>
> @@ -1065,31 +1066,33 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
> goto error;
> }
>
> -n = virJSONValueArraySize(json_devicelist);
> +/* mdevctl list --dumpjson produces an output that is an array that
> + * contains only a single object which contains a property for each
> parent
> + * device */
> +if (virJSONValueArraySize(json_devicelist) != 1) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Unexpected format for mdevctl response"));
> +goto error;
> +}
> +
> +obj = virJSONValueArrayGet(json_devicelist, 0);
> +
> +if (!virJSONValueIsObject(obj)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("device list is not an object"));
> +goto error;
> +}
>
> +n = virJSONValueObjectKeysNumber(obj);
> for (i = 0; i < n; i++) {
> -virJSONValue *obj = virJSONValueArrayGet(json_devicelist, i);
> const char *parent;
> virJSONValue *child_array;
> int nchildren;
>
> -if (!virJSONValueIsObject(obj)) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Parent device is not an object"));
> -goto error;
> -}
> -
> -/* mdevctl returns an array of objects. Each object is a parent
> device
> - * object containing a single key-value pair which maps from the name
> - * of the parent device to an array of child devices */
> -if (virJSONValueObjectKeysNumber(obj) != 1) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Unexpected format for parent device object"));
> -goto error;
> -}
> -
> -parent = virJSONValueObjectGetKey(obj, 0);
> -child_array = virJSONValueObjectGetValue(obj, 0);
> +/* The key of each object property is the name of a parent device
> + * which maps to an array of child devices */
> +parent = virJSONValueObjectGetKey(obj, i);
> +child_array = virJSONValueObjectGetValue(obj, i);
>
> if (!virJSONValueIsArray(child_array)) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> index eefcd90c62..ca1918d00a 100644
> --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> @@ -24,9 +24,7 @@
>]
> }
>}
> -]
> - },
> - {
> +],
> "matrix": [
>{ "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
> "mdev_type": "vfio_ap-passthrough",
> --
> 2.31.1
>
Re: [libvirt PATCH 6/7] nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
On 6/22/21 4:33 PM, Jonathon Jongsma wrote: So it appears that there is a bug where an mdev is still marked as autostart even after it's undefined. Was there anything else you were trying to demonstrate? Jonathon Don't you need to resync with mdevctl on nodedev-info? If you would resync and fail to find the definition for the mdev your bugfix would be to set the autostart to "no". The shortcut would be to set autostart to "no" when undefine is called but that would leave out direct usage of mdectl. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [libvirt PATCH 6/7] nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
On Tue, Jun 22, 2021 at 2:08 AM Boris Fiuczynski wrote:
>
> On 6/14/21 10:46 PM, Jonathon Jongsma wrote:
> > On Mon, Jun 14, 2021 at 12:27 PM Boris Fiuczynski
> > wrote:
> >>
> >> On 6/3/21 10:11 PM, Jonathon Jongsma wrote:
> >>> Implement these new API functions in the nodedev driver.
> >>>
> >>> Signed-off-by: Jonathon Jongsma
> >>> ---
> >>>src/node_device/node_device_driver.c | 50
> >>>src/node_device/node_device_driver.h | 6
> >>>src/node_device/node_device_udev.c | 21 +++-
> >>>3 files changed, 69 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/src/node_device/node_device_driver.c
> >>> b/src/node_device/node_device_driver.c
> >>> index 9ebe609aa4..75391f18b8 100644
> >>> --- a/src/node_device/node_device_driver.c
> >>> +++ b/src/node_device/node_device_driver.c
> >>> @@ -1804,3 +1804,53 @@ nodeDeviceGetAutostart(virNodeDevice *device,
> >>>virNodeDeviceObjEndAPI();
> >>>return ret;
> >>>}
> >>> +
> >>> +
> >>> +int
> >>> +nodeDeviceIsPersistent(virNodeDevice *device)
> >>> +{
> >>> +virNodeDeviceObj *obj = NULL;
> >>> +virNodeDeviceDef *def = NULL;
> >>> +int ret = -1;
> >>> +
> >>> +if (nodeDeviceInitWait() < 0)
> >>> +return -1;
> >>> +
> >>> +if (!(obj = nodeDeviceObjFindByName(device->name)))
> >>> +return -1;
> >>> +def = virNodeDeviceObjGetDef(obj);
> >>> +
> >>> +if (virNodeDeviceIsPersistentEnsureACL(device->conn, def) < 0)
> >>> +goto cleanup;
> >>> +
> >>> +ret = virNodeDeviceObjIsPersistent(obj);
> >>> +
> >>> + cleanup:
> >>> +virNodeDeviceObjEndAPI();
> >>> +return ret;
> >>> +}
> >>> +
> >>> +
> >>> +int
> >>> +nodeDeviceIsActive(virNodeDevice *device)
> >>> +{
> >>> +virNodeDeviceObj *obj = NULL;
> >>> +virNodeDeviceDef *def = NULL;
> >>> +int ret = -1;
> >>> +
> >>> +if (nodeDeviceInitWait() < 0)
> >>> +return -1;
> >>> +
> >>> +if (!(obj = nodeDeviceObjFindByName(device->name)))
> >>> +return -1;
> >>> +def = virNodeDeviceObjGetDef(obj);
> >>> +
> >>> +if (virNodeDeviceIsActiveEnsureACL(device->conn, def) < 0)
> >>> +goto cleanup;
> >>> +
> >>> +ret = virNodeDeviceObjIsActive(obj);
> >>> +
> >>> + cleanup:
> >>> +virNodeDeviceObjEndAPI();
> >>> +return ret;
> >>> +}
> >>> diff --git a/src/node_device/node_device_driver.h
> >>> b/src/node_device/node_device_driver.h
> >>> index d178a18180..744dd42632 100644
> >>> --- a/src/node_device/node_device_driver.h
> >>> +++ b/src/node_device/node_device_driver.h
> >>> @@ -180,6 +180,12 @@ int
> >>>nodeDeviceGetAutostart(virNodeDevice *dev,
> >>> int *autostart);
> >>>
> >>> +int
> >>> +nodeDeviceIsPersistent(virNodeDevice *dev);
> >>> +
> >>> +int
> >>> +nodeDeviceIsActive(virNodeDevice *dev);
> >>> +
> >>>virCommand*
> >>>nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def,
> >>>bool autostart,
> >>> diff --git a/src/node_device/node_device_udev.c
> >>> b/src/node_device/node_device_udev.c
> >>> index 21273083a6..eb15ccce7f 100644
> >>> --- a/src/node_device/node_device_udev.c
> >>> +++ b/src/node_device/node_device_udev.c
> >>> @@ -1487,7 +1487,7 @@ udevAddOneDevice(struct udev_device *device)
> >>>virObjectEvent *event = NULL;
> >>>bool new_device = true;
> >>>int ret = -1;
> >>> -bool was_persistent = false;
> >>> +bool persistent = true;
> >>>bool autostart = true;
> >>>bool is_mdev;
> >>>
> >>> @@ -1518,7 +1518,8 @@ udevAddOneDevice(struct udev_device *device)
> >>>
> >>>if (is_mdev)
> >>>nodeDeviceDefCopyFromMdevctl(def, objdef);
> >>> -was_persistent = virNodeDeviceObjIsPersistent(obj);
> >>> +
> >>> +persistent = virNodeDeviceObjIsPersistent(obj);
> >>>autostart = virNodeDeviceObjIsAutostart(obj);
> >>>
> >>>/* If the device was defined by mdevctl and was never
> >>> instantiated, it
> >>> @@ -1527,11 +1528,12 @@ udevAddOneDevice(struct udev_device *device)
> >>>
> >>>virNodeDeviceObjEndAPI();
> >>>} else {
> >>> -/* All non-mdev devices report themselves as autostart since they
> >>> - * should still be present and active after a reboot unless the
> >>> device
> >>> - * is removed from the host. Mediated devices can only be
> >>> persistent if
> >>> - * they are in already in the device list from parsing the
> >>> mdevctl
> >>> - * output. */
> >>> +/* All non-mdev devices report themselves as persistent and
> >>> autostart
> >>> + * since they should still be present and active after a reboot
> >>> unless
> >>> + * the device is removed from the host. Mediated devices can
> >>> only be
> >>> + * persistent if they are in already in the device list from
> >>> parsing
> >>> + * the mdevctl output. */
>
Re: [libvirt PATCH] docs: Fix some typos
On 6/22/21 3:54 PM, Tim Wiederhake wrote: > Signed-off-by: Tim Wiederhake > --- > docs/kbase/live_full_disk_backup.rst | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Ján Tomko and pushed. Jano
[libvirt PATCH] docs: Fix some typos
Signed-off-by: Tim Wiederhake --- docs/kbase/live_full_disk_backup.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/kbase/live_full_disk_backup.rst b/docs/kbase/live_full_disk_backup.rst index 1605ec05d2..562a9e87b0 100644 --- a/docs/kbase/live_full_disk_backup.rst +++ b/docs/kbase/live_full_disk_backup.rst @@ -97,7 +97,7 @@ virDomainBackupBegin() API: Assuming a guest with a single disk image, create a temporary live QCOW2 overlay (commonly called as "external snapshot") to track the live guest writes. Then backup the original disk image while the guest (live QEMU) keeps writing to the temporary -overlay. Finally, perform the "active block-commit" opertion to +overlay. Finally, perform the "active block-commit" operation to live-merge the temporary overlay disk contents into the original image — i.e. the backing file — and "pivot" the live QEMU process to point to it. @@ -138,7 +138,7 @@ it. you have to explicitly clean the libvirt metadata using ``virsh snapshot-delete vm1 --metadata [name|--current]``. -#. Now, take a backup the orignal image, ``base.raw``, to a different +#. Now, take a backup the original image, ``base.raw``, to a different location using ``cp`` or ``rsync``:: $ cp /var/lib/libvirt/images/base.raw -- 2.31.1
[PATCH v3 2/6] conf: modernize SEV XML parse and format methods
Make use of virDomainLaunchSecurity enum and automatic memory freeing.
Signed-off-by: Boris Fiuczynski
---
src/conf/domain_conf.c | 123 +
src/conf/domain_conf.h | 2 +
2 files changed, 64 insertions(+), 61 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index af2fd03d3c..93ec50ff5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
}
-static void
+void
virDomainSEVDefFree(virDomainSEVDef *def)
{
if (!def)
@@ -14719,73 +14719,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
xmlXPathContextPtr ctxt)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-virDomainSEVDef *def;
unsigned long policy;
g_autofree char *type = NULL;
int rc = -1;
-def = g_new0(virDomainSEVDef, 1);
+g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1);
ctxt->node = sevNode;
if (!(type = virXMLPropString(sevNode, "type"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing launch security type"));
-goto error;
+return NULL;
}
def->sectype = virDomainLaunchSecurityTypeFromString(type);
switch ((virDomainLaunchSecurity) def->sectype) {
case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-break;
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("failed to get launch security policy for "
+ "launch security type SEV"));
+return NULL;
+}
+
+/* the following attributes are platform dependent and if missing,
+ * we can autofill them from domain capabilities later
+ */
+rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
+if (rc == 0) {
+def->haveCbitpos = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security cbitpos"));
+return NULL;
+}
+
+rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
+ >reduced_phys_bits);
+if (rc == 0) {
+def->haveReducedPhysBits = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security "
+ "reduced-phys-bits"));
+return NULL;
+}
+
+def->policy = policy;
+def->dh_cert = virXPathString("string(./dhCert)", ctxt);
+def->session = virXPathString("string(./session)", ctxt);
+
+return g_steal_pointer();
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
default:
virReportError(VIR_ERR_XML_ERROR,
_("unsupported launch security type '%s'"),
type);
-goto error;
-}
-
-if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("failed to get launch security policy for "
- "launch security type SEV"));
-goto error;
-}
-
-/* the following attributes are platform dependent and if missing, we can
- * autofill them from domain capabilities later
- */
-rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
-if (rc == 0) {
-def->haveCbitpos = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security cbitpos"));
-goto error;
-}
-
-rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
- >reduced_phys_bits);
-if (rc == 0) {
-def->haveReducedPhysBits = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security "
- "reduced-phys-bits"));
-goto error;
+return NULL;
}
-
-def->policy = policy;
-def->dh_cert = virXPathString("string(./dhCert)", ctxt);
-def->session = virXPathString("string(./session)", ctxt);
-
-return def;
-
- error:
-virDomainSEVDefFree(def);
-return NULL;
}
@@ -26844,25 +26837,33 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef
*sev)
if (!sev)
return;
-virBufferAsprintf(buf, "\n",
- virDomainLaunchSecurityTypeToString(sev->sectype));
-virBufferAdjustIndent(buf, 2);
+switch ((virDomainLaunchSecurity) sev->sectype) {
+case VIR_DOMAIN_LAUNCH_SECURITY_SEV: {
+virBufferAsprintf(buf, "\n",
+ virDomainLaunchSecurityTypeToString(sev->sectype));
+virBufferAdjustIndent(buf, 2);
-if (sev->haveCbitpos)
-virBufferAsprintf(buf, "%d\n",
[PATCH v3 1/6] schemas: Make SEV policy on launch security optional
Change launch security policy of type SEV from required to
optional and add a test to ensure the required launch security
policy remains required when launch security type is SEV.
Signed-off-by: Boris Fiuczynski
---
docs/schemas/domaincommon.rng | 12 ---
src/conf/domain_conf.c| 3 +-
...urity-sev-missing-policy.x86_64-2.12.0.err | 1 +
.../launch-security-sev-missing-policy.xml| 34 +++
tests/qemuxml2argvtest.c | 1 +
5 files changed, 46 insertions(+), 5 deletions(-)
create mode 100644
tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
create mode 100644
tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5ea14b6dbf..8c1b6c3a09 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -483,7 +483,9 @@
-sev
+
+ sev
+
@@ -496,9 +498,11 @@
-
-
-
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f65509d8ec..af2fd03d3c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14749,7 +14749,8 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("failed to get launch security policy"));
+ _("failed to get launch security policy for "
+ "launch security type SEV"));
goto error;
}
diff --git
a/tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
new file mode 100644
index 00..2019c8bb13
--- /dev/null
+++
b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
@@ -0,0 +1 @@
+XML error: failed to get launch security policy for launch security type SEV
diff --git a/tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
new file mode 100644
index 00..5461b06c9d
--- /dev/null
+++ b/tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
@@ -0,0 +1,34 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+AQAOQAOQAOQAOQAOAAA
+IHAVENOIDEABUTJUSTPROVIDINGASTRING
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 9df28658b9..ef6afae586 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3459,6 +3459,7 @@ mymain(void)
DO_TEST_CAPS_VER("launch-security-sev", "2.12.0");
DO_TEST_CAPS_VER("launch-security-sev", "6.0.0");
DO_TEST_CAPS_VER("launch-security-sev-missing-platform-info", "2.12.0");
+DO_TEST_CAPS_VER_PARSE_ERROR("launch-security-sev-missing-policy",
"2.12.0");
DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory");
DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages");
--
2.30.2
[PATCH v3 5/6] conf: add s390-pv as launch security type
Add launch security type 's390-pv' as well as some tests.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Daniel Henrique Barboza
---
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c| 8 +
src/conf/domain_conf.h| 1 +
src/qemu/qemu_command.c | 26 ++
src/qemu/qemu_firmware.c | 1 +
src/qemu/qemu_namespace.c | 1 +
src/qemu/qemu_process.c | 1 +
src/qemu/qemu_validate.c | 9 +
.../launch-security-s390-pv-ignore-policy.xml | 24 +
.../launch-security-s390-pv.xml | 18 ++
.../launch-security-s390-pv-ignore-policy.xml | 1 +
tests/genericxml2xmltest.c| 2 ++
...ty-s390-pv-ignore-policy.s390x-latest.args | 35 +++
.../launch-security-s390-pv-ignore-policy.xml | 33 +
.../launch-security-s390-pv.s390x-latest.args | 35 +++
.../launch-security-s390-pv.xml | 30
tests/qemuxml2argvtest.c | 3 ++
17 files changed, 229 insertions(+)
create mode 100644
tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml
create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv.xml
create mode 12
tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml
create mode 100644
tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args
create mode 100644
tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml
create mode 100644
tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args
create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8c1b6c3a09..b81c51728d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -485,6 +485,7 @@
sev
+ s390-pv
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2bd5210a16..a7fc8cd65f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1401,6 +1401,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
VIR_DOMAIN_LAUNCH_SECURITY_LAST,
"",
"sev",
+ "s390-pv",
);
static virClass *virDomainObjClass;
@@ -14799,6 +14800,8 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode,
if (!sec->sev)
return NULL;
break;
+case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+break;
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
default:
@@ -26895,6 +26898,11 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef
*sec)
break;
}
+case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+virBufferAsprintf(buf, "\n",
+ virDomainLaunchSecurityTypeToString(sec->sectype));
+break;
+
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fa7ab1895d..9d9acab50c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2645,6 +2645,7 @@ struct _virDomainKeyWrapDef {
typedef enum {
VIR_DOMAIN_LAUNCH_SECURITY_NONE,
VIR_DOMAIN_LAUNCH_SECURITY_SEV,
+VIR_DOMAIN_LAUNCH_SECURITY_PV,
VIR_DOMAIN_LAUNCH_SECURITY_LAST,
} virDomainLaunchSecurity;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4135a8444a..3ab803f7ce 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6975,6 +6975,9 @@ qemuBuildMachineCommandLine(virCommand *cmd,
virBufferAddLit(, ",memory-encryption=sev0");
}
break;
+case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+virBufferAddLit(, ",confidential-guest-support=pv0");
+break;
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
break;
case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
@@ -9870,6 +9873,26 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand
*cmd,
}
+static int
+qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd)
+{
+g_autoptr(virJSONValue) props = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+qemuDomainObjPrivate *priv = vm->privateData;
+
+if (qemuMonitorCreateObjectProps(, "s390-pv-guest", "pv0",
+ NULL) < 0)
+return -1;
+
+if (qemuBuildObjectCommandlineFromJSON(, props, priv->qemuCaps) < 0)
+return -1;
+
+virCommandAddArg(cmd, "-object");
+virCommandAddArgBuffer(cmd, );
+return 0;
+}
+
+
static int
qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd,
virDomainSecDef *sec)
@@ -9881,6 +9904,9 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd,
case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
[PATCH v3 4/6] qemu: add s390-pv-guest capability
Add s390-pv-guest capability.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Daniel Henrique Barboza
---
src/qemu/qemu_capabilities.c| 2 ++
src/qemu/qemu_capabilities.h| 1 +
tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 +
3 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d1cd8f11ac..cc8f61a74a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -636,6 +636,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 405 */
"confidential-guest-support",
"query-display-options",
+ "s390-pv-guest",
);
@@ -1354,6 +1355,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "input-linux", QEMU_CAPS_INPUT_LINUX },
{ "virtio-gpu-gl-pci", QEMU_CAPS_VIRTIO_GPU_GL_PCI },
{ "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL },
+{ "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
};
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 7944b9170a..030467b233 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -616,6 +616,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 405 */
QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine
confidential-guest-support */
QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command
present */
+QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml
index 1806c064c9..aae6364e37 100644
--- a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml
@@ -169,6 +169,7 @@
+
600
0
39100242
--
2.30.2
[PATCH v3 0/6] Support for launchSecurity type s390-pv
This patch series introduces the launch security type s390-pv. Specifying s390-pv as launch security type in an s390 domain prepares for running the guest in protected virtualization secure mode, also known as IBM Secure Execution. diff to v2: - Broke up previous patch one into three patches diff to v1: - Rebased to current master - Added verification check for confidential-guest-support capability Boris Fiuczynski (6): schemas: Make SEV policy on launch security optional conf: modernize SEV XML parse and format methods conf: refactor launch security to allow more types qemu: add s390-pv-guest capability conf: add s390-pv as launch security type docs: add s390-pv documentation docs/formatdomain.rst | 7 + docs/kbase/s390_protected_virt.rst| 55 ++- docs/schemas/domaincommon.rng | 13 +- src/conf/domain_conf.c| 155 +++--- src/conf/domain_conf.h| 14 +- src/conf/virconftypes.h | 2 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_cgroup.c| 4 +- src/qemu/qemu_command.c | 70 +++- src/qemu/qemu_driver.c| 3 +- src/qemu/qemu_firmware.c | 34 ++-- src/qemu/qemu_namespace.c | 21 ++- src/qemu/qemu_process.c | 34 +++- src/qemu/qemu_validate.c | 31 +++- src/security/security_dac.c | 6 +- .../launch-security-s390-pv-ignore-policy.xml | 24 +++ .../launch-security-s390-pv.xml | 18 ++ .../launch-security-s390-pv-ignore-policy.xml | 1 + tests/genericxml2xmltest.c| 2 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + ...ty-s390-pv-ignore-policy.s390x-latest.args | 35 .../launch-security-s390-pv-ignore-policy.xml | 33 .../launch-security-s390-pv.s390x-latest.args | 35 .../launch-security-s390-pv.xml | 30 ...urity-sev-missing-policy.x86_64-2.12.0.err | 1 + .../launch-security-sev-missing-policy.xml| 34 tests/qemuxml2argvtest.c | 4 + 28 files changed, 562 insertions(+), 108 deletions(-) create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv.xml create mode 12 tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.xml create mode 100644 tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err create mode 100644 tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml -- 2.30.2
[PATCH v3 6/6] docs: add s390-pv documentation
Add documentation for launch security type s390-pv.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Daniel Henrique Barboza
---
docs/formatdomain.rst | 7
docs/kbase/s390_protected_virt.rst | 55 +-
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index c6dede053f..a1b028c4ad 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -8078,6 +8078,13 @@ Note: DEA/TDEA is synonymous with DES/TDES.
Launch Security
---
+Specifying in a s390 domain prepares
+the guest to run in protected virtualization secure mode, also known as
+IBM Secure Execution. For more required host and guest preparation steps, see
+`Protected Virtualization on s390 `__
+:since:`Since 7.5.0`
+
+
The contents of the element is used to provide
the guest owners input used for creating an encrypted VM using the AMD SEV
feature (Secure Encrypted Virtualization). SEV is an extension to the AMD-V
diff --git a/docs/kbase/s390_protected_virt.rst
b/docs/kbase/s390_protected_virt.rst
index 1718a556d4..66203568d9 100644
--- a/docs/kbase/s390_protected_virt.rst
+++ b/docs/kbase/s390_protected_virt.rst
@@ -127,10 +127,13 @@ Protected virtualization guests support I/O using virtio
devices.
As the virtio data structures of secure guests are not accessible
by the host, it is necessary to use shared memory ('bounce buffers').
-To enable virtio devices to use shared buffers, it is necessary
-to configure them with platform_iommu enabled. This can done by adding
-``iommu='on'`` to the driver element of a virtio device definition in the
-guest's XML, e.g.
+Since libvirt 7.5.0 the
+` `__
+element with type ``s390-pv`` should be used on protected virtualization
guests.
+Without ``launchSecurity`` you must enable all virtio devices to use shared
+buffers by configuring them with platform_iommu enabled.
+This can done by adding ``iommu='on'`` to the driver element of a virtio
+device definition in the guest's XML, e.g.
::
@@ -140,8 +143,10 @@ guest's XML, e.g.
-It is mandatory to define all virtio bus devices in this way to
-prevent the host from attempting to access protected memory.
+Unless you are using ``launchSecurity`` you must define all virtio bus
+devices in this way to prevent the host from attempting to access
+protected memory.
+
Ballooning will not work and is fenced by QEMU. It should be
disabled by specifying
@@ -158,8 +163,42 @@ allocated 2K entries. A commonly used value for swiotlb is
262144.
Example guest definition
-Minimal domain XML for a protected virtualization guest, essentially
-it's mostly about the ``iommu`` property
+Minimal domain XML for a protected virtualization guest with
+the ``launchSecurity`` element of type ``s390-pv``
+
+::
+
+
+ protected
+ 2048000
+ 2048000
+ 1
+
+ hvm
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+Example guest definition without launchSecurity
+===
+
+Minimal domain XML for a protected virtualization guest using the
+``iommu='on'`` setting for each virtio device.
::
--
2.30.2
[PATCH v3 3/6] conf: refactor launch security to allow more types
Adding virDomainSecDef for general launch security data
and moving virDomainSEVDef as an element for SEV data.
Signed-off-by: Boris Fiuczynski
---
src/conf/domain_conf.c | 127 +++-
src/conf/domain_conf.h | 11 +++-
src/conf/virconftypes.h | 2 +
src/qemu/qemu_cgroup.c | 4 +-
src/qemu/qemu_command.c | 44 +++--
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_firmware.c| 33 ++
src/qemu/qemu_namespace.c | 20 --
src/qemu/qemu_process.c | 33 --
src/qemu/qemu_validate.c| 22 +--
src/security/security_dac.c | 6 +-
11 files changed, 218 insertions(+), 87 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93ec50ff5d..2bd5210a16 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3502,6 +3502,19 @@ virDomainSEVDefFree(virDomainSEVDef *def)
g_free(def);
}
+
+void
+virDomainSecDefFree(virDomainSecDef *def)
+{
+if (!def)
+return;
+
+virDomainSEVDefFree(def->sev);
+
+g_free(def);
+}
+
+
static void
virDomainOSDefClear(virDomainOSDef *os)
{
@@ -3703,7 +3716,7 @@ void virDomainDefFree(virDomainDef *def)
if (def->namespaceData && def->ns.free)
(def->ns.free)(def->namespaceData);
-virDomainSEVDefFree(def->sev);
+virDomainSecDefFree(def->sec);
xmlFreeNode(def->metadata);
@@ -14720,57 +14733,72 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
unsigned long policy;
-g_autofree char *type = NULL;
int rc = -1;
g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1);
ctxt->node = sevNode;
-if (!(type = virXMLPropString(sevNode, "type"))) {
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing launch security type"));
+ _("failed to get launch security policy for "
+ "launch security type SEV"));
return NULL;
}
-def->sectype = virDomainLaunchSecurityTypeFromString(type);
-switch ((virDomainLaunchSecurity) def->sectype) {
-case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("failed to get launch security policy for "
- "launch security type SEV"));
-return NULL;
-}
+/* the following attributes are platform dependent and if missing,
+ * we can autofill them from domain capabilities later
+ */
+rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
+if (rc == 0) {
+def->haveCbitpos = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security cbitpos"));
+return NULL;
+}
-/* the following attributes are platform dependent and if missing,
- * we can autofill them from domain capabilities later
- */
-rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
-if (rc == 0) {
-def->haveCbitpos = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security cbitpos"));
-return NULL;
-}
+rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
+ >reduced_phys_bits);
+if (rc == 0) {
+def->haveReducedPhysBits = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security "
+ "reduced-phys-bits"));
+return NULL;
+}
-rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
- >reduced_phys_bits);
-if (rc == 0) {
-def->haveReducedPhysBits = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security "
- "reduced-phys-bits"));
-return NULL;
-}
+def->policy = policy;
+def->dh_cert = virXPathString("string(./dhCert)", ctxt);
+def->session = virXPathString("string(./session)", ctxt);
+
+return g_steal_pointer();
+}
+
+
+static virDomainSecDef *
+virDomainSecDefParseXML(xmlNodePtr lsecNode,
+xmlXPathContextPtr ctxt)
+{
+g_autoptr(virDomainSecDef) sec = g_new0(virDomainSecDef, 1);
+g_autofree char *type = NULL;
-def->policy = policy;
-def->dh_cert = virXPathString("string(./dhCert)", ctxt);
-def->session = virXPathString("string(./session)", ctxt);
+ctxt->node = lsecNode;
-return g_steal_pointer();
+if (!(type = virXMLPropString(lsecNode, "type"))) {
+
[libvirt PATCH 16/16] virDomainFeaturesDefParse: Simplify APIC parsing
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 50717c4f44..d78f846a52 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17532,7 +17532,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
return -1;
for (i = 0; i < n; i++) {
-g_autofree char *tmp = NULL;
int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name);
if (val < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -17541,18 +17540,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
switch ((virDomainFeature) val) {
-case VIR_DOMAIN_FEATURE_APIC:
-if ((tmp = virXPathString("string(./features/apic/@eoi)", ctxt))) {
-int eoi;
-if ((eoi = virTristateSwitchTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown value for attribute eoi: '%s'"),
- tmp);
-return -1;
-}
-def->apic_eoi = eoi;
-}
-G_GNUC_FALLTHROUGH;
case VIR_DOMAIN_FEATURE_ACPI:
case VIR_DOMAIN_FEATURE_PAE:
case VIR_DOMAIN_FEATURE_VIRIDIAN:
@@ -17560,6 +17547,16 @@ virDomainFeaturesDefParse(virDomainDef *def,
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
+case VIR_DOMAIN_FEATURE_APIC: {
+virTristateSwitch eoi;
+if (virXMLPropTristateSwitch(nodes[i], "eoi", VIR_XML_PROP_NONE,
) < 0)
+return -1;
+
+def->features[val] = VIR_TRISTATE_SWITCH_ON;
+def->apic_eoi = eoi;
+break;
+}
+
case VIR_DOMAIN_FEATURE_MSRS: {
virDomainMsrsUnknown unknown;
if (virXMLPropEnum(nodes[i], "unknown",
--
2.31.1
[libvirt PATCH 13/16] virDomainFeaturesDefParse: Inline MSRS parsing
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b411c1fb8c..915303adcd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17518,10 +17518,21 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_PAE:
case VIR_DOMAIN_FEATURE_VIRIDIAN:
case VIR_DOMAIN_FEATURE_PRIVNET:
-case VIR_DOMAIN_FEATURE_MSRS:
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
+case VIR_DOMAIN_FEATURE_MSRS: {
+virDomainMsrsUnknown unknown;
+if (virXMLPropEnum(nodes[i], "unknown",
+ virDomainMsrsUnknownTypeFromString,
+ VIR_XML_PROP_REQUIRED, ) < 0)
+return -1;
+
+def->features[val] = VIR_TRISTATE_SWITCH_ON;
+def->msrs_features[VIR_DOMAIN_MSRS_UNKNOWN] = unknown;
+break;
+}
+
case VIR_DOMAIN_FEATURE_HYPERV:
if (virDomainFeaturesHyperVDefParse(def, nodes[i]) < 0)
return -1;
@@ -17694,19 +17705,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) {
-virDomainMsrsUnknown unknown;
-xmlNodePtr node = NULL;
-if ((node = virXPathNode("./features/msrs", ctxt)) == NULL)
-return -1;
-
-if (virXMLPropEnum(node, "unknown", virDomainMsrsUnknownTypeFromString,
- VIR_XML_PROP_REQUIRED, ) < 0)
-return -1;
-
-def->msrs_features[VIR_DOMAIN_MSRS_UNKNOWN] = unknown;
-}
-
if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, )) < 0)
return -1;
--
2.31.1
[libvirt PATCH 11/16] virDomainFeaturesXENDefParse: Remove tautological "if"
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 58 --
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 45c4b9cedf..02c06d5ab9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17436,47 +17436,45 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
{
def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
-if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+node = xmlFirstElementChild(node);
+while (node) {
int feature;
virTristateSwitch value;
-node = xmlFirstElementChild(node);
-while (node) {
-feature = virDomainXenTypeFromString((const char *)node->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Xen feature: %s"),
- node->name);
-return -1;
-}
-
-if (virXMLPropTristateSwitch(node, "state",
- VIR_XML_PROP_REQUIRED, ) < 0)
-return -1;
+feature = virDomainXenTypeFromString((const char *)node->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Xen feature: %s"),
+ node->name);
+return -1;
+}
-def->xen_features[feature] = value;
+if (virXMLPropTristateSwitch(node, "state",
+ VIR_XML_PROP_REQUIRED, ) < 0)
+return -1;
-switch ((virDomainXen) feature) {
-case VIR_DOMAIN_XEN_E820_HOST:
-break;
+def->xen_features[feature] = value;
-case VIR_DOMAIN_XEN_PASSTHROUGH:
-if (value != VIR_TRISTATE_SWITCH_ON)
-break;
+switch ((virDomainXen) feature) {
+case VIR_DOMAIN_XEN_E820_HOST:
+break;
-if (virXMLPropEnum(node, "mode",
- virDomainXenPassthroughModeTypeFromString,
- VIR_XML_PROP_NONZERO,
- >xen_passthrough_mode) < 0)
-return -1;
+case VIR_DOMAIN_XEN_PASSTHROUGH:
+if (value != VIR_TRISTATE_SWITCH_ON)
break;
-case VIR_DOMAIN_XEN_LAST:
-break;
-}
+if (virXMLPropEnum(node, "mode",
+ virDomainXenPassthroughModeTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ >xen_passthrough_mode) < 0)
+return -1;
+break;
-node = xmlNextElementSibling(node);
+case VIR_DOMAIN_XEN_LAST:
+break;
}
+
+node = xmlNextElementSibling(node);
}
return 0;
--
2.31.1
[libvirt PATCH 10/16] virDomainFeaturesXENDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e687f18afe..45c4b9cedf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17432,31 +17432,25 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
static int
virDomainFeaturesXENDefParse(virDomainDef *def,
- xmlXPathContext *ctxt)
+ xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
-
def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/xen/*", ctxt, )) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
+node = xmlFirstElementChild(node);
+while (node) {
+feature = virDomainXenTypeFromString((const char *)node->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported Xen feature: %s"),
- nodes[i]->name);
+ node->name);
return -1;
}
-if (virXMLPropTristateSwitch(nodes[i], "state",
+if (virXMLPropTristateSwitch(node, "state",
VIR_XML_PROP_REQUIRED, ) < 0)
return -1;
@@ -17470,7 +17464,7 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-if (virXMLPropEnum(nodes[i], "mode",
+if (virXMLPropEnum(node, "mode",
virDomainXenPassthroughModeTypeFromString,
VIR_XML_PROP_NONZERO,
>xen_passthrough_mode) < 0)
@@ -17480,8 +17474,9 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
case VIR_DOMAIN_XEN_LAST:
break;
}
+
+node = xmlNextElementSibling(node);
}
-VIR_FREE(nodes);
}
return 0;
@@ -17540,7 +17535,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_XEN:
-if (virDomainFeaturesXENDefParse(def, ctxt) < 0)
+if (virDomainFeaturesXENDefParse(def, nodes[i]) < 0)
return -1;
break;
--
2.31.1
[libvirt PATCH 03/16] virDomainFeaturesHyperVDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4ec5557eba..b778dfe463 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17288,32 +17288,27 @@ virDomainResourceDefParse(xmlNodePtr node,
static int
virDomainFeaturesHyperVDefParse(virDomainDef *def,
-xmlXPathContext *ctxt)
+xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
-
def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON;
if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, )) < 0)
-return -1;
-for (i = 0; i < n; i++) {
+node = xmlFirstElementChild(node);
+while (node != NULL) {
xmlNodePtr child;
-feature = virDomainHypervTypeFromString((const char
*)nodes[i]->name);
+feature = virDomainHypervTypeFromString((const char *)node->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported HyperV Enlightenment feature:
%s"),
- nodes[i]->name);
+ node->name);
return -1;
}
-if (virXMLPropTristateSwitch(nodes[i], "state",
+if (virXMLPropTristateSwitch(node, "state",
VIR_XML_PROP_REQUIRED, ) < 0)
return -1;
@@ -17337,7 +17332,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-child = xmlFirstElementChild(nodes[i]);
+child = xmlFirstElementChild(node);
while (child) {
if (STRNEQ((const char *)child->name, "direct")) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -17359,7 +17354,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-if (virXMLPropUInt(nodes[i], "retries", 0,
+if (virXMLPropUInt(node, "retries", 0,
VIR_XML_PROP_REQUIRED,
>hyperv_spinlocks) < 0)
return -1;
@@ -17376,7 +17371,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-if (!(def->hyperv_vendor_id = virXMLPropString(nodes[i],
+if (!(def->hyperv_vendor_id = virXMLPropString(node,
"value"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing 'value' attribute for "
@@ -17403,6 +17398,8 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
case VIR_DOMAIN_HYPERV_LAST:
break;
}
+
+node = xmlNextElementSibling(node);
}
}
@@ -17454,7 +17451,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_HYPERV:
-if (virDomainFeaturesHyperVDefParse(def, ctxt) < 0)
+if (virDomainFeaturesHyperVDefParse(def, nodes[i]) < 0)
return -1;
break;
--
2.31.1
[libvirt PATCH 02/16] virDomainFeaturesHyperVDefParse: Inline hyperv/stimer parsing
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be inlined and
simplified. This also removes the re-use of `nodes`, elimininating
two VIR_FREEs.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 46 ++
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8cf57db7ba..4ec5557eba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17303,6 +17303,8 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
return -1;
for (i = 0; i < n; i++) {
+xmlNodePtr child;
+
feature = virDomainHypervTypeFromString((const char
*)nodes[i]->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -17323,7 +17325,6 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
case VIR_DOMAIN_HYPERV_VPINDEX:
case VIR_DOMAIN_HYPERV_RUNTIME:
case VIR_DOMAIN_HYPERV_SYNIC:
-case VIR_DOMAIN_HYPERV_STIMER:
case VIR_DOMAIN_HYPERV_RESET:
case VIR_DOMAIN_HYPERV_FREQUENCIES:
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
@@ -17332,6 +17333,28 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
case VIR_DOMAIN_HYPERV_EVMCS:
break;
+case VIR_DOMAIN_HYPERV_STIMER:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+child = xmlFirstElementChild(nodes[i]);
+while (child) {
+if (STRNEQ((const char *)child->name, "direct")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Hyper-V stimer feature:
%s"),
+ child->name);
+return -1;
+}
+
+if (virXMLPropTristateSwitch(child, "state",
+ VIR_XML_PROP_REQUIRED,
+ >hyperv_stimer_direct) <
0)
+return -1;
+
+child = xmlNextElementSibling(child);
+}
+break;
+
case VIR_DOMAIN_HYPERV_SPINLOCKS:
if (value != VIR_TRISTATE_SWITCH_ON)
break;
@@ -17381,27 +17404,6 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
break;
}
}
-VIR_FREE(nodes);
-}
-
-if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] ==
VIR_TRISTATE_SWITCH_ON) {
-if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, ))
< 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-if (STRNEQ((const char *)nodes[i]->name, "direct")) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Hyper-V stimer feature: %s"),
- nodes[i]->name);
-return -1;
-}
-
-if (virXMLPropTristateSwitch(nodes[i], "state",
- VIR_XML_PROP_REQUIRED,
- >hyperv_stimer_direct) < 0)
-return -1;
-}
-VIR_FREE(nodes);
}
return 0;
--
2.31.1
[libvirt PATCH 14/16] virDomainFeaturesDefParse: Factor out capabilities parsing into separate function
Cleanup to follow. This removes the last re-use of `nodes` in this function,
eliminating two VIR_FREEs.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 78 +-
1 file changed, 46 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 915303adcd..84c684b004 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17481,6 +17481,51 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
}
+static int
+virDomainFeaturesCapabilitiesDefParse(virDomainDef *def,
+ xmlNodePtr node,
+ xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+virDomainCapabilitiesPolicy policy;
+
+if (virXMLPropEnumDefault(node, "policy",
+ virDomainCapabilitiesPolicyTypeFromString,
+ VIR_XML_PROP_NONE, ,
+ VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT) < 0)
+return -1;
+
+def->features[VIR_DOMAIN_FEATURE_CAPABILITIES] = policy;
+
+if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, )) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+virTristateSwitch state;
+int val = virDomainProcessCapsFeatureTypeFromString((const char
*)nodes[i]->name);
+if (val < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unexpected capability feature '%s'"),
nodes[i]->name);
+return -1;
+}
+
+
+if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE,
+ ) < 0)
+return -1;
+
+if (state == VIR_TRISTATE_SWITCH_ABSENT)
+state = VIR_TRISTATE_SWITCH_ON;
+
+def->caps_features[val] = state;
+}
+
+return 0;
+}
+
+
static int
virDomainFeaturesDefParse(virDomainDef *def,
xmlXPathContextPtr ctxt)
@@ -17549,15 +17594,8 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
-virDomainCapabilitiesPolicy policy;
-
-if (virXMLPropEnumDefault(nodes[i], "policy",
-
virDomainCapabilitiesPolicyTypeFromString,
- VIR_XML_PROP_NONE, ,
- VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT)
< 0)
+if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i], ctxt) < 0)
return -1;
-
-def->features[val] = policy;
break;
}
@@ -17703,31 +17741,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
}
}
-VIR_FREE(nodes);
-if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, )) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-virTristateSwitch state;
-int val = virDomainProcessCapsFeatureTypeFromString((const char
*)nodes[i]->name);
-if (val < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unexpected capability feature '%s'"),
nodes[i]->name);
-return -1;
-}
-
-
-if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE,
- ) < 0)
-return -1;
-
-if (state == VIR_TRISTATE_SWITCH_ABSENT)
-state = VIR_TRISTATE_SWITCH_ON;
-
-def->caps_features[val] = state;
-}
-VIR_FREE(nodes);
return 0;
}
--
2.31.1
[libvirt PATCH 07/16] virDomainFeaturesKVMDefParse: Remove tautological "switch"
`feature` is always one of the values listed in the switch,
ensured by `virDomainKVMTypeFromString` above.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 18 --
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2ad4cbc5a3..62565601ab 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17418,21 +17418,11 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
return -1;
}
-switch ((virDomainKVM) feature) {
-case VIR_DOMAIN_KVM_HIDDEN:
-case VIR_DOMAIN_KVM_DEDICATED:
-case VIR_DOMAIN_KVM_POLLCONTROL:
-if (virXMLPropTristateSwitch(node, "state",
- VIR_XML_PROP_REQUIRED,
- ) < 0)
-return -1;
-
-def->kvm_features[feature] = value;
-break;
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
+ ) < 0)
+return -1;
-case VIR_DOMAIN_KVM_LAST:
-break;
-}
+def->kvm_features[feature] = value;
node = xmlNextElementSibling(node);
}
--
2.31.1
[libvirt PATCH 15/16] virDomainFeaturesCapabilitiesDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 84c684b004..50717c4f44 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17483,12 +17483,8 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
static int
virDomainFeaturesCapabilitiesDefParse(virDomainDef *def,
- xmlNodePtr node,
- xmlXPathContext *ctxt)
+ xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
virDomainCapabilitiesPolicy policy;
if (virXMLPropEnumDefault(node, "policy",
@@ -17499,27 +17495,25 @@ virDomainFeaturesCapabilitiesDefParse(virDomainDef
*def,
def->features[VIR_DOMAIN_FEATURE_CAPABILITIES] = policy;
-if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, )) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
+node = xmlFirstElementChild(node);
+while (node) {
virTristateSwitch state;
-int val = virDomainProcessCapsFeatureTypeFromString((const char
*)nodes[i]->name);
+int val = virDomainProcessCapsFeatureTypeFromString((const char
*)node->name);
if (val < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unexpected capability feature '%s'"),
nodes[i]->name);
+ _("unexpected capability feature '%s'"),
node->name);
return -1;
}
-if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE,
- ) < 0)
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_NONE, )
< 0)
return -1;
if (state == VIR_TRISTATE_SWITCH_ABSENT)
state = VIR_TRISTATE_SWITCH_ON;
def->caps_features[val] = state;
+node = xmlNextElementSibling(node);
}
return 0;
@@ -17594,7 +17588,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
-if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i], ctxt) < 0)
+if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i]) < 0)
return -1;
break;
}
--
2.31.1
[libvirt PATCH 05/16] virDomainFeaturesDefParse: Factor out KVM parsing into separate function
Only moving code, cleanup to follow.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 88 +-
1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ba41869ec..384740c364 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17398,6 +17398,54 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
}
+static int
+virDomainFeaturesKVMDefParse(virDomainDef *def,
+ xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+
+def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
+
+if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
+int feature;
+virTristateSwitch value;
+if ((n = virXPathNodeSet("./features/kvm/*", ctxt, )) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+feature = virDomainKVMTypeFromString((const char *)nodes[i]->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported KVM feature: %s"),
+ nodes[i]->name);
+return -1;
+}
+
+switch ((virDomainKVM) feature) {
+case VIR_DOMAIN_KVM_HIDDEN:
+case VIR_DOMAIN_KVM_DEDICATED:
+case VIR_DOMAIN_KVM_POLLCONTROL:
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_REQUIRED,
+ ) < 0)
+return -1;
+
+def->kvm_features[feature] = value;
+break;
+
+case VIR_DOMAIN_KVM_LAST:
+break;
+}
+}
+VIR_FREE(nodes);
+}
+
+return 0;
+}
+
+
static int
virDomainFeaturesDefParse(virDomainDef *def,
xmlXPathContextPtr ctxt)
@@ -17435,7 +17483,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_PAE:
case VIR_DOMAIN_FEATURE_VIRIDIAN:
case VIR_DOMAIN_FEATURE_PRIVNET:
-case VIR_DOMAIN_FEATURE_KVM:
case VIR_DOMAIN_FEATURE_MSRS:
case VIR_DOMAIN_FEATURE_XEN:
def->features[val] = VIR_TRISTATE_SWITCH_ON;
@@ -17446,6 +17493,11 @@ virDomainFeaturesDefParse(virDomainDef *def,
return -1;
break;
+case VIR_DOMAIN_FEATURE_KVM:
+if (virDomainFeaturesKVMDefParse(def, ctxt) < 0)
+return -1;
+break;
+
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
virDomainCapabilitiesPolicy policy;
@@ -17579,40 +17631,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
-int feature;
-virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/kvm/*", ctxt, )) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-feature = virDomainKVMTypeFromString((const char *)nodes[i]->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported KVM feature: %s"),
- nodes[i]->name);
-return -1;
-}
-
-switch ((virDomainKVM) feature) {
-case VIR_DOMAIN_KVM_HIDDEN:
-case VIR_DOMAIN_KVM_DEDICATED:
-case VIR_DOMAIN_KVM_POLLCONTROL:
-if (virXMLPropTristateSwitch(nodes[i], "state",
- VIR_XML_PROP_REQUIRED,
- ) < 0)
-return -1;
-
-def->kvm_features[feature] = value;
-break;
-
-case VIR_DOMAIN_KVM_LAST:
-break;
-}
-}
-VIR_FREE(nodes);
-}
-
if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
--
2.31.1
[libvirt PATCH 12/16] virDomainFeaturesDefParse: Inline SMM parsing
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02c06d5ab9..b411c1fb8c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17554,8 +17554,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_HAP:
case VIR_DOMAIN_FEATURE_PMU:
case VIR_DOMAIN_FEATURE_PVSPINLOCK:
-case VIR_DOMAIN_FEATURE_VMPORT:
-case VIR_DOMAIN_FEATURE_SMM: {
+case VIR_DOMAIN_FEATURE_VMPORT: {
virTristateSwitch state;
if (virXMLPropTristateSwitch(nodes[i], "state",
@@ -17569,6 +17568,31 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
}
+case VIR_DOMAIN_FEATURE_SMM: {
+virTristateSwitch state;
+
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_NONE, ) < 0)
+return -1;
+
+if ((state == VIR_TRISTATE_SWITCH_ABSENT) ||
+(state == VIR_TRISTATE_SWITCH_ON)) {
+int rv = virParseScaledValue("string(./features/smm/tseg)",
+
"string(./features/smm/tseg/@unit)",
+ ctxt,
+ >tseg_size,
+ 1024 * 1024, /* Defaults to
mebibytes */
+ ULLONG_MAX,
+ false);
+if (rv < 0)
+return -1;
+
+def->features[val] = VIR_TRISTATE_SWITCH_ON;
+def->tseg_specified = rv != 0;
+}
+break;
+}
+
case VIR_DOMAIN_FEATURE_GIC:
if (virXMLPropEnum(nodes[i], "version",
virGICVersionTypeFromString,
VIR_XML_PROP_NONZERO, >gic_version) < 0)
@@ -17670,19 +17694,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) {
-int rv = virParseScaledValue("string(./features/smm/tseg)",
- "string(./features/smm/tseg/@unit)",
- ctxt,
- >tseg_size,
- 1024 * 1024, /* Defaults to mebibytes */
- ULLONG_MAX,
- false);
-if (rv < 0)
-return -1;
-def->tseg_specified = rv;
-}
-
if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) {
virDomainMsrsUnknown unknown;
xmlNodePtr node = NULL;
--
2.31.1
[libvirt PATCH 06/16] virDomainFeaturesKVMDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 384740c364..2ad4cbc5a3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17400,26 +17400,21 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
static int
virDomainFeaturesKVMDefParse(virDomainDef *def,
- xmlXPathContext *ctxt)
+ xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
-
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/kvm/*", ctxt, )) < 0)
-return -1;
-for (i = 0; i < n; i++) {
-feature = virDomainKVMTypeFromString((const char *)nodes[i]->name);
+node = xmlFirstElementChild(node);
+while (node) {
+feature = virDomainKVMTypeFromString((const char *)node->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported KVM feature: %s"),
- nodes[i]->name);
+ node->name);
return -1;
}
@@ -17427,7 +17422,7 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
case VIR_DOMAIN_KVM_HIDDEN:
case VIR_DOMAIN_KVM_DEDICATED:
case VIR_DOMAIN_KVM_POLLCONTROL:
-if (virXMLPropTristateSwitch(nodes[i], "state",
+if (virXMLPropTristateSwitch(node, "state",
VIR_XML_PROP_REQUIRED,
) < 0)
return -1;
@@ -17438,8 +17433,9 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
case VIR_DOMAIN_KVM_LAST:
break;
}
+
+node = xmlNextElementSibling(node);
}
-VIR_FREE(nodes);
}
return 0;
@@ -17494,7 +17490,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_KVM:
-if (virDomainFeaturesKVMDefParse(def, ctxt) < 0)
+if (virDomainFeaturesKVMDefParse(def, nodes[i]) < 0)
return -1;
break;
--
2.31.1
[libvirt PATCH 01/16] virDomainFeaturesDefParse: Factor out HyperV parsing into separate function
Only moving code, cleanup to follow.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 236 ++---
1 file changed, 127 insertions(+), 109 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f65509d8ec..8cf57db7ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17286,6 +17286,128 @@ virDomainResourceDefParse(xmlNodePtr node,
}
+static int
+virDomainFeaturesHyperVDefParse(virDomainDef *def,
+xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+
+def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON;
+
+if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
+int feature;
+virTristateSwitch value;
+if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, )) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+feature = virDomainHypervTypeFromString((const char
*)nodes[i]->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported HyperV Enlightenment feature:
%s"),
+ nodes[i]->name);
+return -1;
+}
+
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_REQUIRED, ) < 0)
+return -1;
+
+def->hyperv_features[feature] = value;
+
+switch ((virDomainHyperv) feature) {
+case VIR_DOMAIN_HYPERV_RELAXED:
+case VIR_DOMAIN_HYPERV_VAPIC:
+case VIR_DOMAIN_HYPERV_VPINDEX:
+case VIR_DOMAIN_HYPERV_RUNTIME:
+case VIR_DOMAIN_HYPERV_SYNIC:
+case VIR_DOMAIN_HYPERV_STIMER:
+case VIR_DOMAIN_HYPERV_RESET:
+case VIR_DOMAIN_HYPERV_FREQUENCIES:
+case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
+case VIR_DOMAIN_HYPERV_TLBFLUSH:
+case VIR_DOMAIN_HYPERV_IPI:
+case VIR_DOMAIN_HYPERV_EVMCS:
+break;
+
+case VIR_DOMAIN_HYPERV_SPINLOCKS:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+if (virXMLPropUInt(nodes[i], "retries", 0,
+ VIR_XML_PROP_REQUIRED,
+ >hyperv_spinlocks) < 0)
+return -1;
+
+if (def->hyperv_spinlocks < 0xFFF) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("HyperV spinlock retry count must be "
+ "at least 4095"));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_HYPERV_VENDOR_ID:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+if (!(def->hyperv_vendor_id = virXMLPropString(nodes[i],
+ "value"))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing 'value' attribute for "
+ "HyperV feature 'vendor_id'"));
+return -1;
+}
+
+if (strlen(def->hyperv_vendor_id) >
VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("HyperV vendor_id value must not be more "
+ "than %d characters."),
+ VIR_DOMAIN_HYPERV_VENDOR_ID_MAX);
+return -1;
+}
+
+/* ensure that the string can be passed to qemu */
+if (strchr(def->hyperv_vendor_id, ',')) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("HyperV vendor_id value is invalid"));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_HYPERV_LAST:
+break;
+}
+}
+VIR_FREE(nodes);
+}
+
+if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] ==
VIR_TRISTATE_SWITCH_ON) {
+if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, ))
< 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+if (STRNEQ((const char *)nodes[i]->name, "direct")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Hyper-V stimer feature: %s"),
+ nodes[i]->name);
+return -1;
+}
+
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_REQUIRED,
+ >hyperv_stimer_direct) < 0)
+return -1;
+}
[libvirt PATCH 09/16] virDomainFeaturesDefParse: Factor out XEN parsing into separate function
Only moving code, cleanup to follow.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 108 -
1 file changed, 63 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24529f3093..e687f18afe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17430,6 +17430,64 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
}
+static int
+virDomainFeaturesXENDefParse(virDomainDef *def,
+ xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+
+def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
+
+if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+int feature;
+virTristateSwitch value;
+
+if ((n = virXPathNodeSet("./features/xen/*", ctxt, )) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Xen feature: %s"),
+ nodes[i]->name);
+return -1;
+}
+
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_REQUIRED, ) < 0)
+return -1;
+
+def->xen_features[feature] = value;
+
+switch ((virDomainXen) feature) {
+case VIR_DOMAIN_XEN_E820_HOST:
+break;
+
+case VIR_DOMAIN_XEN_PASSTHROUGH:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+if (virXMLPropEnum(nodes[i], "mode",
+ virDomainXenPassthroughModeTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ >xen_passthrough_mode) < 0)
+return -1;
+break;
+
+case VIR_DOMAIN_XEN_LAST:
+break;
+}
+}
+VIR_FREE(nodes);
+}
+
+return 0;
+}
+
+
static int
virDomainFeaturesDefParse(virDomainDef *def,
xmlXPathContextPtr ctxt)
@@ -17468,7 +17526,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_VIRIDIAN:
case VIR_DOMAIN_FEATURE_PRIVNET:
case VIR_DOMAIN_FEATURE_MSRS:
-case VIR_DOMAIN_FEATURE_XEN:
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
@@ -17482,6 +17539,11 @@ virDomainFeaturesDefParse(virDomainDef *def,
return -1;
break;
+case VIR_DOMAIN_FEATURE_XEN:
+if (virDomainFeaturesXENDefParse(def, ctxt) < 0)
+return -1;
+break;
+
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
virDomainCapabilitiesPolicy policy;
@@ -17615,50 +17677,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
-int feature;
-virTristateSwitch value;
-
-if ((n = virXPathNodeSet("./features/xen/*", ctxt, )) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Xen feature: %s"),
- nodes[i]->name);
-return -1;
-}
-
-if (virXMLPropTristateSwitch(nodes[i], "state",
- VIR_XML_PROP_REQUIRED, ) < 0)
-return -1;
-
-def->xen_features[feature] = value;
-
-switch ((virDomainXen) feature) {
-case VIR_DOMAIN_XEN_E820_HOST:
-break;
-
-case VIR_DOMAIN_XEN_PASSTHROUGH:
-if (value != VIR_TRISTATE_SWITCH_ON)
-break;
-
-if (virXMLPropEnum(nodes[i], "mode",
- virDomainXenPassthroughModeTypeFromString,
- VIR_XML_PROP_NONZERO,
- >xen_passthrough_mode) < 0)
-return -1;
-break;
-
-case VIR_DOMAIN_XEN_LAST:
-break;
-}
-}
-VIR_FREE(nodes);
-}
-
if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) {
int rv = virParseScaledValue("string(./features/smm/tseg)",
"string(./features/smm/tseg/@unit)",
--
2.31.1
[libvirt PATCH 00/16] Refactor virDomainFeaturesDefParse
Some refactoring in preparation for adding support for qemu's "hv-passthrough" and the yet-to-be-merged "hv-defaults". Tim Wiederhake (16): virDomainFeaturesDefParse: Factor out HyperV parsing into separate function virDomainFeaturesHyperVDefParse: Inline hyperv/stimer parsing virDomainFeaturesHyperVDefParse: Remove ctxt virDomainFeaturesHyperVDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out KVM parsing into separate function virDomainFeaturesKVMDefParse: Remove ctxt virDomainFeaturesKVMDefParse: Remove tautological "switch" virDomainFeaturesKVMDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out XEN parsing into separate function virDomainFeaturesXENDefParse: Remove ctxt virDomainFeaturesXENDefParse: Remove tautological "if" virDomainFeaturesDefParse: Inline SMM parsing virDomainFeaturesDefParse: Inline MSRS parsing virDomainFeaturesDefParse: Factor out capabilities parsing into separate function virDomainFeaturesCapabilitiesDefParse: Remove ctxt virDomainFeaturesDefParse: Simplify APIC parsing src/conf/domain_conf.c | 557 ++--- 1 file changed, 296 insertions(+), 261 deletions(-) -- 2.31.1
[libvirt PATCH 08/16] virDomainFeaturesKVMDefParse: Remove tautological "if"
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 62565601ab..24529f3093 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17404,28 +17404,26 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
{
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
-if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
+node = xmlFirstElementChild(node);
+while (node) {
int feature;
virTristateSwitch value;
-node = xmlFirstElementChild(node);
-while (node) {
-feature = virDomainKVMTypeFromString((const char *)node->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported KVM feature: %s"),
- node->name);
-return -1;
-}
+feature = virDomainKVMTypeFromString((const char *)node->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported KVM feature: %s"),
+ node->name);
+return -1;
+}
-if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
- ) < 0)
-return -1;
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
+ ) < 0)
+return -1;
-def->kvm_features[feature] = value;
+def->kvm_features[feature] = value;
-node = xmlNextElementSibling(node);
-}
+node = xmlNextElementSibling(node);
}
return 0;
--
2.31.1
[libvirt PATCH 04/16] virDomainFeaturesHyperVDefParse: Remove tautological "if"
Fix some line wrapping in the process.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 163 +++--
1 file changed, 77 insertions(+), 86 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b778dfe463..3ba41869ec 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17292,115 +17292,106 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
{
def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON;
-if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
+node = xmlFirstElementChild(node);
+while (node != NULL) {
int feature;
virTristateSwitch value;
+xmlNodePtr child;
-node = xmlFirstElementChild(node);
-while (node != NULL) {
-xmlNodePtr child;
+feature = virDomainHypervTypeFromString((const char *)node->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported HyperV Enlightenment feature: %s"),
+ node->name);
+return -1;
+}
-feature = virDomainHypervTypeFromString((const char *)node->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported HyperV Enlightenment feature:
%s"),
- node->name);
-return -1;
-}
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
+ ) < 0)
+return -1;
-if (virXMLPropTristateSwitch(node, "state",
- VIR_XML_PROP_REQUIRED, ) < 0)
-return -1;
+def->hyperv_features[feature] = value;
-def->hyperv_features[feature] = value;
+switch ((virDomainHyperv) feature) {
+case VIR_DOMAIN_HYPERV_RELAXED:
+case VIR_DOMAIN_HYPERV_VAPIC:
+case VIR_DOMAIN_HYPERV_VPINDEX:
+case VIR_DOMAIN_HYPERV_RUNTIME:
+case VIR_DOMAIN_HYPERV_SYNIC:
+case VIR_DOMAIN_HYPERV_RESET:
+case VIR_DOMAIN_HYPERV_FREQUENCIES:
+case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
+case VIR_DOMAIN_HYPERV_TLBFLUSH:
+case VIR_DOMAIN_HYPERV_IPI:
+case VIR_DOMAIN_HYPERV_EVMCS:
+break;
-switch ((virDomainHyperv) feature) {
-case VIR_DOMAIN_HYPERV_RELAXED:
-case VIR_DOMAIN_HYPERV_VAPIC:
-case VIR_DOMAIN_HYPERV_VPINDEX:
-case VIR_DOMAIN_HYPERV_RUNTIME:
-case VIR_DOMAIN_HYPERV_SYNIC:
-case VIR_DOMAIN_HYPERV_RESET:
-case VIR_DOMAIN_HYPERV_FREQUENCIES:
-case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
-case VIR_DOMAIN_HYPERV_TLBFLUSH:
-case VIR_DOMAIN_HYPERV_IPI:
-case VIR_DOMAIN_HYPERV_EVMCS:
+case VIR_DOMAIN_HYPERV_STIMER:
+if (value != VIR_TRISTATE_SWITCH_ON)
break;
-case VIR_DOMAIN_HYPERV_STIMER:
-if (value != VIR_TRISTATE_SWITCH_ON)
-break;
+child = xmlFirstElementChild(node);
+while (child) {
+if (STRNEQ((const char *)child->name, "direct")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Hyper-V stimer feature: %s"),
+ child->name);
+return -1;
+}
-child = xmlFirstElementChild(node);
-while (child) {
-if (STRNEQ((const char *)child->name, "direct")) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Hyper-V stimer feature:
%s"),
- child->name);
-return -1;
-}
+if (virXMLPropTristateSwitch(child, "state",
VIR_XML_PROP_REQUIRED,
+ >hyperv_stimer_direct) < 0)
+return -1;
-if (virXMLPropTristateSwitch(child, "state",
- VIR_XML_PROP_REQUIRED,
- >hyperv_stimer_direct) <
0)
-return -1;
+child = xmlNextElementSibling(child);
+}
+break;
-child = xmlNextElementSibling(child);
-}
+case VIR_DOMAIN_HYPERV_SPINLOCKS:
+if (value != VIR_TRISTATE_SWITCH_ON)
break;
-case VIR_DOMAIN_HYPERV_SPINLOCKS:
-if (value != VIR_TRISTATE_SWITCH_ON)
-break;
+if (virXMLPropUInt(node, "retries", 0, VIR_XML_PROP_REQUIRED,
+
[PATCH] tests: qemucapabilities: Bump test data for qemu-6.1 on x86_64
Update the caps data for the upcoming qemu version. Notable changes are: - 'query-sev-attestation-report' command added - 'sample-pages' members for dirty rate calculation added - 'qtest' device added - 'share' member added to query-memdev and 'reserve' members added to query-memdev/memory-backend-[file,memfd,ram] - 'qemu-vdagent' chardev added - 'mptcp' toggle added to inet servers - 'zstd' compression for qcow2 - new cpu models: - "Snowridge-v3" - "Skylake-Server-v5" - "Skylake-Client-v4" - "Icelake-Server-v5" - "Icelake-Client-v3" - "Dhyana-v2" - "Denverton-v3" - "Cooperlake-v2" - "Cascadelake-Server-v5" - 'avx-vnni' added to some existing cpu models - 'model-id' is now being reported as the host cpu again rather than QEMU TCG as I've noted in previous bump Signed-off-by: Peter Krempa --- .../caps_6.1.0.x86_64.replies | 3663 ++--- .../caps_6.1.0.x86_64.xml | 366 +- 2 files changed, 2525 insertions(+), 1504 deletions(-) diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.replies index 9291840bd2..2217e1331c 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.replies @@ -21,7 +21,7 @@ "minor": 0, "major": 6 }, -"package": "v6.0.0-540-g6005ee07c3" +"package": "v6.0.0-1820-g0add99ea3e" }, "id": "libvirt-2" } [...] Trimmed for brevity. Full version: https://gitlab.com/pipo.sk/libvirt/-/commit/9fd3bc550ff91d6340d1c0316b46fb05bb9e92b8 diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index 1937b88a4d..f173daf788 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -263,7 +263,7 @@ 650 0 43100243 - v6.0.0-540-g6005ee07c3 + v6.0.0-1820-g0add99ea3e x86_64 @@ -297,7 +297,7 @@ - + @@ -314,7 +314,7 @@ - + @@ -359,6 +359,7 @@ + @@ -427,7 +428,7 @@ - + @@ -442,7 +443,7 @@ - + @@ -516,7 +517,7 @@ - + @@ -568,7 +569,7 @@ - + @@ -636,6 +637,16 @@ + + + + + + + + + + @@ -672,6 +683,22 @@ + + + + + + + + + + + + + + + + @@ -790,6 +817,12 @@ + + + + + + @@ -916,6 +949,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1076,6 +1138,22 @@ + + + + + + + + + + + + + + + + @@ -1224,8 +1302,13 @@ + + + + + @@ -1244,6 +1327,29 @@ + + + + + + + + + + + + + + + + + + + + + + + @@ -1258,6 +1364,7 @@ + @@ -1280,6 +1387,7 @@ + @@ -1290,6 +1398,24 @@ + + + + + + + + + + + + + + + + + + @@ -1472,9 +1598,9 @@ + - @@ -1538,7 +1664,7 @@ - + @@ -1600,6 +1726,7 @@ + @@ -1668,7 +1795,7 @@ - + @@ -1683,7 +1810,7 @@ - + @@ -1859,6 +1986,25 @@ + + + + + + + + + + + + + + + + + + + @@ -1913,6 +2059,26 @@ + + + + + + + + + + + + + + + + + + + + @@ -2052,6 +2218,21 @@ + + + + + + + + + + + + + + + @@ -2282,6 +2463,48 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -2484,6 +2707,32 @@ + + +
RE: [RFC PATCH 0/7] LIBVIRT: X86: TDX support
> -Original Message- > From: Peter Krempa > Sent: Monday, June 21, 2021 1:06 AM > To: Duan, Zhenzhong > Cc: libvir-list@redhat.com; Yamahata, Isaku ; Tian, > Jun J ; Qiang, Chenyi > > Subject: Re: [RFC PATCH 0/7] LIBVIRT: X86: TDX support ... > > > > Using these patches we have succesfully booted and tested a guest both > > > > with and without TDX enabled. > > > > > > > > > > > > [1] https://lkml.org/lkml/2020/11/16/1106 > > > > [2] https://github.com/codomania/libvirt/commits/v9 > > > > > > Could you please also point to the relevant qemu patches? > > > > > > The first commit mentions 'query-tdx-capabilities' which is not in qemu > > > upstream yet. > > Hi Peter, > > > > Sorry, seems qemu patches link is missed in [1]. List all links below for > > your reference. > > > > kvm TDX branch: https://github.com/intel/tdx/tree/kvm > > TDX guest branch: https://github.com/intel/tdx/tree/guest > > TDVF branch: https://github.com/tianocore/edk2-staging/tree/TDVF > > qemu TDX branch: https://github.com/intel/qemu-tdx/tree/tdx > > In my quick search I didn't find any reference to those patches on > the qemu-devel mailing list. Please note that libvirt accepts only > features which are supported by the upstream releases [1] of the > hypervisor in question. > > Thus if the qemu part indeed wasn't yet posted for review to qemu-devel > you should do so if you want this series to be accepted in libvirt. > > [1] Pushed upstream waiting for the next release is okay. For qemu-devel, here is the reference. https://lore.kernel.org/qemu-devel/cover.1613188118.git.isaku.yamah...@intel.com/ I'm not sure why lists.nongnu.org search doesn't show result with TDX keyword. Thanks, Isaku Yamahata
[PATCH] qemu: Don't use memory-backend-memfd for NVDIMMs
If guest is configured to use memfd then the function that build
memory-backend-* part of command line will put
memory-backend-memfd, always. Even for NVDIMMs. This is not
correct, because NVDIMMs need a backing path (usually to a real
host NVDIMM device). Therefore, regardless of memfd being
requested, we have to stick with memory-backend-file.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_command.c | 3 ++-
.../memfd-memory-numa.x86_64-latest.args | 6 --
tests/qemuxml2argvdata/memfd-memory-numa.xml | 11 +++
tests/qemuxml2xmltest.c | 3 ++-
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ea513693f7..0473e7deaa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3052,7 +3052,8 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
props = virJSONValueNewObject();
-if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) {
+if (!mem->nvdimmPath &&
+def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) {
backendType = "memory-backend-memfd";
if (useHugepage) {
diff --git a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
index 5e54908666..3b33db3c55 100644
--- a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
@@ -10,13 +10,15 @@
XDG_CONFIG_HOME=/tmp/lib/domain--1-instance-0092/.config \
-name guest=instance-0092,debug-threads=on \
-S \
-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-instance-0092/master-key.aes"}'
\
--machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off \
+-machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off,nvdimm=on \
-cpu qemu64 \
--m 14336 \
+-m size=14680064k,slots=16,maxmem=1099511627776k \
-overcommit mem-lock=off \
-smp 8,sockets=1,dies=1,cores=8,threads=1 \
-object
'{"qom-type":"memory-backend-memfd","id":"ram-node0","hugetlb":true,"hugetlbsize":2097152,"share":true,"prealloc":true,"size":15032385536,"host-nodes":[3],"policy":"preferred"}'
\
-numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
+-object
'{"qom-type":"memory-backend-file","id":"memnvdimm0","mem-path":"/tmp/nvdimm","share":true,"prealloc":true,"size":536870912,"host-nodes":[3],"policy":"preferred"}'
\
+-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
-uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
-display none \
-no-user-config \
diff --git a/tests/qemuxml2argvdata/memfd-memory-numa.xml
b/tests/qemuxml2argvdata/memfd-memory-numa.xml
index 3f448790a6..d9e1a9f564 100644
--- a/tests/qemuxml2argvdata/memfd-memory-numa.xml
+++ b/tests/qemuxml2argvdata/memfd-memory-numa.xml
@@ -1,6 +1,7 @@
instance-0092
126f2720-6f8e-45ab-a886-ec9277079a67
+ 1099511627776
14680064
14680064
@@ -41,5 +42,15 @@
+
+
+/tmp/nvdimm
+
+
+523264
+0
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 40e027aaa4..c5005d41a0 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1305,7 +1305,8 @@ mymain(void)
DO_TEST("memfd-memory-numa",
QEMU_CAPS_OBJECT_MEMORY_MEMFD,
QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB,
-QEMU_CAPS_OBJECT_MEMORY_FILE);
+QEMU_CAPS_OBJECT_MEMORY_FILE,
+QEMU_CAPS_DEVICE_NVDIMM);
DO_TEST("memfd-memory-default-hugepage",
QEMU_CAPS_OBJECT_MEMORY_MEMFD,
QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB,
--
2.31.1
Re: [libvirt PATCH 6/7] nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
On 6/14/21 10:46 PM, Jonathon Jongsma wrote:
On Mon, Jun 14, 2021 at 12:27 PM Boris Fiuczynski wrote:
On 6/3/21 10:11 PM, Jonathon Jongsma wrote:
Implement these new API functions in the nodedev driver.
Signed-off-by: Jonathon Jongsma
---
src/node_device/node_device_driver.c | 50
src/node_device/node_device_driver.h | 6
src/node_device/node_device_udev.c | 21 +++-
3 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 9ebe609aa4..75391f18b8 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1804,3 +1804,53 @@ nodeDeviceGetAutostart(virNodeDevice *device,
virNodeDeviceObjEndAPI();
return ret;
}
+
+
+int
+nodeDeviceIsPersistent(virNodeDevice *device)
+{
+virNodeDeviceObj *obj = NULL;
+virNodeDeviceDef *def = NULL;
+int ret = -1;
+
+if (nodeDeviceInitWait() < 0)
+return -1;
+
+if (!(obj = nodeDeviceObjFindByName(device->name)))
+return -1;
+def = virNodeDeviceObjGetDef(obj);
+
+if (virNodeDeviceIsPersistentEnsureACL(device->conn, def) < 0)
+goto cleanup;
+
+ret = virNodeDeviceObjIsPersistent(obj);
+
+ cleanup:
+virNodeDeviceObjEndAPI();
+return ret;
+}
+
+
+int
+nodeDeviceIsActive(virNodeDevice *device)
+{
+virNodeDeviceObj *obj = NULL;
+virNodeDeviceDef *def = NULL;
+int ret = -1;
+
+if (nodeDeviceInitWait() < 0)
+return -1;
+
+if (!(obj = nodeDeviceObjFindByName(device->name)))
+return -1;
+def = virNodeDeviceObjGetDef(obj);
+
+if (virNodeDeviceIsActiveEnsureACL(device->conn, def) < 0)
+goto cleanup;
+
+ret = virNodeDeviceObjIsActive(obj);
+
+ cleanup:
+virNodeDeviceObjEndAPI();
+return ret;
+}
diff --git a/src/node_device/node_device_driver.h
b/src/node_device/node_device_driver.h
index d178a18180..744dd42632 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -180,6 +180,12 @@ int
nodeDeviceGetAutostart(virNodeDevice *dev,
int *autostart);
+int
+nodeDeviceIsPersistent(virNodeDevice *dev);
+
+int
+nodeDeviceIsActive(virNodeDevice *dev);
+
virCommand*
nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def,
bool autostart,
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 21273083a6..eb15ccce7f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1487,7 +1487,7 @@ udevAddOneDevice(struct udev_device *device)
virObjectEvent *event = NULL;
bool new_device = true;
int ret = -1;
-bool was_persistent = false;
+bool persistent = true;
bool autostart = true;
bool is_mdev;
@@ -1518,7 +1518,8 @@ udevAddOneDevice(struct udev_device *device)
if (is_mdev)
nodeDeviceDefCopyFromMdevctl(def, objdef);
-was_persistent = virNodeDeviceObjIsPersistent(obj);
+
+persistent = virNodeDeviceObjIsPersistent(obj);
autostart = virNodeDeviceObjIsAutostart(obj);
/* If the device was defined by mdevctl and was never instantiated,
it
@@ -1527,11 +1528,12 @@ udevAddOneDevice(struct udev_device *device)
virNodeDeviceObjEndAPI();
} else {
-/* All non-mdev devices report themselves as autostart since they
- * should still be present and active after a reboot unless the device
- * is removed from the host. Mediated devices can only be persistent if
- * they are in already in the device list from parsing the mdevctl
- * output. */
+/* All non-mdev devices report themselves as persistent and autostart
+ * since they should still be present and active after a reboot unless
+ * the device is removed from the host. Mediated devices can only be
+ * persistent if they are in already in the device list from parsing
+ * the mdevctl output. */
The assumption for all non-mdev devices ends up very misleading.
For example: The parent device of an mdev needs to be bound to a vfio
device driver. Without it the device ends up without the capability to
create mdevs.
If this driver binding is not persisted (e.g. with setting up driverctl)
but instead the device is just manually being rebound to a vfio device
driver than after reboot neither the mdev capability on the parent
device nor the mdev device as a child device will be existing.
A user calling nodedev-info before the reboot gets
on the parent device
Persistent: yes
Autostart: yes
and on the mdev device
Persistent: yes
Autostart: yes
After a reboot he ends up with with nodedev-info
on the parent device
Persistent: yes
Autostart: yes
and the mdev device does not exist.
Re: [RFC PATCH 0/7] LIBVIRT: X86: TDX support
On Mon, Jun 21, 2021 at 19:28:26 +, Yamahata, Isaku wrote: [...] > > > Sorry, seems qemu patches link is missed in [1]. List all links below for > > > your reference. > > > > > > kvm TDX branch: https://github.com/intel/tdx/tree/kvm > > > TDX guest branch: https://github.com/intel/tdx/tree/guest > > > TDVF branch: https://github.com/tianocore/edk2-staging/tree/TDVF > > > qemu TDX branch: https://github.com/intel/qemu-tdx/tree/tdx > > > > In my quick search I didn't find any reference to those patches on > > the qemu-devel mailing list. Please note that libvirt accepts only > > features which are supported by the upstream releases [1] of the > > hypervisor in question. > > > > Thus if the qemu part indeed wasn't yet posted for review to qemu-devel > > you should do so if you want this series to be accepted in libvirt. > > > > [1] Pushed upstream waiting for the next release is okay. > > For qemu-devel, here is the reference. > https://lore.kernel.org/qemu-devel/cover.1613188118.git.isaku.yamah...@intel.com/ > I'm not sure why lists.nongnu.org search doesn't show result with TDX keyword. The qemu patchset you've mentioned doesn't seem to correspond entirely to what this libvirt patchset is adding. I was looking for the 'query-tdx-capabilities' QMP command which is used in patch 1 of the libvirt series, but the patchset you've mentioned doesn't add it at all. As noted libvirt requires that the features exposed by libvirt are accepted by upstream qemu before adding support for them, to prevent maintenance of diverged or non-existing features.
