[PATCH v2 00/10] qemu: Fix image labels lost after migration with shared fs

This is v2 of: https://listman.redhat.com/archives/libvir-list/2021-August/msg00698.html When migrating using shared fs, the dst host will remember the image labels again before launching the VM and the src host will remove the image labels after migration. It leads to that the image labels lost after migration. Patch 8-10 are the real fix patches. v1 -> v2: 1. fix check of virFileIsSharedFS in patch10 (patch9 in v1) [Michal] 2. add implementation of SELinux (patch5, 7, 8) [Michal] Peng Liang (10): security: add virSecurityUpdateTimestampIfexists security: add virSecurityManagerUpdateImageLabel security: implement domainUpdateSecurityImageLabel for stack security: implement domainUpdateSecurityImageLabel for DAC security: implement domainUpdateSecurityImageLabel for SELinux qemu: add qemuSecurityUpdateImageLabel security: rename 2 functions in DAC and SELinux security: don't remember image labels when migrating with shared fs migration: don't remove image labels after migration migration: update image labels in dst after migration src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 12 +++++++ src/qemu/qemu_process.c | 5 ++- src/qemu/qemu_security.c | 10 ++++++ src/qemu/qemu_security.h | 5 +++ src/security/security_dac.c | 58 ++++++++++++++++++++++++-------- src/security/security_driver.h | 5 +++ src/security/security_manager.c | 29 ++++++++++++++++ src/security/security_manager.h | 5 +++ src/security/security_selinux.c | 59 +++++++++++++++++++++++++-------- src/security/security_stack.c | 20 +++++++++++ src/security/security_util.c | 32 ++++++++++++++++++ src/security/security_util.h | 4 +++ 13 files changed, 218 insertions(+), 27 deletions(-) -- 2.31.1

Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/security/security_util.c | 32 ++++++++++++++++++++++++++++++++ src/security/security_util.h | 4 ++++ 2 files changed, 36 insertions(+) diff --git a/src/security/security_util.c b/src/security/security_util.c index 26a7861e2935..07960b577e1a 100644 --- a/src/security/security_util.c +++ b/src/security/security_util.c @@ -227,6 +227,38 @@ virSecurityAddTimestamp(const char *name, } +/** + * virSecurityUpdateTimestampIfexists: + * @name: security driver name + * @path: file name + * + * Update timestamp of @path for given security driver (@name) if the timestamp + * of @path exists. + * + * Returns: 0 on success, + * 1 if timestamp of @path doesn't exist, + * -1 otherwise. + */ +int +virSecurityUpdateTimestampIfexists(const char *name, + const char *path) +{ + g_autofree char *timestamp_name = NULL; + g_autofree char *timestamp_value = NULL; + g_autofree char *old_value = NULL; + + if (!(timestamp_value = virSecurityGetTimestamp()) || + !(timestamp_name = virSecurityGetTimestampAttrName(name))) + return -1; + + if (virFileGetXAttrQuiet(path, timestamp_name, &old_value) < 0) + return 1; + + + return virFileSetXAttr(path, timestamp_name, timestamp_value); +} + + static int virSecurityRemoveTimestamp(const char *name, const char *path) diff --git a/src/security/security_util.h b/src/security/security_util.h index 7af6f009e2ca..b66541fd92c5 100644 --- a/src/security/security_util.h +++ b/src/security/security_util.h @@ -33,5 +33,9 @@ virSecurityMoveRememberedLabel(const char *name, const char *src, const char *dst); +int +virSecurityUpdateTimestampIfexists(const char *name, + const char *path); + bool virSecurityXATTRNamespaceDefined(void); -- 2.31.1

