It was clearly copied over from the SELinux driver without updating its name in the process. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/security/security_dac.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 4b8130630f..7421496fc9 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1671,7 +1671,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManager *mgr, } -struct _virSecuritySELinuxChardevCallbackData { +struct _virSecurityDACChardevCallbackData { virSecurityManager *mgr; bool chardevStdioLogd; }; @@ -1682,7 +1682,7 @@ virSecurityDACRestoreChardevCallback(virDomainDef *def, virDomainChrDef *dev G_GNUC_UNUSED, void *opaque) { - struct _virSecuritySELinuxChardevCallbackData *data = opaque; + struct _virSecurityDACChardevCallbackData *data = opaque; return virSecurityDACRestoreChardevLabel(data->mgr, def, dev->source, data->chardevStdioLogd); @@ -1916,7 +1916,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, size_t i; int rc = 0; - struct _virSecuritySELinuxChardevCallbackData chardevData = { + struct _virSecurityDACChardevCallbackData chardevData = { .mgr = mgr, .chardevStdioLogd = chardevStdioLogd, }; @@ -2018,7 +2018,7 @@ virSecurityDACSetChardevCallback(virDomainDef *def, virDomainChrDef *dev G_GNUC_UNUSED, void *opaque) { - struct _virSecuritySELinuxChardevCallbackData *data = opaque; + struct _virSecurityDACChardevCallbackData *data = opaque; return virSecurityDACSetChardevLabel(data->mgr, def, dev->source, data->chardevStdioLogd); @@ -2141,7 +2141,7 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, uid_t user; gid_t group; - struct _virSecuritySELinuxChardevCallbackData chardevData = { + struct _virSecurityDACChardevCallbackData chardevData = { .mgr = mgr, .chardevStdioLogd = chardevStdioLogd, }; -- 2.44.0

The single caller for each function passes the same value for @src and @parent, which means that we don't really need the additional API. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/security/security_dac.c | 19 +++++-------------- src/security/security_selinux.c | 19 +++++-------------- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9c24a1c4a8..567be4bd23 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -939,12 +939,12 @@ virSecurityDACSetImageLabelInternal(virSecurityManager *mgr, static int -virSecurityDACSetImageLabelRelative(virSecurityManager *mgr, - virDomainDef *def, - virStorageSource *src, - virStorageSource *parent, - virSecurityDomainImageLabelFlags flags) +virSecurityDACSetImageLabel(virSecurityManager *mgr, + virDomainDef *def, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags) { + virStorageSource *parent = src; virStorageSource *n; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { @@ -962,15 +962,6 @@ virSecurityDACSetImageLabelRelative(virSecurityManager *mgr, return 0; } -static int -virSecurityDACSetImageLabel(virSecurityManager *mgr, - virDomainDef *def, - virStorageSource *src, - virSecurityDomainImageLabelFlags flags) -{ - return virSecurityDACSetImageLabelRelative(mgr, def, src, src, flags); -} - static int virSecurityDACRestoreImageLabelInt(virSecurityManager *mgr, virDomainDef *def, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index d491435ae1..b49af26e49 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1980,12 +1980,12 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManager *mgr, static int -virSecuritySELinuxSetImageLabelRelative(virSecurityManager *mgr, - virDomainDef *def, - virStorageSource *src, - virStorageSource *parent, - virSecurityDomainImageLabelFlags flags) +virSecuritySELinuxSetImageLabel(virSecurityManager *mgr, + virDomainDef *def, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags) { + virStorageSource *parent = src; virStorageSource *n; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { @@ -2004,15 +2004,6 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManager *mgr, } -static int -virSecuritySELinuxSetImageLabel(virSecurityManager *mgr, - virDomainDef *def, - virStorageSource *src, - virSecurityDomainImageLabelFlags flags) -{ - return virSecuritySELinuxSetImageLabelRelative(mgr, def, src, src, flags); -} - struct virSecuritySELinuxMoveImageMetadataData { virSecurityManager *mgr; const char *src; -- 2.44.0

As explained in the comment, this can help in scenarios where a shared filesystem can't be detected as such by libvirt, by giving the admin the opportunity to provide this information manually. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf.in | 17 +++++++++++++++++ src/qemu/qemu_conf.c | 17 +++++++++++++++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/test_libvirtd_qemu.aug.in | 5 +++++ 5 files changed, 44 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 2b6526538f..1377fd89cc 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -143,6 +143,8 @@ module Libvirtd_qemu = let storage_entry = bool_entry "storage_use_nbdkit" + let filesystem_entry = str_array_entry "shared_filesystems" + (* Entries that used to exist in the config which are now * deleted. We keep on parsing them so we don't break * ability to parse old configs after upgrade @@ -173,6 +175,7 @@ module Libvirtd_qemu = | swtpm_entry | capability_filters_entry | storage_entry + | filesystem_entry | obsolete_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index f406df8749..db42448239 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -986,3 +986,20 @@ # note that the default might change in future releases. # #storage_use_nbdkit = @USE_NBDKIT_DEFAULT@ + +# libvirt will normally prevent migration if the storage backing the VM is not +# on a shared filesystems. Sometimes, however, the storage *is* shared despite +# not being detected as such: for example, this is the case when one of the +# hosts involved in the migration is exporting its local storage to the other +# one via NFS. +# +# Any directory listed here will be assumed to live on a shared filesystem, +# making migration possible in scenarios such as the one described above. +# +# If you need this feature, you probably want to set remember_owner=0 too. +# +#shared_filesystems = [ +# "/var/lib/libvirt/images", +# "/var/lib/libvirt/qemu/nvram", +# "/var/lib/libvirt/swtpm" +#] diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4050a82341..01c6bcc793 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -374,6 +374,8 @@ static void virQEMUDriverConfigDispose(void *obj) g_strfreev(cfg->capabilityfilters); + g_strfreev(cfg->sharedFilesystems); + g_free(cfg->deprecationBehavior); } @@ -1084,6 +1086,18 @@ virQEMUDriverConfigLoadStorageEntry(virQEMUDriverConfig *cfg, } +static int +virQEMUDriverConfigLoadFilesystemEntry(virQEMUDriverConfig *cfg, + virConf *conf) +{ + if (virConfGetValueStringList(conf, "shared_filesystems", false, + &cfg->sharedFilesystems) < 0) + return -1; + + return 0; +} + + int virQEMUDriverConfigLoadFile(virQEMUDriverConfig *cfg, const char *filename, bool privileged) @@ -1158,6 +1172,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfig *cfg, if (virQEMUDriverConfigLoadStorageEntry(cfg, conf) < 0) return -1; + if (virQEMUDriverConfigLoadFilesystemEntry(cfg, conf) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 36049b4bfa..b53d56be02 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -233,6 +233,8 @@ struct _virQEMUDriverConfig { bool storageUseNbdkit; virQEMUSchedCore schedCore; + + char **sharedFilesystems; }; G_DEFINE_AUTOPTR_CLEANUP_FUNC(virQEMUDriverConfig, virObjectUnref); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index b97e6de11e..f0a7a2a30e 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -119,3 +119,8 @@ module Test_libvirtd_qemu = { "deprecation_behavior" = "none" } { "sched_core" = "none" } { "storage_use_nbdkit" = "@USE_NBDKIT_DEFAULT@" } +{ "shared_filesystems" + { "1" = "/var/lib/libvirt/images" } + { "2" = "/var/lib/libvirt/qemu/nvram" } + { "3" = "/var/lib/libvirt/swtpm" } +} -- 2.44.0

On Wed, Mar 20, 2024 at 12:37:37 +0100, Peter Krempa wrote: TBH this feels a bit weird. Having the NFS server on the host you are migrating from will e.g. cause storage to start have network latency which it didn't before. Additionally you can't even take the host down for maintenance as it's still serving storage for the VM. How is this expected to be used? I don't seem to understand why would anybody want to do this in contrast to e.g. migrating also the storage fully to the destination.

On Wed, Mar 20, 2024 at 01:22:15PM +0100, Peter Krempa wrote: You can do a block copy / rebase after you have migrated in order to then pull the storage over to the new host. If disk is much larger than RAM, this has the advantage that you can get the live migration completed quickly, and then let the disk copy run in the background a long time. Obviously not much use for an emergency evacuation of a failing host, but reasonable for planned maint actions. 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 :|

On Wed, Mar 20, 2024 at 08:43:24AM -0700, Andrea Bolognani wrote: I've managed to track this down, and I wasn't far off. The issue is that, when remember_owner is enabled, we perform a bunch of additional locking on files around the actual relabel operation; specifically, we call virSecurityManagerTransactionCommit() with the last argument set to true. This doesn't seem to cause any issues in general, *except* when it comes to swtpm's storage lock. The swtpm process holds this lock while it's running, and only releases it once migration is triggered. So, when we're about to start the target swtpm process and want to prepare the environment by setting up labels, we try to acquire the storage lock, and can't proceed because the source swtpm process is still holding on to it. The hacky patch below makes migration work even when remember_owner is enabled. Obviously I'd rewrite it so that we'd only skip locking for incoming migration, but I wonder if there could be nasty side effects to this... Other ideas? Can we perhaps change things so that swtpm releases the lock earlier upon our request or something like that? diff --git a/src/security/security_manager.c b/src/security/security_manager.c index fc2747c45e..2aa06eaec2 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1356,6 +1356,9 @@ virSecurityManagerMetadataLock(virSecurityManager *mgr G_GNUC_UNUSED, continue; } + if (g_str_has_suffix(p, "/.lock")) + continue; + if ((fd = open(p, O_RDWR)) < 0) { if (errno == EROFS) { /* There is nothing we can do for RO filesystem. */ -- Andrea Bolognani / Red Hat / Virtualization

virFileIsSharedFS() is the function that ultimately decides whether a filesystem should be considered shared, but the list of manually configured shared filesystems is part of the QEMU driver's configuration, so we need to pass the information through several layers in order to make use of it. Note that with this change the list is propagated all the way through, but its contents are still ignored, so the behavior remains the same for now. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 4 ++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_extdevice.c | 2 +- src/qemu/qemu_migration.c | 12 ++++++++---- src/qemu/qemu_security.c | 14 ++++++++++++-- src/qemu/qemu_tpm.c | 27 ++++++++++++++++++--------- src/qemu/qemu_tpm.h | 8 +++++--- src/security/security_apparmor.c | 2 ++ src/security/security_dac.c | 17 +++++++++++++---- src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 20 ++++++++++++++------ src/security/security_manager.h | 4 ++++ src/security/security_nop.c | 4 ++++ src/security/security_selinux.c | 18 +++++++++++++++--- src/security/security_stack.c | 16 ++++++++++++---- src/util/virfile.c | 5 +++-- src/util/virfile.h | 3 ++- tests/securityselinuxlabeltest.c | 2 +- tests/virfiletest.c | 2 +- 21 files changed, 124 insertions(+), 46 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 505b71d05e..0b82fb9624 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1919,7 +1919,7 @@ static int virLXCControllerSetupDisk(virLXCController *ctrl, /* Labelling normally operates on src, but we need * to actually label the dst here, so hack the config */ def->src->path = dst; - if (virSecurityManagerSetImageLabel(securityDriver, ctrl->def, def->src, + if (virSecurityManagerSetImageLabel(securityDriver, ctrl->def, def->src, NULL, VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0) goto cleanup; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 39992bdf96..701d22efef 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3260,7 +3260,7 @@ lxcDomainAttachDeviceMknodHelper(pid_t pid G_GNUC_UNUSED, char *tmpsrc = def->src->path; def->src->path = data->file; if (virSecurityManagerSetImageLabel(data->driver->securityManager, - data->vm->def, def->src, + data->vm->def, def->src, NULL, VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0) { def->src->path = tmpsrc; goto cleanup; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index bfdcefd01b..a426d915ab 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -171,7 +171,7 @@ static void virLXCProcessCleanup(virLXCDriver *driver, if (flags & VIR_LXC_PROCESS_CLEANUP_RESTORE_SECLABEL) { virSecurityManagerRestoreAllLabel(driver->securityManager, - vm->def, false, false); + vm->def, NULL, false, false); } if (flags & VIR_LXC_PROCESS_CLEANUP_RELEASE_SECLABEL) { @@ -1327,7 +1327,7 @@ int virLXCProcessStart(virLXCDriver * driver, VIR_DEBUG("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, - vm->def, NULL, false, false) < 0) + vm->def, NULL, NULL, false, false) < 0) goto cleanup; stopFlags |= VIR_LXC_PROCESS_CLEANUP_RESTORE_SECLABEL; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bc6cf133d4..a2f22dafe8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11877,7 +11877,7 @@ virQEMUFileOpenAs(uid_t fallback_uid, bool need_unlink = false; unsigned int vfoflags = 0; int fd = -1; - int path_shared = virFileIsSharedFS(path); + int path_shared = virFileIsSharedFS(path, NULL); uid_t uid = geteuid(); gid_t gid = getegid(); diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index ed5976d1f7..dc1bb56237 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -165,7 +165,7 @@ qemuExtDevicesCleanupHost(virQEMUDriver *driver, virDomainTPMDef *tpm = def->tpms[i]; if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) - qemuExtTPMCleanupHost(tpm, flags, outgoingMigration); + qemuExtTPMCleanupHost(driver, tpm, flags, outgoingMigration); } } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1faab5dd23..330efb069b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1429,6 +1429,8 @@ qemuMigrationSrcIsAllowed(virDomainObj *vm, unsigned int flags) { qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); int nsnapshots; int pauseReason; size_t i; @@ -1599,7 +1601,7 @@ qemuMigrationSrcIsAllowed(virDomainObj *vm, } } - if (qemuTPMHasSharedStorage(vm->def)&& + if (qemuTPMHasSharedStorage(vm->def, cfg->sharedFilesystems) && !qemuTPMCanMigrateSharedStorage(vm->def)) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("the running swtpm does not support migration with shared storage")); @@ -1612,6 +1614,7 @@ qemuMigrationSrcIsAllowed(virDomainObj *vm, static bool qemuMigrationSrcIsSafe(virDomainDef *def, + virQEMUDriverConfig *cfg, virQEMUCaps *qemuCaps, size_t nmigrate_disks, const char **migrate_disks, @@ -1643,7 +1646,7 @@ qemuMigrationSrcIsSafe(virDomainDef *def, /* However, disks on local FS (e.g. ext4) are not safe. */ switch (actualType) { case VIR_STORAGE_TYPE_FILE: - if ((rc = virFileIsSharedFS(src)) < 0) { + if ((rc = virFileIsSharedFS(src, cfg->sharedFilesystems)) < 0) { return false; } else if (rc == 0) { unsafe = true; @@ -2582,6 +2585,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, const char **migrate_disks, unsigned int flags) { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivate *priv = vm->privateData; unsigned int cookieFlags = QEMU_MIGRATION_COOKIE_LOCKSTATE; @@ -2604,7 +2608,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, return NULL; if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) && - !qemuMigrationSrcIsSafe(vm->def, priv->qemuCaps, + !qemuMigrationSrcIsSafe(vm->def, cfg, priv->qemuCaps, nmigrate_disks, migrate_disks, flags)) return NULL; @@ -6091,7 +6095,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, goto endjob; if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) && - !qemuMigrationSrcIsSafe(vm->def, priv->qemuCaps, + !qemuMigrationSrcIsSafe(vm->def, cfg, priv->qemuCaps, nmigrate_disks, migrate_disks, flags)) goto endjob; diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 4aaa863ae9..3aaa93a76c 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -38,6 +38,7 @@ qemuSecuritySetAllLabel(virQEMUDriver *driver, { int ret = -1; qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); pid_t pid = -1; if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) @@ -48,6 +49,7 @@ qemuSecuritySetAllLabel(virQEMUDriver *driver, if (virSecurityManagerSetAllLabel(driver->securityManager, vm->def, + cfg->sharedFilesystems, incomingPath, priv->chardevStdioLogd, migrated) < 0) @@ -70,6 +72,7 @@ qemuSecurityRestoreAllLabel(virQEMUDriver *driver, bool migrated) { qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool transactionStarted = false; /* In contrast to qemuSecuritySetAllLabel, do not use vm->pid @@ -83,6 +86,7 @@ qemuSecurityRestoreAllLabel(virQEMUDriver *driver, virSecurityManagerRestoreAllLabel(driver->securityManager, vm->def, + cfg->sharedFilesystems, migrated, priv->chardevStdioLogd); @@ -103,6 +107,7 @@ qemuSecuritySetImageLabel(virQEMUDriver *driver, bool chainTop) { qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); pid_t pid = -1; int ret = -1; virSecurityDomainImageLabelFlags labelFlags = 0; @@ -120,7 +125,9 @@ qemuSecuritySetImageLabel(virQEMUDriver *driver, goto cleanup; if (virSecurityManagerSetImageLabel(driver->securityManager, - vm->def, src, labelFlags) < 0) + vm->def, src, + cfg->sharedFilesystems, + labelFlags) < 0) goto cleanup; if (virSecurityManagerTransactionCommit(driver->securityManager, @@ -141,6 +148,7 @@ qemuSecurityRestoreImageLabel(virQEMUDriver *driver, bool backingChain) { qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); pid_t pid = -1; int ret = -1; virSecurityDomainImageLabelFlags labelFlags = 0; @@ -155,7 +163,9 @@ qemuSecurityRestoreImageLabel(virQEMUDriver *driver, goto cleanup; if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, src, labelFlags) < 0) + vm->def, src, + cfg->sharedFilesystems, + labelFlags) < 0) goto cleanup; if (virSecurityManagerTransactionCommit(driver->securityManager, diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index bf0c6bcb0d..f1b4283a70 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -538,6 +538,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath, * @privileged: whether we are running in privileged mode * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root) * @swtpm_group: The gid for the swtpm to run as + * @sharedFilesystems: list of filesystem to consider shared * @incomingMigration: whether we have an incoming migration * * Create the virCommand use for starting the emulator @@ -551,6 +552,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, bool privileged, uid_t swtpm_user, gid_t swtpm_group, + char *const *sharedFilesystems, bool incomingMigration) { g_autoptr(virCommand) cmd = NULL; @@ -568,7 +570,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, /* Do not create storage and run swtpm_setup on incoming migration over * shared storage */ - on_shared_storage = virFileIsSharedFS(tpm->data.emulator.storagepath) == 1; + on_shared_storage = virFileIsSharedFS(tpm->data.emulator.storagepath, sharedFilesystems) == 1; if (incomingMigration && on_shared_storage) create_storage = false; @@ -734,6 +736,7 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, /** * qemuTPMEmulatorCleanupHost: + * @driver: QEMU driver * @tpm: TPM definition * @flags: flags indicating whether to keep or remove TPM persistent state * @outgoingMigration: whether cleanup is due to an outgoing migration @@ -741,15 +744,18 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, * Clean up persistent storage for the swtpm. */ static void -qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm, +qemuTPMEmulatorCleanupHost(virQEMUDriver *driver, + virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, bool outgoingMigration) { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + /* Never remove the state in case of outgoing migration with shared * storage. */ if (outgoingMigration && - virFileIsSharedFS(tpm->data.emulator.storagepath) == 1) + virFileIsSharedFS(tpm->data.emulator.storagepath, cfg->sharedFilesystems) == 1) return; /* @@ -935,6 +941,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, driver->privileged, cfg->swtpm_user, cfg->swtpm_group, + cfg->sharedFilesystems, incomingMigration))) return -1; @@ -950,7 +957,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, virCommandSetErrorFD(cmd, &errfd); if (incomingMigration && - virFileIsSharedFS(tpm->data.emulator.storagepath) == 1) { + virFileIsSharedFS(tpm->data.emulator.storagepath, cfg->sharedFilesystems) == 1) { /* security labels must have been set up on source already */ setTPMStateLabel = false; } @@ -1010,7 +1017,8 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, bool -qemuTPMHasSharedStorage(virDomainDef *def) +qemuTPMHasSharedStorage(virDomainDef *def, + char *const *sharedFilesystems) { size_t i; @@ -1019,7 +1027,7 @@ qemuTPMHasSharedStorage(virDomainDef *def) switch (tpm->type) { case VIR_DOMAIN_TPM_TYPE_EMULATOR: - return virFileIsSharedFS(tpm->data.emulator.storagepath) == 1; + return virFileIsSharedFS(tpm->data.emulator.storagepath, sharedFilesystems) == 1; case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: case VIR_DOMAIN_TPM_TYPE_EXTERNAL: case VIR_DOMAIN_TPM_TYPE_LAST: @@ -1097,11 +1105,12 @@ qemuExtTPMPrepareHost(virQEMUDriver *driver, void -qemuExtTPMCleanupHost(virDomainTPMDef *tpm, +qemuExtTPMCleanupHost(virQEMUDriver *driver, + virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, bool outgoingMigration) { - qemuTPMEmulatorCleanupHost(tpm, flags, outgoingMigration); + qemuTPMEmulatorCleanupHost(driver, tpm, flags, outgoingMigration); } @@ -1133,7 +1142,7 @@ qemuExtTPMStop(virQEMUDriver *driver, return; qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); - if (outgoingMigration && qemuTPMHasSharedStorage(vm->def)) + if (outgoingMigration && qemuTPMHasSharedStorage(vm->def, cfg->sharedFilesystems)) restoreTPMStateLabel = false; if (qemuSecurityRestoreTPMLabels(driver, vm, restoreTPMStateLabel) < 0) diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 33ba5d2268..709e956fce 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -35,10 +35,11 @@ int qemuExtTPMPrepareHost(virQEMUDriver *driver, ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; -void qemuExtTPMCleanupHost(virDomainTPMDef *tpm, +void qemuExtTPMCleanupHost(virQEMUDriver *driver, + virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, bool outgoingMigration) - ATTRIBUTE_NONNULL(1); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuExtTPMStart(virQEMUDriver *driver, virDomainObj *vm, @@ -59,7 +60,8 @@ int qemuExtTPMSetupCgroup(virQEMUDriver *driver, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; -bool qemuTPMHasSharedStorage(virDomainDef *def) +bool qemuTPMHasSharedStorage(virDomainDef *def, + char *const *sharedFilesystems) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index c1dc859751..8746c96275 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -508,6 +508,7 @@ AppArmorReleaseSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED, static int AppArmorRestoreSecurityAllLabel(virSecurityManager *mgr G_GNUC_UNUSED, virDomainDef *def, + char *const *sharedFilesystems G_GNUC_UNUSED, bool migrated G_GNUC_UNUSED, bool chardevStdioLogd G_GNUC_UNUSED) { @@ -627,6 +628,7 @@ static int AppArmorRestoreSecurityImageLabel(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems G_GNUC_UNUSED, virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) { if (!virStorageSourceIsLocalStorage(src)) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 567be4bd23..376b364beb 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -864,6 +864,7 @@ virSecurityDACSetImageLabelInternal(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virStorageSource *parent, + char *const *sharedFilesystems G_GNUC_UNUSED, bool isChainTop) { virSecurityLabelDef *secdef; @@ -942,6 +943,7 @@ static int virSecurityDACSetImageLabel(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags) { virStorageSource *parent = src; @@ -950,7 +952,7 @@ virSecurityDACSetImageLabel(virSecurityManager *mgr, for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { const bool isChainTop = flags & VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP; - if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent, isChainTop) < 0) + if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent, sharedFilesystems, isChainTop) < 0) return -1; if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) @@ -966,6 +968,7 @@ static int virSecurityDACRestoreImageLabelInt(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, bool migrated) { virSecurityDACData *priv = virSecurityManagerGetPrivateData(mgr); @@ -1006,7 +1009,7 @@ virSecurityDACRestoreImageLabelInt(virSecurityManager *mgr, if (!src->path) return 0; - if ((rc = virFileIsSharedFS(src->path)) < 0) + if ((rc = virFileIsSharedFS(src->path, sharedFilesystems)) < 0) return -1; } @@ -1042,9 +1045,10 @@ static int virSecurityDACRestoreImageLabel(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) { - return virSecurityDACRestoreImageLabelInt(mgr, def, src, false); + return virSecurityDACRestoreImageLabelInt(mgr, def, src, sharedFilesystems, false); } @@ -1886,6 +1890,7 @@ virSecurityDACRestoreSysinfoLabel(virSecurityManager *mgr, static int virSecurityDACRestoreAllLabel(virSecurityManager *mgr, virDomainDef *def, + char *const *sharedFilesystems, bool migrated, bool chardevStdioLogd) { @@ -1911,6 +1916,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, if (virSecurityDACRestoreImageLabelInt(mgr, def, def->disks[i]->src, + sharedFilesystems, migrated) < 0) rc = -1; } @@ -1967,7 +1973,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, if (def->os.loader && def->os.loader->nvram) { if (virSecurityDACRestoreImageLabelInt(mgr, def, def->os.loader->nvram, - migrated) < 0) + sharedFilesystems, migrated) < 0) rc = -1; } @@ -2109,6 +2115,7 @@ virSecurityDACSetSysinfoLabel(virSecurityManager *mgr, static int virSecurityDACSetAllLabel(virSecurityManager *mgr, virDomainDef *def, + char *const *sharedFilesystems, const char *incomingPath G_GNUC_UNUSED, bool chardevStdioLogd, bool migrated G_GNUC_UNUSED) @@ -2134,6 +2141,7 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, if (virDomainDiskGetType(def->disks[i]) == VIR_STORAGE_TYPE_DIR) continue; if (virSecurityDACSetImageLabel(mgr, def, def->disks[i]->src, + sharedFilesystems, VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) return -1; @@ -2193,6 +2201,7 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, if (def->os.loader && def->os.loader->nvram) { if (virSecurityDACSetImageLabel(mgr, def, def->os.loader->nvram, + sharedFilesystems, VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) return -1; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index aa1fb2125d..ea990d7210 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -81,11 +81,13 @@ typedef int (*virSecurityDomainReleaseLabel) (virSecurityManager *mgr, virDomainDef *sec); typedef int (*virSecurityDomainSetAllLabel) (virSecurityManager *mgr, virDomainDef *sec, + char *const *sharedFilesystems, const char *incomingPath, bool chardevStdioLogd, bool migrated); typedef int (*virSecurityDomainRestoreAllLabel) (virSecurityManager *mgr, virDomainDef *def, + char *const *sharedFilesystems, bool migrated, bool chardevStdioLogd); typedef int (*virSecurityDomainGetProcessLabel) (virSecurityManager *mgr, @@ -115,10 +117,12 @@ typedef int (*virSecurityDomainSetHugepages) (virSecurityManager *mgr, typedef int (*virSecurityDomainSetImageLabel) (virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags); typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags); typedef int (*virSecurityDomainMoveImageMetadata) (virSecurityManager *mgr, pid_t pid, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 24f2f3d3dc..57de40ef65 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -404,6 +404,7 @@ virSecurityManagerGetPrivileged(virSecurityManager *mgr) * @mgr: security manager object * @vm: domain definition object * @src: disk source definition to operate on + * @sharedFilesystems: list of filesystem to consider shared * @flags: bitwise or of 'virSecurityDomainImageLabelFlags' * * Removes security label from @src according to @flags. @@ -414,6 +415,7 @@ int virSecurityManagerRestoreImageLabel(virSecurityManager *mgr, virDomainDef *vm, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags) { VIR_LOCK_GUARD lock = virObjectLockGuard(mgr); @@ -423,7 +425,7 @@ virSecurityManagerRestoreImageLabel(virSecurityManager *mgr, return -1; } - return mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src, flags); + return mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src, sharedFilesystems, flags); } @@ -512,6 +514,7 @@ virSecurityManagerClearSocketLabel(virSecurityManager *mgr, * @mgr: security manager object * @vm: domain definition object * @src: disk source definition to operate on + * @sharedFilesystems: list of filesystem to consider shared * @flags: bitwise or of 'virSecurityDomainImageLabelFlags' * * Labels a storage image with the configured security label according to @flags. @@ -522,6 +525,7 @@ int virSecurityManagerSetImageLabel(virSecurityManager *mgr, virDomainDef *vm, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags) { VIR_LOCK_GUARD lock = virObjectLockGuard(mgr); @@ -531,7 +535,8 @@ virSecurityManagerSetImageLabel(virSecurityManager *mgr, return -1; } - return mgr->drv->domainSetSecurityImageLabel(mgr, vm, src, flags); + return mgr->drv->domainSetSecurityImageLabel(mgr, vm, src, + sharedFilesystems, flags); } @@ -817,6 +822,7 @@ int virSecurityManagerCheckAllLabel(virSecurityManager *mgr, int virSecurityManagerSetAllLabel(virSecurityManager *mgr, virDomainDef *vm, + char *const *sharedFilesystems, const char *incomingPath, bool chardevStdioLogd, bool migrated) @@ -828,7 +834,8 @@ virSecurityManagerSetAllLabel(virSecurityManager *mgr, return -1; } - return mgr->drv->domainSetSecurityAllLabel(mgr, vm, incomingPath, + return mgr->drv->domainSetSecurityAllLabel(mgr, vm, sharedFilesystems, + incomingPath, chardevStdioLogd, migrated); } @@ -836,6 +843,7 @@ virSecurityManagerSetAllLabel(virSecurityManager *mgr, int virSecurityManagerRestoreAllLabel(virSecurityManager *mgr, virDomainDef *vm, + char *const *sharedFilesystems, bool migrated, bool chardevStdioLogd) { @@ -846,8 +854,8 @@ virSecurityManagerRestoreAllLabel(virSecurityManager *mgr, return -1; } - return mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated, - chardevStdioLogd); + return mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, sharedFilesystems, + migrated, chardevStdioLogd); } int @@ -1355,7 +1363,7 @@ virSecurityManagerMetadataLock(virSecurityManager *mgr G_GNUC_UNUSED, } #endif /* !WIN32 */ - if (virFileIsSharedFS(p)) { + if (virFileIsSharedFS(p, NULL)) { /* Probably a root squashed NFS. */ continue; } diff --git a/src/security/security_manager.h b/src/security/security_manager.h index a416af3215..da2ab7f584 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -130,11 +130,13 @@ int virSecurityManagerCheckAllLabel(virSecurityManager *mgr, virDomainDef *sec); int virSecurityManagerSetAllLabel(virSecurityManager *mgr, virDomainDef *sec, + char *const *sharedFilesystems, const char *incomingPath, bool chardevStdioLogd, bool migrated); int virSecurityManagerRestoreAllLabel(virSecurityManager *mgr, virDomainDef *def, + char *const *sharedFilesystems, bool migrated, bool chardevStdioLogd); int virSecurityManagerGetProcessLabel(virSecurityManager *mgr, @@ -170,10 +172,12 @@ typedef enum { int virSecurityManagerSetImageLabel(virSecurityManager *mgr, virDomainDef *vm, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags); int virSecurityManagerRestoreImageLabel(virSecurityManager *mgr, virDomainDef *vm, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags); int virSecurityManagerMoveImageMetadata(virSecurityManager *mgr, pid_t pid, diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 1413f43d57..f9c0d3cad1 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -117,6 +117,7 @@ virSecurityDomainReleaseLabelNop(virSecurityManager *mgr G_GNUC_UNUSED, static int virSecurityDomainSetAllLabelNop(virSecurityManager *mgr G_GNUC_UNUSED, virDomainDef *sec G_GNUC_UNUSED, + char *const *sharedFilesystems G_GNUC_UNUSED, const char *incomingPath G_GNUC_UNUSED, bool chardevStdioLogd G_GNUC_UNUSED, bool migrated G_GNUC_UNUSED) @@ -127,6 +128,7 @@ virSecurityDomainSetAllLabelNop(virSecurityManager *mgr G_GNUC_UNUSED, static int virSecurityDomainRestoreAllLabelNop(virSecurityManager *mgr G_GNUC_UNUSED, virDomainDef *vm G_GNUC_UNUSED, + char *const *sharedFilesystems G_GNUC_UNUSED, bool migrated G_GNUC_UNUSED, bool chardevStdioLogd G_GNUC_UNUSED) { @@ -191,6 +193,7 @@ static int virSecurityDomainRestoreImageLabelNop(virSecurityManager *mgr G_GNUC_UNUSED, virDomainDef *def G_GNUC_UNUSED, virStorageSource *src G_GNUC_UNUSED, + char *const *sharedFilesystems G_GNUC_UNUSED, virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) { return 0; @@ -200,6 +203,7 @@ static int virSecurityDomainSetImageLabelNop(virSecurityManager *mgr G_GNUC_UNUSED, virDomainDef *def G_GNUC_UNUSED, virStorageSource *src G_GNUC_UNUSED, + char *const *sharedFilesystems G_GNUC_UNUSED, virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) { return 0; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b49af26e49..a891ad5839 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1777,6 +1777,7 @@ static int virSecuritySELinuxRestoreImageLabelInt(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, bool migrated) { virSecurityLabelDef *seclabel; @@ -1833,7 +1834,7 @@ virSecuritySELinuxRestoreImageLabelInt(virSecurityManager *mgr, if (!src->path) return 0; - if ((rc = virFileIsSharedFS(src->path)) < 0) + if ((rc = virFileIsSharedFS(src->path, sharedFilesystems)) < 0) return -1; } @@ -1867,9 +1868,10 @@ static int virSecuritySELinuxRestoreImageLabel(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) { - return virSecuritySELinuxRestoreImageLabelInt(mgr, def, src, false); + return virSecuritySELinuxRestoreImageLabelInt(mgr, def, src, sharedFilesystems, false); } @@ -1878,6 +1880,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virStorageSource *parent, + char *const *sharedFilesystems G_GNUC_UNUSED, bool isChainTop) { virSecuritySELinuxData *data = virSecurityManagerGetPrivateData(mgr); @@ -1983,6 +1986,7 @@ static int virSecuritySELinuxSetImageLabel(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags) { virStorageSource *parent = src; @@ -1991,7 +1995,9 @@ virSecuritySELinuxSetImageLabel(virSecurityManager *mgr, for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { const bool isChainTop = flags & VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP; - if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent, isChainTop) < 0) + if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent, + sharedFilesystems, + isChainTop) < 0) return -1; if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) @@ -2819,6 +2825,7 @@ virSecuritySELinuxRestoreSysinfoLabel(virSecurityManager *mgr, static int virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr, virDomainDef *def, + char *const *sharedFilesystems, bool migrated, bool chardevStdioLogd) { @@ -2843,6 +2850,7 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr, virDomainDiskDef *disk = def->disks[i]; if (virSecuritySELinuxRestoreImageLabelInt(mgr, def, disk->src, + sharedFilesystems, migrated) < 0) rc = -1; } @@ -2889,6 +2897,7 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr, if (def->os.loader && def->os.loader->nvram) { if (virSecuritySELinuxRestoreImageLabelInt(mgr, def, def->os.loader->nvram, + sharedFilesystems, migrated) < 0) rc = -1; } @@ -3231,6 +3240,7 @@ virSecuritySELinuxSetSysinfoLabel(virSecurityManager *mgr, static int virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, virDomainDef *def, + char *const *sharedFilesystems, const char *incomingPath G_GNUC_UNUSED, bool chardevStdioLogd, bool migrated G_GNUC_UNUSED) @@ -3258,6 +3268,7 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, continue; } if (virSecuritySELinuxSetImageLabel(mgr, def, def->disks[i]->src, + sharedFilesystems, VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) return -1; @@ -3308,6 +3319,7 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, if (def->os.loader && def->os.loader->nvram) { if (virSecuritySELinuxSetImageLabel(mgr, def, def->os.loader->nvram, + sharedFilesystems, VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) return -1; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 369b5dd3a6..dc52df0bff 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -338,6 +338,7 @@ virSecurityStackRestoreHostdevLabel(virSecurityManager *mgr, static int virSecurityStackSetAllLabel(virSecurityManager *mgr, virDomainDef *vm, + char *const *sharedFilesystems, const char *incomingPath, bool chardevStdioLogd, bool migrated) @@ -347,8 +348,8 @@ virSecurityStackSetAllLabel(virSecurityManager *mgr, for (; item; item = item->next) { if (virSecurityManagerSetAllLabel(item->securityManager, vm, - incomingPath, chardevStdioLogd, - migrated) < 0) + sharedFilesystems, incomingPath, + chardevStdioLogd, migrated) < 0) goto rollback; } @@ -358,6 +359,7 @@ virSecurityStackSetAllLabel(virSecurityManager *mgr, for (item = item->prev; item; item = item->prev) { if (virSecurityManagerRestoreAllLabel(item->securityManager, vm, + sharedFilesystems, migrated, chardevStdioLogd) < 0) { VIR_WARN("Unable to restore all labels after failed set label call " @@ -374,6 +376,7 @@ virSecurityStackSetAllLabel(virSecurityManager *mgr, static int virSecurityStackRestoreAllLabel(virSecurityManager *mgr, virDomainDef *vm, + char *const *sharedFilesystems, bool migrated, bool chardevStdioLogd) { @@ -383,6 +386,7 @@ virSecurityStackRestoreAllLabel(virSecurityManager *mgr, for (; item; item = item->next) { if (virSecurityManagerRestoreAllLabel(item->securityManager, vm, + sharedFilesystems, migrated, chardevStdioLogd) < 0) rc = -1; } @@ -640,6 +644,7 @@ static int virSecurityStackSetImageLabel(virSecurityManager *mgr, virDomainDef *vm, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags) { virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr); @@ -647,7 +652,7 @@ virSecurityStackSetImageLabel(virSecurityManager *mgr, for (; item; item = item->next) { if (virSecurityManagerSetImageLabel(item->securityManager, vm, src, - flags) < 0) + sharedFilesystems, flags) < 0) goto rollback; } @@ -658,6 +663,7 @@ virSecurityStackSetImageLabel(virSecurityManager *mgr, if (virSecurityManagerRestoreImageLabel(item->securityManager, vm, src, + sharedFilesystems, flags) < 0) { VIR_WARN("Unable to restore image label after failed set label " "call virDriver=%s driver=%s domain=%s src=%p (path=%s) " @@ -674,6 +680,7 @@ static int virSecurityStackRestoreImageLabel(virSecurityManager *mgr, virDomainDef *vm, virStorageSource *src, + char *const *sharedFilesystems, virSecurityDomainImageLabelFlags flags) { virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr); @@ -682,7 +689,8 @@ virSecurityStackRestoreImageLabel(virSecurityManager *mgr, for (; item; item = item->next) { if (virSecurityManagerRestoreImageLabel(item->securityManager, - vm, src, flags) < 0) + vm, src, sharedFilesystems, + flags) < 0) rc = -1; } diff --git a/src/util/virfile.c b/src/util/virfile.c index deaf4555fd..a6a7de9829 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2598,7 +2598,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, /* On Linux we can also verify the FS-type of the * directory. (this is a NOP on other platforms). */ - if (virFileIsSharedFS(path) <= 0) + if (virFileIsSharedFS(path, NULL) <= 0) goto error; } @@ -3795,7 +3795,8 @@ virFileGetDefaultHugepage(virHugeTLBFS *fs, return NULL; } -int virFileIsSharedFS(const char *path) +int virFileIsSharedFS(const char *path, + char *const *overrides G_GNUC_UNUSED) { return virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS | diff --git a/src/util/virfile.h b/src/util/virfile.h index 56fe309bce..3fdd7f526c 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -235,7 +235,8 @@ enum { }; int virFileIsSharedFSType(const char *path, unsigned int fstypes) ATTRIBUTE_NONNULL(1); -int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1); +int virFileIsSharedFS(const char *path, + char *const *overrides) ATTRIBUTE_NONNULL(1); int virFileIsClusterFS(const char *path) ATTRIBUTE_NONNULL(1); int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1); int virFileIsCDROM(const char *path) diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 04bffe4356..f23772dcde 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -270,7 +270,7 @@ testSELinuxLabeling(const void *opaque) if (!(def = testSELinuxLoadDef(testname))) goto cleanup; - if (virSecurityManagerSetAllLabel(mgr, def, NULL, false, false) < 0) + if (virSecurityManagerSetAllLabel(mgr, def, NULL, NULL, false, false) < 0) goto cleanup; if (testSELinuxCheckLabels(files, nfiles) < 0) diff --git a/tests/virfiletest.c b/tests/virfiletest.c index 9fbfc37e56..e05925a321 100644 --- a/tests/virfiletest.c +++ b/tests/virfiletest.c @@ -313,7 +313,7 @@ testFileIsSharedFSType(const void *opaque G_GNUC_UNUSED) goto cleanup; } - actual = virFileIsSharedFS(data->filename); + actual = virFileIsSharedFS(data->filename, NULL); if (actual != data->expected) { fprintf(stderr, "Unexpected FS type. Expected %d got %d\n", -- 2.44.0

On Wed, Mar 20, 2024 at 01:10:00PM +0100, Peter Krempa wrote: No reason at all, it was just an oversight O:-) Thanks a lot for catching that! I've already fixed it locally. -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Mar 20, 2024 at 10:19:13 +0100, Andrea Bolognani wrote: This is g_strv_contains in disguise. Return 0 directly. Same here, return success directly rather than having the reader look what's happening. Here too. And return failure/negative answer here also directly. Also consider returning boolean as it's returning just 1 and 0, which would make the first question I had (What do the return values mean?!?!) auto-answered.

On 3/20/24 05:19, Andrea Bolognani wrote: Is NFS the only shared filesystem to consider or is NFS only an example? > + which mounts it, is allowed in both directions. Without it, libvirt would > + block migration from the host that is accessing the data locally. > + > * **Improvements** > > * **Bug fixes**