Re: [PATCH v2 0/9] qemu: tpm: Add support for migration across shared storage
On 10/6/22 09:26, Michal Prívozník wrote: On 10/5/22 16:01, Stefan Berger wrote: This series of patches adds support for migrating vTPMs across hosts whose storage has been set up to share the directory structure holding the state of the TPM (swtpm). A new migration flag VIR_MIGRATE_TPM_SHARED_STORAGE is added to enable this. This flag influences the management of the directory structure holding the TPM state, which for example is only removed when a domain is undefined and not when a VM is removed on the migration source host. Further, when shared storage is used then security labeling on the destination side is skipped assuming that the labeling was already done on the source side. I have tested this with an NFS setup where I had to turn SELinux off on the hosts since the SELinux MLS range labeling is not supported by NFS. Shared storage migration requires (upcoming) swtpm v0.8. Stefan Stefan Berger (9): util: Add parsing support for swtpm's cmdarg-migration capability qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration qemu: tpm: Conditionally create storage on incoming migration qemu: tpm: Pass --migration option to swtpm if supported qemu: tpm: Avoid security labels on incoming migration with shared storage qemu: tpm: Require UNDEFINE_TPM to be set to remove TPM state qemu: tpm: Determine whether to remove TPM state during migration qemu: tpm: Enable migration with VIR_MIGRATE_TPM_SHARED_STORAGE virsh: Add support for --tpm-shared-storage flag for migration docs/manpages/virsh.rst | 6 +++ include/libvirt/libvirt-domain.h | 8 +++ src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_extdevice.c| 5 +- src/qemu/qemu_extdevice.h| 3 +- src/qemu/qemu_migration.c| 23 +++-- src/qemu/qemu_migration.h| 1 + src/qemu/qemu_process.c | 10 ++-- src/qemu/qemu_process.h | 6 ++- src/qemu/qemu_saveimage.c| 2 +- src/qemu/qemu_snapshot.c | 4 +- src/qemu/qemu_tpm.c | 87 src/qemu/qemu_tpm.h | 24 - src/util/virtpm.c| 1 + src/util/virtpm.h| 1 + tools/virsh-domain.c | 7 +++ 17 files changed, 164 insertions(+), 29 deletions(-) Overall, I like this. I've raised couple of points in my review. I've made suggested changes as 'fixup' commits and pushed everything on my gitlab: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/tpm_migration_v2 (except for private data for TPM which I'm suggesting somewhere in review). Feel free to take them an squash them in. Or just parts of it. I mean, I wasn't sure where exactly I should stop passing 'flags' and set 'sharedStorage' bool argument. Maybe I was too aggressive and flags can be passed all the way down. I forgot about your series.. I put a v2 with fixes here now: https://github.com/stefanberger/libvirt-tpm/tree/swtpm_shared_storage.v2.fixed It has the private data support and a fix for post migration cleanup. The new flag is still in there... Stefan Michal
Re: [PATCH v2 6/9] qemu: tpm: Require UNDEFINE_TPM to be set to remove TPM state
On 10/6/22 09:26, Michal Prívozník wrote:
On 10/5/22 16:02, Stefan Berger wrote:
When migrating the TPM in a setup that has shared storage for the TPM state
files setup between hosts we never remove the state.
Signed-off-by: Stefan Berger
---
src/qemu/qemu_tpm.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 2b2d2eba5a..59de13061a 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -737,6 +737,10 @@ static void
qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm,
virDomainUndefineFlagsValues flags)
{
+/* Never remove the state in case of migration with shared storage. */
+if ((flags & VIR_MIGRATE_TPM_SHARED_STORAGE))
+return;
This is testing a flag from a different enum. If there's ever an
Uuuh, right.
If we had a function that we could always call to determine whether we migrate
across shared storage then any access to VIR_MIGRATE_TPM_SHARED_STORAGE could
be replaced with a call to this function.
undefine flag like:
VIR_DOMAIN_UNDEFINE_EXAMPLE = (1<<21)
then this is going to be wrongly evaluated. Can't callers just pass
VIR_DOMAIN_UNDEFINE_KEEP_TPM?
We have this here in this function.
/*
* remove TPM state if:
* - persistent_state flag is set and the UNDEFINE_TPM flag is set
* - persistent_state flag is not set and the KEEP_TPM flag is not set
*/
if ((tpm->data.emulator.persistent_state && (flags &
VIR_DOMAIN_UNDEFINE_TPM)) ||
(!tpm->data.emulator.persistent_state && !(flags &
VIR_DOMAIN_UNDEFINE_KEEP_TPM))) {
qemuTPMEmulatorDeleteStorage(tpm);
}
Alternatively, if we invent private data (see my comment to one of
previous patches), this can be plain:
if (QEMU_DOMAIN_TPM_PRIVATE(tpm)->migrating)
return;
Iirc you responded to me asking for being able to store info whether the
currently running version of swtpm is able to handle shared storage migration
(due to the additional flags for the release of the lock) and you suggested
that boolean could be stored there but this boolean is NOT an indicator for
whether shared storage is actually set up but whether it could handle shared
storage migration if it had to.
--> QEMU_DOMAIN_TPM_PRIVATE(tpm)->swtpm.shared_storage_migration
stefan
(or whatever member I suggested).
Michal
[PATCH] NEWS: Mention new channel and redirdev devices in domcaps
Signed-off-by: Jim Fehlig --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index b6bcb5524d..cd297c4fed 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -32,6 +32,11 @@ v8.9.0 (unreleased) * Improved documentation of CPU ``usable`` attribute in domain capabilities + * Report ``channel`` and ``redirdev`` devices in domain capabilities + +The channel and redirect devices supported by the hypervisor are now +reported in domain capabilities. + * **Bug fixes** * qemu: Disable all blocker features in CPU baseline -- 2.37.3
Re: [PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration
On 10/17/22 11:28, Michal Prívozník wrote: On 10/17/22 17:17, Stefan Berger wrote: On 10/17/22 09:48, Daniel P. Berrangé wrote: On Mon, Oct 17, 2022 at 09:39:52AM -0400, Stefan Berger wrote: The key is in qemuMigrationSrcIsSafe(), and how it determines if a migration is safe. * Disk on local storage, no flags => unsafe, migration error * Disk on local storage, VIR_MIGRATE_NON_SHARED_DISK => ok, copies disk storage * Disk on shared storage, no flags => safe * Disk on shared storage, VIR_MIGRATE_NON_SHARED_DISK => ok, but needlessly copies disk storage The key helper methods are virFileIsSharedFS and virFileIsClusterFS which check the filesystem type for the path against a known list of shared/cluster FS. So we won't do it this way for TPM state migration. Instead we can try to write on the source migration side a) a simple file and detect whether the file is at the destination b) a file with either a name or content that only the source and destination libvirts would know at this point b) is to prevent stale files from becoming indicators for shared storage. No, please use the virFileIsSharedFS/ClusterFS helpers. I tried to use virFileIsSharedFS on the source and destination side of my NFS setup to see how they work. The issue here is that the NFS server side, that exports /var/lib/libvirt/swtpm and is also the migration source at some point, says this: /var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is shared: 0 the NFS client side then says this: /var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is shared: 1VIR_MIGRATE_TPM_SHARED_STORAGE Are both sides mounted as NFS or just one of them? I think it's safe to It's just one of them that is the client, the other is the server exporting the file system. So it will be a matter of documentation on how to setup the shared filesystem. For NFS this means that both sides of the migration must be an NFS client and not one side acting as the server exporting the filesystem to the client. assume that either both sides are on a shared FS or none. Otherwise I'm quite sure file locking won't work anyways. We can go even further and I saw an error about the locks on cleanup (due to missing code in the series to avoid security label cleanup on TPM stop) but otherwise it's working fine. test whether shared_fs(source) == shared_fs(dst); and throw an error if it doesn't. You mean we would set the (now non-public? or not allowed to be set by user?) flag VIR_MIGRATE_TPM_SHARED_STORAGE on the sources side if we determine shared storage and again test on the destination side whether it thinks about shared storage (hopefully with the source host and not some other host -- presumably an excluded case) in the same way as the source host did? And since I've misguided you on the way with flag I am willing to help you write patches. Please let me know if you need any help. I am not sure at this point, probably you have a better feeling how to do this correctly than I do. If we could run virFileIsSharedFS on both sides and get the same result, this would be easy but I think it's not that simple. It looks like we need to determine the VIR_MIGRATE_TPM_SHARED_STORAGE on the source side and pass it to the destination side and possibly test again there... Stefan Michal
[PATCH 0/3] test: Fix regression parsing nested XML
This adds a couple extensions to the example/ testdriver XML, and fixes a regression introduced in b3e33a0ef7e Cole Robinson (3): examples: testdriver: Add xmlns runstate example examples: testdriver: Add a nested inline example test: Fix parsing nested XML examples/xml/test/testnodeinline.xml | 25 - src/test/test_driver.c | 2 +- 2 files changed, 25 insertions(+), 2 deletions(-) -- 2.37.3
[PATCH 3/3] test: Fix parsing nested XML
Reproducer:
./build/tools/virsh \
--connect test:///`pwd`/examples/xml/test/testnodeinline.xml \
vol-list default-pool
Fixes: b3e33a0ef7e62169175280c647aa9ac361bd554d
Signed-off-by: Cole Robinson
---
src/test/test_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 8675f8ad07..67c70de11d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1152,7 +1152,7 @@ testOpenVolumesForPool(const char *file,
g_autofree xmlNodePtr *nodes = NULL;
g_autoptr(virStorageVolDef) volDef = NULL;
-num = virXPathNodeSet("/pool/volume", ctxt, );
+num = virXPathNodeSet("./volume", ctxt, );
if (num < 0)
return -1;
--
2.37.3
[PATCH 2/3] examples: testdriver: Add a nested inline example
Signed-off-by: Cole Robinson --- examples/xml/test/testnodeinline.xml | 22 ++ 1 file changed, 22 insertions(+) diff --git a/examples/xml/test/testnodeinline.xml b/examples/xml/test/testnodeinline.xml index 90805f025a..4657ecadd2 100644 --- a/examples/xml/test/testnodeinline.xml +++ b/examples/xml/test/testnodeinline.xml @@ -177,6 +177,28 @@ + + default-pool + 35bb2ad9-388a-cdfe-461a-b8907f6e53fe + 107374182400 + 0 + 107374182400 + +/default-pool + + 0700 + 10736 + 10736 + + + + +default-vol +100 +5 + + + 6000 -- 2.37.3
[PATCH 1/3] examples: testdriver: Add xmlns runstate example
The testdriver has xmlns support for overriding object default state. demo it by pausing a VM Signed-off-by: Cole Robinson --- examples/xml/test/testnodeinline.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/xml/test/testnodeinline.xml b/examples/xml/test/testnodeinline.xml index 9165d9302d..90805f025a 100644 --- a/examples/xml/test/testnodeinline.xml +++ b/examples/xml/test/testnodeinline.xml @@ -86,7 +86,8 @@ - + +3 fc5 08721f993d1d4aec96eb97803297bb36 -- 2.37.3
Re: [PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration
On 10/17/22 17:17, Stefan Berger wrote: > > > On 10/17/22 09:48, Daniel P. Berrangé wrote: >> On Mon, Oct 17, 2022 at 09:39:52AM -0400, Stefan Berger wrote: >>> >>> > >> >> The key is in qemuMigrationSrcIsSafe(), and how it determines if a >> migration is safe. >> >> * Disk on local storage, no flags => unsafe, migration error >> * Disk on local storage, VIR_MIGRATE_NON_SHARED_DISK => ok, copies >> disk storage >> * Disk on shared storage, no flags => safe >> * Disk on shared storage, VIR_MIGRATE_NON_SHARED_DISK => ok, but >> needlessly copies disk storage >> >> The key helper methods are virFileIsSharedFS and virFileIsClusterFS >> which check the filesystem type for the path against a known list >> of shared/cluster FS. >> >>> So we won't do it this way for TPM state migration. Instead we can >>> try to write on the source migration side >>> >>> a) a simple file and detect whether the file is at the destination >>> b) a file with either a name or content that only the source and >>> destination libvirts would know at this point >>> >>> b) is to prevent stale files from becoming indicators for shared >>> storage. >> >> No, please use the virFileIsSharedFS/ClusterFS helpers. >> > > I tried to use virFileIsSharedFS on the source and destination side of > my NFS setup to see how they work. The issue here is that the NFS server > side, that exports /var/lib/libvirt/swtpm and is also the migration > source at some point, says this: > > /var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is > shared: 0 > > the NFS client side then says this: > > > /var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is > shared: 1 > Are both sides mounted as NFS or just one of them? I think it's safe to assume that either both sides are on a shared FS or none. Otherwise I'm quite sure file locking won't work anyways. We can go even further and test whether shared_fs(source) == shared_fs(dst); and throw an error if it doesn't. And since I've misguided you on the way with flag I am willing to help you write patches. Please let me know if you need any help. Michal
Re: [PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration
On 10/17/22 09:48, Daniel P. Berrangé wrote:
On Mon, Oct 17, 2022 at 09:39:52AM -0400, Stefan Berger wrote:
The key is in qemuMigrationSrcIsSafe(), and how it determines if a
migration is safe.
* Disk on local storage, no flags => unsafe, migration error
* Disk on local storage, VIR_MIGRATE_NON_SHARED_DISK => ok, copies disk
storage
* Disk on shared storage, no flags => safe
* Disk on shared storage, VIR_MIGRATE_NON_SHARED_DISK => ok, but needlessly
copies disk storage
The key helper methods are virFileIsSharedFS and virFileIsClusterFS
which check the filesystem type for the path against a known list
of shared/cluster FS.
So we won't do it this way for TPM state migration. Instead we can
try to write on the source migration side
a) a simple file and detect whether the file is at the destination
b) a file with either a name or content that only the source and
destination libvirts would know at this point
b) is to prevent stale files from becoming indicators for shared storage.
No, please use the virFileIsSharedFS/ClusterFS helpers.
I tried to use virFileIsSharedFS on the source and destination side of my NFS
setup to see how they work. The issue here is that the NFS server side, that
exports /var/lib/libvirt/swtpm and is also the migration source at some point,
says this:
/var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is shared: 0
the NFS client side then says this:
/var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is shared: 1
The latter is correct, the former obviously not. Is this an illegal NFS setup
or a shortcoming of the AP
I suppose both sides should be able to run the API (and come up with the same
result) and not the one side setting an additional migration flag when shared
storage is found and that flag then appearing on the migration destination
side, which then avoids having to run the API again there?
Stefan
+#include "virfile.h"
+
+static inline void
+qemuTestSharedStorage(virQEMUDriver *driver, virDomainObj *vm)
+{
+virDomainDef *def = vm->def;
+size_t i;
+int n;
+
+for (i = 0; i < def->ntpms; i++) {
+if (def->tpms[i]->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) {
+n = qemuExtTPMInitPaths(driver, def, (def->tpms[i]));
+n = virFileIsSharedFS(def->tpms[i]->data.emulator.storagepath);
+fprintf(stderr, "%s is shared: %d\n",
def->tpms[i]->data.emulator.storagepath, n);
+}
+}
+}
+
With regards,
Daniel
[libvirt PATCH 4/4] qemu: add tests for external swtpm
Signed-off-by: Ján Tomko
---
.../tpm-external.x86_64-latest.args | 36 +++
tests/qemuxml2argvtest.c | 1 +
2 files changed, 37 insertions(+)
create mode 100644 tests/qemuxml2argvdata/tpm-external.x86_64-latest.args
diff --git a/tests/qemuxml2argvdata/tpm-external.x86_64-latest.args
b/tests/qemuxml2argvdata/tpm-external.x86_64-latest.args
new file mode 100644
index 00..41421af6e1
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-external.x86_64-latest.args
@@ -0,0 +1,36 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-TPM-VM \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-TPM-VM/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-TPM-VM/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-TPM-VM/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=TPM-VM,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-TPM-VM/master-key.aes"}'
\
+-machine pc-i440fx-2.12,usb=off,dump-guest-core=off,memory-backend=pc.ram \
+-accel tcg \
+-cpu qemu64 \
+-m 2048 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":2147483648}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot menu=on,strict=on \
+-device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-chardev socket,id=chrtpm,path=/tmp/path.sock \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-device '{"driver":"tpm-tis","tpmdev":"tpm-tpm0","id":"tpm0"}' \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-device
'{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
+-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index de69cd426a..76bb771273 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2296,6 +2296,7 @@ mymain(void)
DO_TEST_CAPS_LATEST_PPC64("tpm-emulator-spapr");
DO_TEST_CAPS_ARCH_LATEST("aarch64-tpm", "aarch64");
DO_TEST_PARSE_ERROR_NOCAPS("aarch64-tpm-wrong-model");
+DO_TEST_CAPS_LATEST("tpm-external");
g_setenv(TEST_TPM_ENV_VAR, TPM_VER_2_0, true);
DO_TEST_CAPS_LATEST_PARSE_ERROR("tpm-emulator");
--
2.37.3
[libvirt PATCH 3/4] qemu: add external backend for tpm
Introduce a new backend type 'external' for connecting to a swtpm daemon
not managed by libvirtd.
Mostly in one commit, thanks to -Wswitch and the way we generate
capabilities.
https://bugzilla.redhat.com/show_bug.cgi?id=2063723
Signed-off-by: Ján Tomko
---
src/conf/domain_audit.c | 11 +
src/conf/domain_conf.c| 16
src/conf/domain_conf.h| 4 ++
src/conf/domain_validate.c| 1 +
src/conf/schemas/domaincommon.rng | 18 +
src/qemu/qemu_capabilities.c | 4 +-
src/qemu/qemu_cgroup.c| 1 +
src/qemu/qemu_command.c | 11 -
src/qemu/qemu_domain.c| 3 ++
src/qemu/qemu_namespace.c | 1 +
src/security/security_dac.c | 2 +
src/security/security_selinux.c | 2 +
.../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_4.2.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_5.0.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 +
.../qemu_5.0.0-virt.aarch64.xml | 1 +
tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 1 +
tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_5.0.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_5.1.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_5.1.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_5.2.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 +
.../qemu_5.2.0-virt.aarch64.xml | 1 +
tests/domaincapsdata/qemu_5.2.0.aarch64.xml | 1 +
tests/domaincapsdata/qemu_5.2.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_5.2.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_6.0.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml | 1 +
.../qemu_6.0.0-virt.aarch64.xml | 1 +
tests/domaincapsdata/qemu_6.0.0.aarch64.xml | 1 +
tests/domaincapsdata/qemu_6.0.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_6.1.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_6.1.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 1 +
.../qemu_6.2.0-virt.aarch64.xml | 1 +
tests/domaincapsdata/qemu_6.2.0.aarch64.xml | 1 +
tests/domaincapsdata/qemu_6.2.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_6.2.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_7.0.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_7.0.0-tcg.x86_64.xml | 1 +
.../qemu_7.0.0-virt.aarch64.xml | 1 +
tests/domaincapsdata/qemu_7.0.0.aarch64.xml | 1 +
tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_7.0.0.x86_64.xml| 1 +
.../domaincapsdata/qemu_7.1.0-q35.x86_64.xml | 1 +
.../domaincapsdata/qemu_7.1.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_7.1.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_7.1.0.x86_64.xml| 1 +
tests/qemuxml2argvdata/tpm-external.xml | 40 +++
.../tpm-external.x86_64-latest.xml| 1 +
tests/qemuxml2xmltest.c | 1 +
57 files changed, 156 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuxml2argvdata/tpm-external.xml
create mode 12 tests/qemuxml2xmloutdata/tpm-external.x86_64-latest.xml
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 974df5a037..82cf6ab749 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -556,6 +556,17 @@ virDomainAuditTPM(virDomainObj *vm, virDomainTPMDef *tpm,
"virt=%s resrc=tpm-emulator reason=%s %s uuid=%s %s",
virt, reason, vmname, uuidstr, device);
break;
+case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
+path = tpm->data.external.source->data.nix.path;
+if (!(device = virAuditEncode("device", VIR_AUDIT_STR(path {
+VIR_WARN("OOM while encoding audit message");
+goto cleanup;
+}
+
+VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "virt=%s resrc=tpm-external reason=%s %s uuid=%s %s",
+ virt, reason, vmname, uuidstr, device);
+break;
case VIR_DOMAIN_TPM_TYPE_LAST:
default:
break;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7dba65cfeb..2059c60a63 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1280,6 +1280,7 @@ VIR_ENUM_IMPL(virDomainTPMBackend,
VIR_DOMAIN_TPM_TYPE_LAST,
"passthrough",
"emulator",
+ "external",
);
VIR_ENUM_IMPL(virDomainTPMVersion,
@@ -3291,6 +3292,9 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
g_free(def->data.emulator.logfile);
[libvirt PATCH 1/4] schema: domain: Allow interleaving of 'tpm' config elements
From: Peter Krempa Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index d346442510..6e30512c73 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5530,13 +5530,15 @@ - - - - - - - + + + + + + + + + @@ -5551,10 +5553,12 @@ - emulator +emulator - - + + + + -- 2.37.3
[libvirt PATCH 2/4] qemu: tpm: fix spacing
Signed-off-by: Ján Tomko
---
src/qemu/qemu_tpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index dc09c94a4d..789f00fb69 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -213,7 +213,7 @@ qemuTPMEmulatorCreateStorage(virDomainTPMDef *tpm,
static void
qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm)
{
-g_autofree char *path =
g_path_get_dirname(tpm->data.emulator.storagepath);
+g_autofree char *path = g_path_get_dirname(tpm->data.emulator.storagepath);
ignore_value(virFileDeleteTree(path));
}
--
2.37.3
[libvirt RFC PATCH 0/4] add external backend for tpm
Ján Tomko (3): qemu: tpm: fix spacing qemu: add external backend for tpm qemu: add tests for external swtpm Peter Krempa (1): schema: domain: Allow interleaving of 'tpm' config elements src/conf/domain_audit.c | 11 + src/conf/domain_conf.c| 16 +++ src/conf/domain_conf.h| 4 ++ src/conf/domain_validate.c| 1 + src/conf/schemas/domaincommon.rng | 42 ++- src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_cgroup.c| 1 + src/qemu/qemu_command.c | 11 - src/qemu/qemu_domain.c| 3 ++ src/qemu/qemu_namespace.c | 1 + src/qemu/qemu_tpm.c | 2 +- src/security/security_dac.c | 2 + src/security/security_selinux.c | 2 + .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.x86_64.xml| 1 + .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 + .../qemu_5.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.x86_64.xml| 1 + .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_5.1.0.x86_64.xml| 1 + .../domaincapsdata/qemu_5.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 + .../qemu_5.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.x86_64.xml| 1 + .../domaincapsdata/qemu_6.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml | 1 + .../qemu_6.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.0.0.x86_64.xml| 1 + .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_6.1.0.x86_64.xml| 1 + .../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 1 + .../qemu_6.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.2.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_6.2.0.x86_64.xml| 1 + .../domaincapsdata/qemu_7.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_7.0.0-tcg.x86_64.xml | 1 + .../qemu_7.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_7.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_7.0.0.x86_64.xml| 1 + .../domaincapsdata/qemu_7.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_7.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_7.1.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_7.1.0.x86_64.xml| 1 + .../tpm-external.x86_64-latest.args | 36 tests/qemuxml2argvdata/tpm-external.xml | 40 ++ tests/qemuxml2argvtest.c | 1 + .../tpm-external.x86_64-latest.xml| 1 + tests/qemuxml2xmltest.c | 1 + 60 files changed, 208 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-external.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tpm-external.xml create mode 12 tests/qemuxml2xmloutdata/tpm-external.x86_64-latest.xml -- 2.37.3
Re: [PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration
On Mon, Oct 17, 2022 at 09:39:52AM -0400, Stefan Berger wrote:
>
>
> On 10/14/22 11:28, Daniel P. Berrangé wrote:
> > On Thu, Oct 06, 2022 at 04:07:13PM +0200, Michal Prívozník wrote:
> > > On 10/6/22 15:47, Daniel P. Berrangé wrote:
> > > > On Wed, Oct 05, 2022 at 10:02:00AM -0400, Stefan Berger wrote:
> > > > > Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for migrating a TPM across
> > > > > shared storage.
> > > > >
> > > > > At this point do not support this flag in 'virsh', yet.
> > > > >
> > > > > Signed-off-by: Stefan Berger
> > > > > ---
> > > > > include/libvirt/libvirt-domain.h | 8
> > > > > 1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/include/libvirt/libvirt-domain.h
> > > > > b/include/libvirt/libvirt-domain.h
> > > > > index 8357aea797..110929039d 100644
> > > > > --- a/include/libvirt/libvirt-domain.h
> > > > > +++ b/include/libvirt/libvirt-domain.h
> > > > > @@ -1098,6 +1098,14 @@ typedef enum {
> > > > >* Since: 8.5.0
> > > > >*/
> > > > > VIR_MIGRATE_ZEROCOPY = (1 << 20),
> > > > > +
> > > > > +/* Support TPM migration across hosts that have shared storage
> > > > > setup for
> > > > > + * the directory structure holding the state of TPMs. Typically
> > > > > this would
> > > > > + * mean that the directory /var/lib/libvirt/swtpm is shared.
> > > > > + *
> > > > > + * Since: 8.9.0
> > > > > + */
> > > > > +VIR_MIGRATE_TPM_SHARED_STORAGE = (1 << 21),
> > > >
> > > > Why do we need this flag at all. We don't require users to set any flag
> > > > when dealing with shared storage for disks, we just make sure we detect
> > > > shared storage and "do the right thing" with it.
> > >
> > > That could work. Until the state is stored on a shared FS but not shared
> > > across migration hosts. But I guess our disk migration code would fail
> > > then too, wouldn't it?
> >
> > Exactly, if our existing code is good enough for disks for the last
> > NNN years, then its good enough for TPM too. If someone finds a broken
> > scenario, then we'll need to fix it for all cases, and that'll require
> > something more general than a VIR_MIGRATE_TPM_SHARED_STORAGE flag.
> >
> > It is basically akin to a "make it work=yes" setting, and actually
> > introduces failure scenarios that would not otherwise exist. eg
> > if someone sets VIR_MIGRATE_TPM_SHARED_STORAGE when the TPM is on
> > local only storage, or fails to set it when using shared storage.
>
>
> I was trying to find out how storage is being copied and whether there
> is any testing going on regarding whether storage for storage is shared
> or not and how that's done. However, it seems there's an explicit hint
> coming from the user encoded in the virsh command line options about
> non-shared storage for storage. Or maybe I not dig deep enough?
>
> if (vshCommandOptBool(cmd, "copy-storage-all"))
> flags |= VIR_MIGRATE_NON_SHARED_DISK;
>
> if (vshCommandOptBool(cmd, "copy-storage-inc"))
> flags |= VIR_MIGRATE_NON_SHARED_INC;
>
> if (vshCommandOptBool(cmd, "copy-storage-synchronous-writes")) {
> if (!(flags & VIR_MIGRATE_NON_SHARED_DISK) &&
> !(flags & VIR_MIGRATE_NON_SHARED_INC)) {
> vshError(ctl, "'--copy-storage-synchronous-writes' requires one
> of '--copy-storage-all', 'copy-storage-inc'");
> goto out;
> }
> flags |= VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES;
> }
The key is in qemuMigrationSrcIsSafe(), and how it determines if a
migration is safe.
* Disk on local storage, no flags => unsafe, migration error
* Disk on local storage, VIR_MIGRATE_NON_SHARED_DISK => ok, copies disk
storage
* Disk on shared storage, no flags => safe
* Disk on shared storage, VIR_MIGRATE_NON_SHARED_DISK => ok, but needlessly
copies disk storage
The key helper methods are virFileIsSharedFS and virFileIsClusterFS
which check the filesystem type for the path against a known list
of shared/cluster FS.
> So we won't do it this way for TPM state migration. Instead we can
> try to write on the source migration side
>
> a) a simple file and detect whether the file is at the destination
> b) a file with either a name or content that only the source and
>destination libvirts would know at this point
>
> b) is to prevent stale files from becoming indicators for shared storage.
No, please use the virFileIsSharedFS/ClusterFS helpers.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration
On 10/14/22 11:28, Daniel P. Berrangé wrote:
On Thu, Oct 06, 2022 at 04:07:13PM +0200, Michal Prívozník wrote:
On 10/6/22 15:47, Daniel P. Berrangé wrote:
On Wed, Oct 05, 2022 at 10:02:00AM -0400, Stefan Berger wrote:
Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for migrating a TPM across
shared storage.
At this point do not support this flag in 'virsh', yet.
Signed-off-by: Stefan Berger
---
include/libvirt/libvirt-domain.h | 8
1 file changed, 8 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 8357aea797..110929039d 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1098,6 +1098,14 @@ typedef enum {
* Since: 8.5.0
*/
VIR_MIGRATE_ZEROCOPY = (1 << 20),
+
+/* Support TPM migration across hosts that have shared storage setup for
+ * the directory structure holding the state of TPMs. Typically this would
+ * mean that the directory /var/lib/libvirt/swtpm is shared.
+ *
+ * Since: 8.9.0
+ */
+VIR_MIGRATE_TPM_SHARED_STORAGE = (1 << 21),
Why do we need this flag at all. We don't require users to set any flag
when dealing with shared storage for disks, we just make sure we detect
shared storage and "do the right thing" with it.
That could work. Until the state is stored on a shared FS but not shared
across migration hosts. But I guess our disk migration code would fail
then too, wouldn't it?
Exactly, if our existing code is good enough for disks for the last
NNN years, then its good enough for TPM too. If someone finds a broken
scenario, then we'll need to fix it for all cases, and that'll require
something more general than a VIR_MIGRATE_TPM_SHARED_STORAGE flag.
It is basically akin to a "make it work=yes" setting, and actually
introduces failure scenarios that would not otherwise exist. eg
if someone sets VIR_MIGRATE_TPM_SHARED_STORAGE when the TPM is on
local only storage, or fails to set it when using shared storage.
I was trying to find out how storage is being copied and whether there is any
testing going on regarding whether storage for storage is shared or not and how
that's done. However, it seems there's an explicit hint coming from the user
encoded in the virsh command line options about non-shared storage for storage.
Or maybe I not dig deep enough?
if (vshCommandOptBool(cmd, "copy-storage-all"))
flags |= VIR_MIGRATE_NON_SHARED_DISK;
if (vshCommandOptBool(cmd, "copy-storage-inc"))
flags |= VIR_MIGRATE_NON_SHARED_INC;
if (vshCommandOptBool(cmd, "copy-storage-synchronous-writes")) {
if (!(flags & VIR_MIGRATE_NON_SHARED_DISK) &&
!(flags & VIR_MIGRATE_NON_SHARED_INC)) {
vshError(ctl, "'--copy-storage-synchronous-writes' requires one of
'--copy-storage-all', 'copy-storage-inc'");
goto out;
}
flags |= VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES;
}
So we won't do it this way for TPM state migration. Instead we can try to write
on the source migration side
a) a simple file and detect whether the file is at the destination
b) a file with either a name or content that only the source and destination
libvirts would know at this point
b) is to prevent stale files from becoming indicators for shared storage.
If you have any suggestions on how to do it or what data to write into the test
file, let me know. A hash over some shared knowledge would do the trick I guess.
Stefan
Ergo, we should not add this flag to our public API.
With regards,
Daniel
Re: [PATCH 00/15] schema: Fix forgotten interleaving of elements (part 1)
On a Monday in 2022, Peter Krempa wrote: Context: https://listman.redhat.com/archives/libvir-list/2022-October/234860.html This series fixes the trivial/straightforward cases found by using Daniel's "fuzzing" script. Note that you have to exclude '-invalid.xml' files and all output-only XMLs (domaincaps, capabilities). Best viewed with 'git show -w'. There's 7 more pathces needed to fix the schema to work with the above mentioned script but they have more complex interactions and I'll need to re-consider them. Peter Krempa (15): schema: domain: Allow interleaving of character device config elements schema: domain: Allow interleaving of PCI controller config elements schema: domain: Allow interleaving of 'tpm' config elements schema: domain: Allow interleaving of subelements of 'memtune' schema: domain: Allow interleaving of subelements of disk's 'mirror' schema: domain: Allow interleaving 'ip' and 'route' sub-elements of interface schema: domain: Add the 'type' subelement of the osexe case of 'os' element to interleave schema: domain: Allow interleaving of and schema: domain: Allow interleaving of 'watchdog' subelements schema: networkport: Allow interleaving of subelements of 'owner' schema: networkport: Allow interleaving of subelements of 'driver' subelement of hostdev-pci schema: nwfilter: Allow interleaving subelements of the top level 'filter' element schema: nwfilterbinding: Allow interleaving of subelements of 'owner' element schema: storagepool: Allow interleaving of per-pool custom namespace elements schema: cpu: include 'arch' subelement in interleave definition of 'hostcpu' src/conf/schemas/cputypes.rng| 6 +- src/conf/schemas/domaincommon.rng| 813 ++- src/conf/schemas/networkport.rng | 42 +- src/conf/schemas/nwfilter.rng| 36 +- src/conf/schemas/nwfilterbinding.rng | 14 +- src/conf/schemas/storagepool.rng | 12 +- 6 files changed, 476 insertions(+), 447 deletions(-) Reviewed-by: Ján Tomko Jano
[PATCH 14/15] schema: storagepool: Allow interleaving of per-pool custom namespace elements
The custom namespace parameters for 'rbd' and 'netfs' pool types were not included in the interleave statement. Signed-off-by: Peter Krempa --- src/conf/schemas/storagepool.rng | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/schemas/storagepool.rng b/src/conf/schemas/storagepool.rng index d81ead532a..63a8b75fd8 100644 --- a/src/conf/schemas/storagepool.rng +++ b/src/conf/schemas/storagepool.rng @@ -69,10 +69,10 @@ + + + - - - @@ -166,10 +166,10 @@ + + + - - - -- 2.37.3
[PATCH 11/15] schema: networkport: Allow interleaving of subelements of 'driver' subelement of hostdev-pci
Signed-off-by: Peter Krempa --- src/conf/schemas/networkport.rng | 28 +++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/conf/schemas/networkport.rng b/src/conf/schemas/networkport.rng index 2ca76c10df..14db949578 100644 --- a/src/conf/schemas/networkport.rng +++ b/src/conf/schemas/networkport.rng @@ -143,20 +143,22 @@ - - - - -kvm -vfio - - - + + + + + + kvm + vfio + + + + + + + - - - - + -- 2.37.3
[PATCH 13/15] schema: nwfilterbinding: Allow interleaving of subelements of 'owner' element
Signed-off-by: Peter Krempa --- src/conf/schemas/nwfilterbinding.rng | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/conf/schemas/nwfilterbinding.rng b/src/conf/schemas/nwfilterbinding.rng index a0a956eb01..c91312b09d 100644 --- a/src/conf/schemas/nwfilterbinding.rng +++ b/src/conf/schemas/nwfilterbinding.rng @@ -12,12 +12,14 @@ - - - - - - + + + + + + + + -- 2.37.3
[PATCH 12/15] schema: nwfilter: Allow interleaving subelements of the top level 'filter' element
Signed-off-by: Peter Krempa --- src/conf/schemas/nwfilter.rng | 36 ++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/conf/schemas/nwfilter.rng b/src/conf/schemas/nwfilter.rng index a75de7ed3e..262bd551e3 100644 --- a/src/conf/schemas/nwfilter.rng +++ b/src/conf/schemas/nwfilter.rng @@ -7,20 +7,21 @@ - - - - - - - - - - + + + + + - - - + + + + + + + + + @@ -199,10 +200,11 @@ - - - - + + + + + -- 2.37.3
[PATCH 15/15] schema: cpu: include 'arch' subelement in interleave definition of 'hostcpu'
Signed-off-by: Peter Krempa --- src/conf/schemas/cputypes.rng | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/schemas/cputypes.rng b/src/conf/schemas/cputypes.rng index 986a5f3715..3e79bdd563 100644 --- a/src/conf/schemas/cputypes.rng +++ b/src/conf/schemas/cputypes.rng @@ -323,10 +323,10 @@ - - - + + + -- 2.37.3
[PATCH 10/15] schema: networkport: Allow interleaving of subelements of 'owner'
Signed-off-by: Peter Krempa --- src/conf/schemas/networkport.rng | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/conf/schemas/networkport.rng b/src/conf/schemas/networkport.rng index 1a12a32c3c..2ca76c10df 100644 --- a/src/conf/schemas/networkport.rng +++ b/src/conf/schemas/networkport.rng @@ -44,12 +44,14 @@ - - - - - - + + + + + + + + -- 2.37.3
[PATCH 06/15] schema: domain: Allow interleaving 'ip' and 'route' sub-elements of interface
Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 50 --- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index a1b004921b..b6371940e4 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3806,32 +3806,34 @@ All ip-related info for either the host or guest side of an interface --> - - - - - - - - - - - - - - - - - + + + + - - - - - - - + + + + + + + + + + + + + + + + + + + + + + -- 2.37.3
[PATCH 08/15] schema: domain: Allow interleaving of and
Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 203 +++--- 1 file changed, 103 insertions(+), 100 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 2801f8fcb8..57457d1aa8 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -281,116 +281,119 @@ - - - - - - - - - bios - efi - - - - - - - - - + + + + + + + + + +bios +efi + + + + + + + + + + + + + + enrolled-keys + secure-boot + + + + + + + + + + - + + + + + + + + -enrolled-keys -secure-boot +rom +pflash - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + - rom - pflash + + + + + + - - - - + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + -- 2.37.3
[PATCH 09/15] schema: domain: Allow interleaving of 'watchdog' subelements
Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 54 --- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 57457d1aa8..ebdf21fe99 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5264,35 +5264,37 @@ - - - i6300esb - ib700 - diag288 - - - - + + -reset -shutdown -poweroff -pause -none -dump -inject-nmi +i6300esb +ib700 +diag288 - - - - - - - - - - + + + + reset + shutdown + poweroff + pause + none + dump + inject-nmi + + + + + + + + + + + + + -- 2.37.3
[PATCH 05/15] schema: domain: Allow interleaving of subelements of disk's 'mirror'
While for now the 'mirror' element is output only, the idea was to allow it to be used for input too to restore the mirror job if that becomes the necessity. Allowing interleaving of the subelements can be done regardless. Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 79 --- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 99a705aa3c..a1b004921b 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -7141,57 +7141,60 @@ - - - - - - - - + + + + + - - + + + + + + + +copy + + + + + + + + + + + + - copy + +copy +active-commit + - - - + - - - - + + + + - copy - active-commit + yes + abort + pivot - - - - - - - - - - - -yes -abort -pivot - - - - + + + + -- 2.37.3
[PATCH 07/15] schema: domain: Add the 'type' subelement of the osexe case of 'os' element to interleave
The 'type' element was outside of the 'interleave' definition. Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index b6371940e4..2801f8fcb8 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -438,15 +438,15 @@ - - - - - - -exe - + + + + + + + exe + -- 2.37.3
[PATCH 01/15] schema: domain: Allow interleaving of character device config elements
Allow interleaving in the 'qemucdevSrcDef' definition which is shared by all places using character device as backend. Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 143 +++--- 1 file changed, 73 insertions(+), 70 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index d346442510..49a61d2748 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -4716,82 +4716,85 @@ + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + - + - + - - - - - - - - - raw - telnet - telnets - tls - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +raw +telnet +telnets +tls + + + + + + + + + - - - + + + + + + + +
[PATCH 04/15] schema: domain: Allow interleaving of subelements of 'memtune'
Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 50 --- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index ea10d13a4f..99a705aa3c 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -956,30 +956,32 @@ - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + -- 2.37.3
[PATCH 02/15] schema: domain: Allow interleaving of PCI controller config elements
The 'model' and 'target' element can be freely moved around. Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 190 +++--- 1 file changed, 96 insertions(+), 94 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 49a61d2748..ecc9cd41c4 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2691,103 +2691,105 @@ pci - - - - - -spapr-pci-host-bridge - -pci-bridge - -i82801b11-bridge - -pcie-pci-bridge - -ioh3420 -pcie-root-port - -x3130-upstream - -xio3130-downstream - -pxb - -pxb-pcie - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + spapr-pci-host-bridge + + pci-bridge + + i82801b11-bridge + + pcie-pci-bridge + + ioh3420 + pcie-root-port + + x3130-upstream + + xio3130-downstream + + pxb + + pxb-pcie + - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + pci-root + pcie-root + - - - - + + + + + + + + + + pci-bridge + dmi-to-pci-bridge + pcie-to-pci-bridge + pcie-root-port + pcie-switch-upstream-port + pcie-switch-downstream-port + pci-expander-bus + pcie-expander-bus + - - - - - - - - - - - - - -pci-root -pcie-root - - - - - - - - - - - -pci-bridge -dmi-to-pci-bridge -pcie-to-pci-bridge -pcie-root-port -pcie-switch-upstream-port -pcie-switch-downstream-port -pci-expander-bus -pcie-expander-bus - - - - + +
[PATCH 03/15] schema: domain: Allow interleaving of 'tpm' config elements
Signed-off-by: Peter Krempa --- src/conf/schemas/domaincommon.rng | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index ecc9cd41c4..ea10d13a4f 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5535,13 +5535,15 @@ - - - - - - - + + + + + + + + + @@ -5556,10 +5558,12 @@ - emulator +emulator - - + + + + -- 2.37.3
[PATCH 00/15] schema: Fix forgotten interleaving of elements (part 1)
Context: https://listman.redhat.com/archives/libvir-list/2022-October/234860.html This series fixes the trivial/straightforward cases found by using Daniel's "fuzzing" script. Note that you have to exclude '-invalid.xml' files and all output-only XMLs (domaincaps, capabilities). Best viewed with 'git show -w'. There's 7 more pathces needed to fix the schema to work with the above mentioned script but they have more complex interactions and I'll need to re-consider them. Peter Krempa (15): schema: domain: Allow interleaving of character device config elements schema: domain: Allow interleaving of PCI controller config elements schema: domain: Allow interleaving of 'tpm' config elements schema: domain: Allow interleaving of subelements of 'memtune' schema: domain: Allow interleaving of subelements of disk's 'mirror' schema: domain: Allow interleaving 'ip' and 'route' sub-elements of interface schema: domain: Add the 'type' subelement of the osexe case of 'os' element to interleave schema: domain: Allow interleaving of and schema: domain: Allow interleaving of 'watchdog' subelements schema: networkport: Allow interleaving of subelements of 'owner' schema: networkport: Allow interleaving of subelements of 'driver' subelement of hostdev-pci schema: nwfilter: Allow interleaving subelements of the top level 'filter' element schema: nwfilterbinding: Allow interleaving of subelements of 'owner' element schema: storagepool: Allow interleaving of per-pool custom namespace elements schema: cpu: include 'arch' subelement in interleave definition of 'hostcpu' src/conf/schemas/cputypes.rng| 6 +- src/conf/schemas/domaincommon.rng| 813 ++- src/conf/schemas/networkport.rng | 42 +- src/conf/schemas/nwfilter.rng| 36 +- src/conf/schemas/nwfilterbinding.rng | 14 +- src/conf/schemas/storagepool.rng | 12 +- 6 files changed, 476 insertions(+), 447 deletions(-) -- 2.37.3
[libvirt PATCH] docs: formatdomain: fix since tag for TPM PCR banks
Fixes: a5bbe1a8b6321852634817c423695e58518c4f4f Signed-off-by: Ján Tomko --- Pushed as trivial. docs/formatdomain.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 34e4906eb4..e28b805009 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7784,7 +7784,7 @@ Example: usage of the TPM Emulator active PCR banks upon VM start but leave them at their last configuration. This attribute requires that swtpm_setup v0.7 or later is installed and may not have any effect otherwise. The selection of PCR banks only works - with the ``emulator`` backend. since:`Since 7.10.0` + with the ``emulator`` backend. :since:`Since 7.10.0` ``encryption`` The ``encryption`` element allows the state of a TPM emulator to be -- 2.37.3
Re: [PATCH v2] capabilities: use g_autofree in capabilities.c
On 10/15/22 11:18, Jiang Jiacheng wrote: > Use g_autofree in capabilities.c for some pointers still using manual cleanup, > and remove unnecessary cleanup. > > Signed-off-by: Jiang Jiacheng > --- > src/conf/capabilities.c | 38 -- > 1 file changed, 12 insertions(+), 26 deletions(-) Reviewed-by: Michal Privoznik and pushed. Michal
Re: [libvirt PATCH] virvhba.c: use g_autofree
On 10/16/22 12:47, ttxine wrote: > Change strings to use g_autofree. > > Signed-off-by: Maxim Kostin > --- > src/util/virvhba.c | 21 +++-- > 1 file changed, 7 insertions(+), 14 deletions(-) We can do a bit more. Some variables are only used within loop bodies. Those can be declared inside of their respective bodies and marked as g_autofree. After this, some labels contain nothing just a return statement, so those can be dropped too. I'm fixing those places and pushing. Congratulations on your first libvirt contribution! Reviewed-by: Michal Privoznik Michal
Re: [PATCH 2/3] qemu: get the stackManager lock before getting nested lock
[please keep the list on CC] On 10/15/22 13:55, Jiang Jiacheng wrote: > Thanks for your reply. > In other similar situation, we will get the StacksecurityManager lock > before get other driver's lock to keep the use of the security drivers > is internally serialized(in 'virSecurityManagerGenLabel'). > > In the functions of this patch, we haven't get the StacksecurityManager > lock but get the lock of other drivers in qemuSecurityGetBaseLabel. I > think it's the reason of this problem. > > And I think 'virSecurityManagerGetNested' is used to get a list of other > SecurityManager, i'm not sure whether we need a lock here. Yeah, we might need to rework locking a bit. Or shuffle some code around, but locking the object from outside does not feel right, since it's supposed to manage locks itself. Michal
Re: [PATCH] qemu: Report sev measurement value and nonce explicitly
On 10/16/22 22:06, Cole Robinson wrote: > The value returned by qemu's query-sev-launch-measure comes > straight from the LAUNCH_MEASURE SEV firmware command. It's two > values packed together: first 32 bytes is the launch measurement, > last 16 bytes is the nonce. > > This combined value is really just an artifact of the return value > of the firmware command, it has no direct usage. Users want the two > individual values. But because qemu and libvirt do not separate them > apart, every app that wants to process this value will have to do > it manually. > > This performs the split for the user, and delivers the values in two > new TYPED_PARAM fields: sev-measurement-value, sev-measurement-nonce > > Signed-off-by: Cole Robinson > --- > include/libvirt/libvirt-domain.h | 22 ++ > src/qemu/qemu_driver.c | 23 +++ > 2 files changed, 45 insertions(+) > Reviewed-by: Michal Privoznik Michal
Re: [PATCH V3] conf: Add channel devices to domain capabilities
On 10/15/22 00:23, Jim Fehlig wrote: > As qemu becomes more modularized, it is important for libvirt to advertise > availability of the modularized functionality through capabilities. This > change adds channel devices to domain capabilities, allowing clients such > as virt-install to avoid using spicevmc channel devices when not supported > by the target qemu. > > Signed-off-by: Jim Fehlig > --- > > V2: > https://listman.redhat.com/archives/libvir-list/2022-October/234840.html > > New in V3: > - rebased to current master > - use existing QEMU_CAPS_SPICE for spicevmc channel device > > docs/formatdomaincaps.rst | 24 +++ > src/conf/domain_capabilities.c| 13 ++ > src/conf/domain_capabilities.h| 8 +++ > src/conf/schemas/domaincaps.rng | 10 > src/qemu/qemu_capabilities.c | 15 > src/qemu/qemu_capabilities.h | 3 +++ > .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 7 ++ > .../qemu_4.2.0-virt.aarch64.xml | 6 + > tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 6 + > tests/domaincapsdata/qemu_4.2.0.ppc64.xml | 6 + > tests/domaincapsdata/qemu_4.2.0.s390x.xml | 6 + > tests/domaincapsdata/qemu_4.2.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 7 ++ > .../qemu_5.0.0-virt.aarch64.xml | 6 + > tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 6 + > tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 6 + > tests/domaincapsdata/qemu_5.0.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 7 ++ > tests/domaincapsdata/qemu_5.1.0.sparc.xml | 7 ++ > tests/domaincapsdata/qemu_5.1.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_5.2.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 7 ++ > .../qemu_5.2.0-virt.aarch64.xml | 6 + > tests/domaincapsdata/qemu_5.2.0.aarch64.xml | 6 + > tests/domaincapsdata/qemu_5.2.0.ppc64.xml | 6 + > tests/domaincapsdata/qemu_5.2.0.s390x.xml | 6 + > tests/domaincapsdata/qemu_5.2.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_6.0.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml | 7 ++ > .../qemu_6.0.0-virt.aarch64.xml | 6 + > tests/domaincapsdata/qemu_6.0.0.aarch64.xml | 6 + > tests/domaincapsdata/qemu_6.0.0.s390x.xml | 6 + > tests/domaincapsdata/qemu_6.0.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml | 7 ++ > tests/domaincapsdata/qemu_6.1.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 7 ++ > .../qemu_6.2.0-virt.aarch64.xml | 7 ++ > tests/domaincapsdata/qemu_6.2.0.aarch64.xml | 7 ++ > tests/domaincapsdata/qemu_6.2.0.ppc64.xml | 6 + > tests/domaincapsdata/qemu_6.2.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_7.0.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_7.0.0-tcg.x86_64.xml | 7 ++ > .../qemu_7.0.0-virt.aarch64.xml | 7 ++ > tests/domaincapsdata/qemu_7.0.0.aarch64.xml | 7 ++ > tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 6 + > tests/domaincapsdata/qemu_7.0.0.x86_64.xml| 7 ++ > .../domaincapsdata/qemu_7.1.0-q35.x86_64.xml | 7 ++ > .../domaincapsdata/qemu_7.1.0-tcg.x86_64.xml | 7 ++ > tests/domaincapsdata/qemu_7.1.0.ppc64.xml | 6 + > tests/domaincapsdata/qemu_7.1.0.x86_64.xml| 7 ++ > 55 files changed, 399 insertions(+) Reviewed-by: Michal Privoznik Michal
Re: [PATCH V2 2/3] conf: Add channel devices to domain capabilities
On 10/14/22 23:34, Jim Fehlig wrote: > > Yes, it does. Thanks for the detailed response! I suppose we'll need to > rethink the dependencies in our downstream packages. E.g. currently it's > possible to install qemu-ui-spice-core (which contains > ui-spice-core.so), but not qemu-chardev-spice (which contains > chardev-spice.so). That seems to fly in the face of the upstream logic. Ah, so this kind of warrants resurrection of the old capability, because the assumption the commit which retired the capabilty made is not correct anymore (thanks to qemu dynamically loading modules). BUT, if we want to do so, we must remove the X_ prefix first because that prefix is reserved for retired capabilities [1]. But yeah, breaking down qemu too much, into many separate packages looks like overkill. And I see you already posted v3 so let me review that. Michal 1: Why we don't just remove the capability from virQEMUCaps enum? Because in the domain status XML we store the set of qemu capabilities the domain was started with. And then, when libvirtd restarts we use that str2enum helper (declared at the beginning of qemu_capabilities.c) to reconstruct the capabilities bitmap. QEMU and Libvirt might have been upgraded after the domain was started. And if we were to remove a capability, the str2enum helper would simply fail and we wouldn't be able to reconstruct the caps.