After migration, some labels of images need to be updated. So add virSecurityManagerUpdateImageLabel to do it. Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/libvirt_private.syms | 1 + src/security/security_driver.h | 5 +++++ src/security/security_manager.c | 29 +++++++++++++++++++++++++++++ src/security/security_manager.h | 5 +++++ 4 files changed, 40 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fd0eea0777e2..ed750de262a1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1720,6 +1720,7 @@ virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; virSecurityManagerTransactionStart; +virSecurityManagerUpdateImageLabel; virSecurityManagerVerify; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index a1fc23be383f..7c1e9a5a8596 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -123,6 +123,10 @@ typedef int (*virSecurityDomainMoveImageMetadata) (virSecurityManager *mgr, pid_t pid, virStorageSource *src, virStorageSource *dst); +typedef int (*virSecurityDomainUpdateImageLabel) (virSecurityManager *mgr, + virDomainDef *def, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags); typedef int (*virSecurityDomainSetMemoryLabel) (virSecurityManager *mgr, virDomainDef *def, virDomainMemoryDef *mem); @@ -186,6 +190,7 @@ struct _virSecurityDriver { virSecurityDomainSetImageLabel domainSetSecurityImageLabel; virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel; virSecurityDomainMoveImageMetadata domainMoveImageMetadata; + virSecurityDomainUpdateImageLabel domainUpdateSecurityImageLabel; virSecurityDomainSetMemoryLabel domainSetSecurityMemoryLabel; virSecurityDomainRestoreMemoryLabel domainRestoreSecurityMemoryLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index d8a03a19cb8b..bbdecbf41606 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager *mgr, } +/** + * virSecurityManagerUpdateImageLabel: + * @mgr: security manager object + * @vm: domain definition object + * @src: disk source definition to operate on + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags' + * + * Update security label from @src according to @flags. + * + * Returns: 0 on success, -1 on error. + */ +int +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr, + virDomainDef *vm, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags) +{ + if (mgr->drv->domainUpdateSecurityImageLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags); + virObjectUnlock(mgr); + return ret; + } + + return 0; +} + + int virSecurityManagerSetDaemonSocketLabel(virSecurityManager *mgr, virDomainDef *vm) diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 59020b147527..365f18e2dcfd 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -175,6 +175,11 @@ int virSecurityManagerMoveImageMetadata(virSecurityManager *mgr, pid_t pid, virStorageSource *src, virStorageSource *dst); +int +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr, + virDomainDef *vm, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags); int virSecurityManagerSetMemoryLabel(virSecurityManager *mgr, virDomainDef *vm, -- 2.31.1

Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/security/security_stack.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 3c2239910aa5..7712cac3b542 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -706,6 +706,25 @@ virSecurityStackMoveImageMetadata(virSecurityManager *mgr, return rc; } +static int +virSecurityStackUpdateImageLabel(virSecurityManager *mgr, + virDomainDef *vm, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags) +{ + virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItem *item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerUpdateImageLabel(item->securityManager, + vm, src, flags) < 0) + rc = -1; + } + + return rc; +} + static int virSecurityStackSetMemoryLabel(virSecurityManager *mgr, virDomainDef *vm, @@ -1033,6 +1052,7 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityImageLabel = virSecurityStackSetImageLabel, .domainRestoreSecurityImageLabel = virSecurityStackRestoreImageLabel, .domainMoveImageMetadata = virSecurityStackMoveImageMetadata, + .domainUpdateSecurityImageLabel = virSecurityStackUpdateImageLabel, .domainSetSecurityMemoryLabel = virSecurityStackSetMemoryLabel, .domainRestoreSecurityMemoryLabel = virSecurityStackRestoreMemoryLabel, -- 2.31.1

Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/security/security_dac.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 1733d63410b3..d1e1552bb683 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1132,6 +1132,23 @@ virSecurityDACMoveImageMetadata(virSecurityManager *mgr, } +static int +virSecurityDACUpdateImageLabel(virSecurityManager *mgr G_GNUC_UNUSED, + virDomainDef *def G_GNUC_UNUSED, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) +{ + virStorageSource *n; + + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (virSecurityUpdateTimestampIfexists(SECURITY_DAC_NAME, src->path) < 0) + return -1; + } + + return 0; +} + + static int virSecurityDACSetHostdevLabelHelper(const char *file, bool remember, @@ -2539,6 +2556,7 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityImageLabel = virSecurityDACSetImageLabel, .domainRestoreSecurityImageLabel = virSecurityDACRestoreImageLabel, .domainMoveImageMetadata = virSecurityDACMoveImageMetadata, + .domainUpdateSecurityImageLabel = virSecurityDACUpdateImageLabel, .domainSetSecurityMemoryLabel = virSecurityDACSetMemoryLabel, .domainRestoreSecurityMemoryLabel = virSecurityDACRestoreMemoryLabel, -- 2.31.1

Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/security/security_selinux.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index cc7245332980..5c491fc131ea 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1996,6 +1996,23 @@ virSecuritySELinuxMoveImageMetadata(virSecurityManager *mgr, } +static int +virSecuritySELinuxUpdateImageLabel(virSecurityManager *mgr G_GNUC_UNUSED, + virDomainDef *def G_GNUC_UNUSED, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED) +{ + virStorageSource *n; + + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (virSecurityUpdateTimestampIfexists(SECURITY_SELINUX_NAME, src->path) < 0) + return -1; + } + + return 0; +} + + static int virSecuritySELinuxSetHostdevLabelHelper(const char *file, bool remember, @@ -3587,6 +3604,7 @@ virSecurityDriver virSecurityDriverSELinux = { .domainSetSecurityImageLabel = virSecuritySELinuxSetImageLabel, .domainRestoreSecurityImageLabel = virSecuritySELinuxRestoreImageLabel, .domainMoveImageMetadata = virSecuritySELinuxMoveImageMetadata, + .domainUpdateSecurityImageLabel = virSecuritySELinuxUpdateImageLabel, .domainSetSecurityMemoryLabel = virSecuritySELinuxSetMemoryLabel, .domainRestoreSecurityMemoryLabel = virSecuritySELinuxRestoreMemoryLabel, -- 2.31.1

Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/qemu/qemu_security.c | 10 ++++++++++ src/qemu/qemu_security.h | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 19d957dd4b96..d8f2049e0d46 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -188,6 +188,16 @@ qemuSecurityMoveImageMetadata(virQEMUDriver *driver, } +int +qemuSecurityUpdateImageLabel(virQEMUDriver *driver, + virDomainObj *vm, + virStorageSource *src) +{ + return virSecurityManagerUpdateImageLabel(driver->securityManager, vm->def, + src, 0); +} + + int qemuSecuritySetHostdevLabel(virQEMUDriver *driver, virDomainObj *vm, diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index 8b26ea3f9922..ea6e1404936e 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -49,6 +49,11 @@ int qemuSecurityMoveImageMetadata(virQEMUDriver *driver, virStorageSource *src, virStorageSource *dst); +int +qemuSecurityUpdateImageLabel(virQEMUDriver *driver, + virDomainObj *vm, + virStorageSource *src); + int qemuSecuritySetHostdevLabel(virQEMUDriver *driver, virDomainObj *vm, virDomainHostdevDef *hostdev); -- 2.31.1

virSecurity{DAC,SELinux}SetImageLabelInt will be added in the next patch, so rename virSecurity{DAC,SELinux}SetImageLabelInternal to virSecurity{DAC,SELinux}SetImageLabelSingle to avoid confusion and keep consistent with virSecurity{DAC,SELinux}RestoreImageLabelInt and virSecurity{DAC,SELinux}RestoreImageLabelSingle. Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/security/security_dac.c | 12 ++++++------ src/security/security_selinux.c | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index d1e1552bb683..2c0e12a6f810 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -863,11 +863,11 @@ virSecurityDACRestoreFileLabel(virSecurityManager *mgr, static int -virSecurityDACSetImageLabelInternal(virSecurityManager *mgr, - virDomainDef *def, - virStorageSource *src, - virStorageSource *parent, - bool isChainTop) +virSecurityDACSetImageLabelSingle(virSecurityManager *mgr, + virDomainDef *def, + virStorageSource *src, + virStorageSource *parent, + bool isChainTop) { virSecurityLabelDef *secdef; virSecurityDeviceLabelDef *disk_seclabel; @@ -949,7 +949,7 @@ virSecurityDACSetImageLabelRelative(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 (virSecurityDACSetImageLabelSingle(mgr, def, n, parent, isChainTop) < 0) return -1; if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5c491fc131ea..f6fa412de89a 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1805,11 +1805,11 @@ virSecuritySELinuxRestoreImageLabel(virSecurityManager *mgr, static int -virSecuritySELinuxSetImageLabelInternal(virSecurityManager *mgr, - virDomainDef *def, - virStorageSource *src, - virStorageSource *parent, - bool isChainTop) +virSecuritySELinuxSetImageLabelSingle(virSecurityManager *mgr, + virDomainDef *def, + virStorageSource *src, + virStorageSource *parent, + bool isChainTop) { virSecuritySELinuxData *data = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDef *secdef; @@ -1912,7 +1912,7 @@ virSecuritySELinuxSetImageLabelRelative(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 (virSecuritySELinuxSetImageLabelSingle(mgr, def, n, parent, isChainTop) < 0) return -1; if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) -- 2.31.1

When migrating with shared fs, the image labels has been remembered in the src host. If the dst host trys to remember image labels again, then the origin labels remembered in the src host will lost. Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/security/security_dac.c | 32 +++++++++++++++++++++++--------- src/security/security_selinux.c | 33 ++++++++++++++++++++++++--------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 2c0e12a6f810..65cdf348e4c1 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -867,7 +867,8 @@ virSecurityDACSetImageLabelSingle(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virStorageSource *parent, - bool isChainTop) + bool isChainTop, + bool migrated) { virSecurityLabelDef *secdef; virSecurityDeviceLabelDef *disk_seclabel; @@ -931,7 +932,8 @@ virSecurityDACSetImageLabelSingle(virSecurityManager *mgr, * but the top layer, or read only image, or disk explicitly * marked as shared. */ - remember = isChainTop && !src->readonly && !src->shared; + remember = isChainTop && !src->readonly && !src->shared && + !(migrated && virFileIsSharedFS(src->path) > 0); return virSecurityDACSetOwnership(mgr, src, NULL, user, group, remember); } @@ -942,14 +944,15 @@ virSecurityDACSetImageLabelRelative(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virStorageSource *parent, - virSecurityDomainImageLabelFlags flags) + virSecurityDomainImageLabelFlags flags, + bool migrated) { virStorageSource *n; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { const bool isChainTop = flags & VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP; - if (virSecurityDACSetImageLabelSingle(mgr, def, n, parent, isChainTop) < 0) + if (virSecurityDACSetImageLabelSingle(mgr, def, n, parent, isChainTop, migrated) < 0) return -1; if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) @@ -961,13 +964,23 @@ virSecurityDACSetImageLabelRelative(virSecurityManager *mgr, return 0; } +static int +virSecurityDACSetImageLabelInt(virSecurityManager *mgr, + virDomainDef *def, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags, + bool migrated) +{ + return virSecurityDACSetImageLabelRelative(mgr, def, src, src, flags, migrated); +} + static int virSecurityDACSetImageLabel(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virSecurityDomainImageLabelFlags flags) { - return virSecurityDACSetImageLabelRelative(mgr, def, src, src, flags); + return virSecurityDACSetImageLabelInt(mgr, def, src, flags, false); } static int @@ -2118,7 +2131,7 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, virDomainDef *def, const char *incomingPath G_GNUC_UNUSED, bool chardevStdioLogd, - bool migrated G_GNUC_UNUSED) + bool migrated) { virSecurityDACData *priv = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDef *secdef; @@ -2140,9 +2153,10 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, /* XXX fixme - we need to recursively label the entire tree :-( */ if (virDomainDiskGetType(def->disks[i]) == VIR_STORAGE_TYPE_DIR) continue; - if (virSecurityDACSetImageLabel(mgr, def, def->disks[i]->src, - VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | - VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) + if (virSecurityDACSetImageLabelInt(mgr, def, def->disks[i]->src, + VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | + VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP, + migrated) < 0) return -1; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index f6fa412de89a..78d0e610f68c 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1809,7 +1809,8 @@ virSecuritySELinuxSetImageLabelSingle(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virStorageSource *parent, - bool isChainTop) + bool isChainTop, + bool migrated) { virSecuritySELinuxData *data = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDef *secdef; @@ -1840,7 +1841,8 @@ virSecuritySELinuxSetImageLabelSingle(virSecurityManager *mgr, * but the top layer, or read only image, or disk explicitly * marked as shared. */ - remember = isChainTop && !src->readonly && !src->shared; + remember = isChainTop && !src->readonly && !src->shared && + !(migrated && virFileIsSharedFS(src->path) > 0); disk_seclabel = virStorageSourceGetSecurityLabelDef(src, SECURITY_SELINUX_NAME); @@ -1905,14 +1907,15 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virStorageSource *parent, - virSecurityDomainImageLabelFlags flags) + virSecurityDomainImageLabelFlags flags, + bool migrated) { virStorageSource *n; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { const bool isChainTop = flags & VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP; - if (virSecuritySELinuxSetImageLabelSingle(mgr, def, n, parent, isChainTop) < 0) + if (virSecuritySELinuxSetImageLabelSingle(mgr, def, n, parent, isChainTop, migrated) < 0) return -1; if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) @@ -1925,13 +1928,24 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManager *mgr, } +static int +virSecuritySELinuxSetImageLabelInt(virSecurityManager *mgr, + virDomainDef *def, + virStorageSource *src, + virSecurityDomainImageLabelFlags flags, + bool migrated) +{ + return virSecuritySELinuxSetImageLabelRelative(mgr, def, src, src, flags, migrated); +} + + static int virSecuritySELinuxSetImageLabel(virSecurityManager *mgr, virDomainDef *def, virStorageSource *src, virSecurityDomainImageLabelFlags flags) { - return virSecuritySELinuxSetImageLabelRelative(mgr, def, src, src, flags); + return virSecuritySELinuxSetImageLabelInt(mgr, def, src, flags, false); } struct virSecuritySELinuxMoveImageMetadataData { @@ -3156,7 +3170,7 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, virDomainDef *def, const char *incomingPath G_GNUC_UNUSED, bool chardevStdioLogd, - bool migrated G_GNUC_UNUSED) + bool migrated) { size_t i; virSecuritySELinuxData *data = virSecurityManagerGetPrivateData(mgr); @@ -3180,9 +3194,10 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, def->disks[i]->dst); continue; } - if (virSecuritySELinuxSetImageLabel(mgr, def, def->disks[i]->src, - VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | - VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) + if (virSecuritySELinuxSetImageLabelInt(mgr, def, def->disks[i]->src, + VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | + VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP, + migrated) < 0) return -1; } /* XXX fixme process def->fss if relabel == true */ -- 2.31.1

After migration, the image labels will be removed in the src host (on success) or the dst host (on failure). However, if we migrate using shared fs and remove image labels after migration in one host, the image labels will also lost in another host, which leads to that the ownership of the image will be restore to root:root instead of the origin ownership when shutting down the VM. Hence, don't remove image labels after migration with shared fs. Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/qemu/qemu_process.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1d0165af6daa..a6e64fcba8ba 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8173,7 +8173,10 @@ void qemuProcessStop(virQEMUDriver *driver, qemuHostdevReAttachOneNVMeDisk(driver, vm->def->name, disk->mirror); } - qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src); + if ((reason != VIR_DOMAIN_SHUTOFF_MIGRATED && + !(flags & VIR_QEMU_PROCESS_STOP_MIGRATED)) || + virFileIsSharedFS(disk->src->path) <= 0) + qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src); /* for now transient disks are forbidden with migration so they * can be handled here */ -- 2.31.1

Bacause the timestamp XATTR (the uptime of the host) is used to validate the remembered labels, it need to update after migration. Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/qemu/qemu_migration.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index dd226ea4bcb5..b82762aa0ffd 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5619,6 +5619,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver, qemuDomainJobInfo *jobInfo = NULL; bool inPostCopy = false; bool doKill = true; + size_t i; VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d", @@ -5826,6 +5827,17 @@ qemuMigrationDstFinish(virQEMUDriver *driver, /* Guest is successfully running, so cancel previous auto destroy */ qemuProcessAutoDestroyRemove(driver, vm); + for (i = 0; i < vm->def->ndisks; i++) { + virStorageSource *src = vm->def->disks[i]->src; + + if (!virStorageSourceIsLocalStorage(src) || !src->path || + virFileIsSharedFS(src->path) <= 0) + continue; + + if (qemuSecurityUpdateImageLabel(driver, vm, src) < 0) + VIR_WARN("Failed to update security label for %s", src->path); + } + endjob: if (!dom && !(flags & VIR_MIGRATE_OFFLINE) && -- 2.31.1
participants (1)
-
Peng Liang