Rename them to comply with the naming policy. --- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/util/virstoragefile.c | 6 +++--- src/util/virstoragefile.h | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a793b4c..b3f73f9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1904,8 +1904,8 @@ virStorageNetHostTransportTypeFromString; virStorageNetHostTransportTypeToString; virStorageNetProtocolTypeToString; virStorageSourceAuthClear; +virStorageSourceBackingStoreClear; virStorageSourceClear; -virStorageSourceClearBackingStore; virStorageSourceFree; virStorageSourceGetActualType; virStorageSourceGetSecurityLabelDef; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2e55c99..f3f42be 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2437,7 +2437,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (disk->src->backingStore) { if (force) - virStorageSourceClearBackingStore(disk->src); + virStorageSourceBackingStoreClear(disk->src); else goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 22699c1..41d7bb6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12868,7 +12868,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, * recompute it. Better would be storing the chain ourselves rather than * reprobing, but this requires modifying domain_conf and our XML to fully * track the chain across libvirtd restarts. */ - virStorageSourceClearBackingStore(disk->src); + virStorageSourceBackingStoreClear(disk->src); if (virStorageFileInit(snap->src) < 0) goto cleanup; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 6a57327..0c50de1 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1551,14 +1551,14 @@ virStorageSourceGetActualType(virStorageSourcePtr def) /** - * virStorageSourceClearBackingStore: + * virStorageSourceBackingStoreClear: * * @src: disk source to clear * * Clears information about backing store of the current storage file. */ void -virStorageSourceClearBackingStore(virStorageSourcePtr def) +virStorageSourceBackingStoreClear(virStorageSourcePtr def) { if (!def) return; @@ -1599,7 +1599,7 @@ virStorageSourceClear(virStorageSourcePtr def) virStorageNetHostDefFree(def->nhosts, def->hosts); virStorageSourceAuthClear(def); - virStorageSourceClearBackingStore(def); + virStorageSourceBackingStoreClear(def); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index f98a763..48c7e02 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -321,7 +321,7 @@ void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def); void virStorageSourceClear(virStorageSourcePtr def); int virStorageSourceGetActualType(virStorageSourcePtr def); void virStorageSourceFree(virStorageSourcePtr def); -void virStorageSourceClearBackingStore(virStorageSourcePtr def); +void virStorageSourceBackingStoreClear(virStorageSourcePtr def); virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent); typedef int -- 1.9.3

I'm going to add functions that will deal with individual image files rather than whole disks. Rename the security function to make room for the new one. --- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 24 ++++++++++++------------ src/security/security_apparmor.c | 8 ++++---- src/security/security_dac.c | 8 ++++---- src/security/security_driver.h | 8 ++++---- src/security/security_manager.c | 10 +++++----- src/security/security_manager.h | 6 +++--- src/security/security_nop.c | 8 ++++---- src/security/security_selinux.c | 8 ++++---- src/security/security_stack.c | 10 +++++----- 11 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b3f73f9..1e1dd84 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -911,8 +911,8 @@ virSecurityManagerPreFork; virSecurityManagerReleaseLabel; virSecurityManagerReserveLabel; virSecurityManagerRestoreAllLabel; +virSecurityManagerRestoreDiskLabel; virSecurityManagerRestoreHostdevLabel; -virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreSavedStateLabel; virSecurityManagerSetAllLabel; virSecurityManagerSetChildProcessLabel; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 41d7bb6..ce57542 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12087,8 +12087,8 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, disk->readonly = mode == VIR_DISK_CHAIN_READ_ONLY; if (mode == VIR_DISK_CHAIN_NO_ACCESS) { - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, disk) < 0) + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", disk->src->path); if (qemuTeardownDiskCgroup(vm, disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9cd6a3e..5e8aa4e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -161,8 +161,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (ret < 0) goto error; - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, origdisk) < 0) + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, origdisk) < 0) VIR_WARN("Unable to restore security label on ejected image %s", virDomainDiskGetSource(origdisk)); @@ -182,8 +182,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, return ret; error: - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, disk) < 0) + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, disk) < 0) VIR_WARN("Unable to restore security label on new media %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) @@ -347,8 +347,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, &disk->info, src); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, disk) < 0) + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) @@ -597,8 +597,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, return ret; error: - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, disk) < 0) + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) @@ -691,8 +691,8 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn, return ret; error: - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, disk) < 0) + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) @@ -2504,8 +2504,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainReleaseDeviceAddress(vm, &disk->info, src); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm->def, disk) < 0) + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); if (qemuTeardownDiskCgroup(vm, disk) < 0) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index c27ab47..b4cbc61 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -684,9 +684,9 @@ AppArmorClearSecuritySocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, /* Called when hotplugging */ static int -AppArmorRestoreSecurityImageLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def, - virDomainDiskDefPtr disk) +AppArmorRestoreSecurityDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainDiskDefPtr disk) { if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) return 0; @@ -973,7 +973,7 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSecurityVerify = AppArmorSecurityVerify, .domainSetSecurityDiskLabel = AppArmorSetSecurityDiskLabel, - .domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel, + .domainRestoreSecurityDiskLabel = AppArmorRestoreSecurityDiskLabel, .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = AppArmorSetSecuritySocketLabel, diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9760e6f..639f9b0 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -410,9 +410,9 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, static int -virSecurityDACRestoreSecurityImageLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def, - virDomainDiskDefPtr disk) +virSecurityDACRestoreSecurityDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainDiskDefPtr disk) { return virSecurityDACRestoreSecurityImageLabelInt(mgr, def, disk, false); } @@ -1274,7 +1274,7 @@ virSecurityDriver virSecurityDriverDAC = { .domainSecurityVerify = virSecurityDACVerify, .domainSetSecurityDiskLabel = virSecurityDACSetSecurityDiskLabel, - .domainRestoreSecurityImageLabel = virSecurityDACRestoreSecurityImageLabel, + .domainRestoreSecurityDiskLabel = virSecurityDACRestoreSecurityDiskLabel, .domainSetSecurityDaemonSocketLabel = virSecurityDACSetDaemonSocketLabel, .domainSetSecuritySocketLabel = virSecurityDACSetSocketLabel, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 6a17a8e..05d612a 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -51,9 +51,9 @@ typedef const char *(*virSecurityDriverGetBaseLabel) (virSecurityManagerPtr mgr, typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr); -typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, - virDomainDefPtr def, - virDomainDiskDefPtr disk); +typedef int (*virSecurityDomainRestoreDiskLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainDiskDefPtr disk); typedef int (*virSecurityDomainSetDaemonSocketLabel)(virSecurityManagerPtr mgr, virDomainDefPtr vm); typedef int (*virSecurityDomainSetSocketLabel) (virSecurityManagerPtr mgr, @@ -128,7 +128,7 @@ struct _virSecurityDriver { virSecurityDomainSecurityVerify domainSecurityVerify; virSecurityDomainSetDiskLabel domainSetSecurityDiskLabel; - virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel; + virSecurityDomainRestoreDiskLabel domainRestoreSecurityDiskLabel; virSecurityDomainSetDaemonSocketLabel domainSetSecurityDaemonSocketLabel; virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index f0e3ee1..d57bab9 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -306,14 +306,14 @@ bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) return mgr->requireConfined; } -int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - virDomainDiskDefPtr disk) +int virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainDiskDefPtr disk) { - if (mgr->drv->domainRestoreSecurityImageLabel) { + if (mgr->drv->domainRestoreSecurityDiskLabel) { int ret; virObjectLock(mgr); - ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk); + ret = mgr->drv->domainRestoreSecurityDiskLabel(mgr, vm, disk); virObjectUnlock(mgr); return ret; } diff --git a/src/security/security_manager.h b/src/security/security_manager.h index f083b3a..307e1c2 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -61,9 +61,9 @@ bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr); bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr); -int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def, - virDomainDiskDefPtr disk); +int virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainDiskDefPtr disk); int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm); int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 7feeda6..ac9ceae 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -50,9 +50,9 @@ static const char * virSecurityDriverGetDOINop(virSecurityManagerPtr mgr ATTRIBU return "0"; } -static int virSecurityDomainRestoreImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) +static int virSecurityDomainRestoreDiskLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) { return 0; } @@ -207,7 +207,7 @@ virSecurityDriver virSecurityDriverNop = { .domainSecurityVerify = virSecurityDomainVerifyNop, .domainSetSecurityDiskLabel = virSecurityDomainSetDiskLabelNop, - .domainRestoreSecurityImageLabel = virSecurityDomainRestoreImageLabelNop, + .domainRestoreSecurityDiskLabel = virSecurityDomainRestoreDiskLabelNop, .domainSetSecurityDaemonSocketLabel = virSecurityDomainSetDaemonSocketLabelNop, .domainSetSecuritySocketLabel = virSecurityDomainSetSocketLabelNop, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a4c13a1..572f8a1 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1182,9 +1182,9 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, static int -virSecuritySELinuxRestoreSecurityImageLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def, - virDomainDiskDefPtr disk) +virSecuritySELinuxRestoreSecurityDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainDiskDefPtr disk) { return virSecuritySELinuxRestoreSecurityImageLabelInt(mgr, def, disk, false); } @@ -2427,7 +2427,7 @@ virSecurityDriver virSecurityDriverSELinux = { .domainSecurityVerify = virSecuritySELinuxSecurityVerify, .domainSetSecurityDiskLabel = virSecuritySELinuxSetSecurityDiskLabel, - .domainRestoreSecurityImageLabel = virSecuritySELinuxRestoreSecurityImageLabel, + .domainRestoreSecurityDiskLabel = virSecuritySELinuxRestoreSecurityDiskLabel, .domainSetSecurityDaemonSocketLabel = virSecuritySELinuxSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = virSecuritySELinuxSetSecuritySocketLabel, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 63b2720..7f210b2 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -240,16 +240,16 @@ virSecurityStackSetSecurityDiskLabel(virSecurityManagerPtr mgr, static int -virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - virDomainDiskDefPtr disk) +virSecurityStackRestoreSecurityDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainDiskDefPtr disk) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; for (; item; item = item->next) { - if (virSecurityManagerRestoreImageLabel(item->securityManager, vm, disk) < 0) + if (virSecurityManagerRestoreDiskLabel(item->securityManager, vm, disk) < 0) rc = -1; } @@ -579,7 +579,7 @@ virSecurityDriver virSecurityDriverStack = { .domainSecurityVerify = virSecurityStackVerify, .domainSetSecurityDiskLabel = virSecurityStackSetSecurityDiskLabel, - .domainRestoreSecurityImageLabel = virSecurityStackRestoreSecurityImageLabel, + .domainRestoreSecurityDiskLabel = virSecurityStackRestoreSecurityDiskLabel, .domainSetSecurityDaemonSocketLabel = virSecurityStackSetDaemonSocketLabel, .domainSetSecuritySocketLabel = virSecurityStackSetSocketLabel, -- 1.9.3

--- src/security/security_manager.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index d57bab9..84d90ac 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -48,7 +48,17 @@ struct _virSecurityManager { static virClassPtr virSecurityManagerClass; -static void virSecurityManagerDispose(void *obj); + +static +void virSecurityManagerDispose(void *obj) +{ + virSecurityManagerPtr mgr = obj; + + if (mgr->drv->close) + mgr->drv->close(mgr); + VIR_FREE(mgr->privateData); +} + static int virSecurityManagerOnceInit(void) { @@ -231,15 +241,6 @@ void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr) } -static void virSecurityManagerDispose(void *obj) -{ - virSecurityManagerPtr mgr = obj; - - if (mgr->drv->close) - mgr->drv->close(mgr); - VIR_FREE(mgr->privateData); -} - const char * virSecurityManagerGetDriver(virSecurityManagerPtr mgr) { -- 1.9.3

--- src/security/security_manager.c | 248 +++++++++++++++++++++++++--------------- 1 file changed, 157 insertions(+), 91 deletions(-) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 84d90ac..bb12e8e 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -19,10 +19,8 @@ * * Author: Daniel P. Berrange <berrange(a)redhat.com&gt; */ - #include <config.h> - #include "security_driver.h" #include "security_stack.h" #include "security_dac.h" @@ -60,7 +58,8 @@ void virSecurityManagerDispose(void *obj) } -static int virSecurityManagerOnceInit(void) +static int +virSecurityManagerOnceInit(void) { if (!(virSecurityManagerClass = virClassNew(virClassForObjectLockable(), "virSecurityManagerClass", @@ -73,11 +72,13 @@ static int virSecurityManagerOnceInit(void) VIR_ONCE_GLOBAL_INIT(virSecurityManager); -static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr drv, - const char *virtDriver, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined) + +static virSecurityManagerPtr +virSecurityManagerNewDriver(virSecurityDriverPtr drv, + const char *virtDriver, + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined) { virSecurityManagerPtr mgr; char *privateData; @@ -114,7 +115,9 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr return mgr; } -virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary) + +virSecurityManagerPtr +virSecurityManagerNewStack(virSecurityManagerPtr primary) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverStack, @@ -131,21 +134,25 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary) return mgr; } -int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, - virSecurityManagerPtr nested) + +int +virSecurityManagerStackAddNested(virSecurityManagerPtr stack, + virSecurityManagerPtr nested) { if (!STREQ("stack", stack->drv->name)) return -1; return virSecurityStackAddNested(stack, nested); } -virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, - uid_t user, - gid_t group, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined, - bool dynamicOwnership) + +virSecurityManagerPtr +virSecurityManagerNewDAC(const char *virtDriver, + uid_t user, + gid_t group, + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined, + bool dynamicOwnership) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC, @@ -161,16 +168,19 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, virSecurityManagerDispose(mgr); return NULL; } + virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership); return mgr; } -virSecurityManagerPtr virSecurityManagerNew(const char *name, - const char *virtDriver, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined) + +virSecurityManagerPtr +virSecurityManagerNew(const char *name, + const char *virtDriver, + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined) { virSecurityDriverPtr drv = virSecurityDriverLookup(name, virtDriver); if (!drv) @@ -211,7 +221,8 @@ virSecurityManagerPtr virSecurityManagerNew(const char *name, * followed by a call to virSecurityManagerPostFork() in both * parent and child. */ -int virSecurityManagerPreFork(virSecurityManagerPtr mgr) +int +virSecurityManagerPreFork(virSecurityManagerPtr mgr) { int ret = 0; @@ -230,12 +241,14 @@ int virSecurityManagerPreFork(virSecurityManagerPtr mgr) * Must be called after fork()'ing in both parent and child * to ensure mutex state is sane for the child to use */ -void virSecurityManagerPostFork(virSecurityManagerPtr mgr) +void +virSecurityManagerPostFork(virSecurityManagerPtr mgr) { virObjectUnlock(mgr); } -void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr) +void * +virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr) { return mgr->privateData; } @@ -247,6 +260,7 @@ virSecurityManagerGetDriver(virSecurityManagerPtr mgr) return mgr->virtDriver; } + const char * virSecurityManagerGetDOI(virSecurityManagerPtr mgr) { @@ -262,6 +276,7 @@ virSecurityManagerGetDOI(virSecurityManagerPtr mgr) return NULL; } + const char * virSecurityManagerGetModel(virSecurityManagerPtr mgr) { @@ -277,9 +292,11 @@ virSecurityManagerGetModel(virSecurityManagerPtr mgr) return NULL; } + /* return NULL if a base label is not present */ const char * -virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType) +virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, + int virtType) { if (mgr->drv->getBaseLabel) { const char *ret; @@ -292,24 +309,32 @@ virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType) return NULL; } -bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) + +bool +virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) { return mgr->allowDiskFormatProbing; } -bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr) + +bool +virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr) { return mgr->defaultConfined; } -bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) + +bool +virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) { return mgr->requireConfined; } -int virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - virDomainDiskDefPtr disk) + +int +virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainDiskDefPtr disk) { if (mgr->drv->domainRestoreSecurityDiskLabel) { int ret; @@ -323,8 +348,10 @@ int virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm) + +int +virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm) { if (mgr->drv->domainSetSecurityDaemonSocketLabel) { int ret; @@ -338,8 +365,10 @@ int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm) + +int +virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm) { if (mgr->drv->domainSetSecuritySocketLabel) { int ret; @@ -353,8 +382,10 @@ int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm) + +int +virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm) { if (mgr->drv->domainClearSecuritySocketLabel) { int ret; @@ -368,9 +399,11 @@ int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - virDomainDiskDefPtr disk) + +int +virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainDiskDefPtr disk) { if (mgr->drv->domainSetSecurityDiskLabel) { int ret; @@ -384,10 +417,12 @@ int virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - virDomainHostdevDefPtr dev, - const char *vroot) + +int +virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainHostdevDefPtr dev, + const char *vroot) { if (mgr->drv->domainRestoreSecurityHostdevLabel) { int ret; @@ -401,10 +436,12 @@ int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - virDomainHostdevDefPtr dev, - const char *vroot) + +int +virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainHostdevDefPtr dev, + const char *vroot) { if (mgr->drv->domainSetSecurityHostdevLabel) { int ret; @@ -418,9 +455,11 @@ int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - const char *savefile) + +int +virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *savefile) { if (mgr->drv->domainSetSavedStateLabel) { int ret; @@ -434,9 +473,10 @@ int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - const char *savefile) +int +virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *savefile) { if (mgr->drv->domainRestoreSavedStateLabel) { int ret; @@ -450,8 +490,10 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm) + +int +virSecurityManagerGenLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm) { int ret = -1; size_t i, j; @@ -545,9 +587,11 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, return ret; } -int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - pid_t pid) + +int +virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + pid_t pid) { if (mgr->drv->domainReserveSecurityLabel) { int ret; @@ -561,8 +605,10 @@ int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm) + +int +virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm) { if (mgr->drv->domainReleaseSecurityLabel) { int ret; @@ -576,9 +622,11 @@ int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - const char *stdin_path) + +int +virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *stdin_path) { if (mgr->drv->domainSetSecurityAllLabel) { int ret; @@ -592,9 +640,11 @@ int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - bool migrated) + +int +virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + bool migrated) { if (mgr->drv->domainRestoreSecurityAllLabel) { int ret; @@ -608,10 +658,11 @@ int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - pid_t pid, - virSecurityLabelPtr sec) +int +virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + pid_t pid, + virSecurityLabelPtr sec) { if (mgr->drv->domainGetSecurityProcessLabel) { int ret; @@ -625,8 +676,10 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm) + +int +virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm) { if (mgr->drv->domainSetSecurityProcessLabel) { int ret; @@ -640,9 +693,11 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetChildProcessLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - virCommandPtr cmd) + +int +virSecurityManagerSetChildProcessLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virCommandPtr cmd) { if (mgr->drv->domainSetSecurityChildProcessLabel) return mgr->drv->domainSetSecurityChildProcessLabel(mgr, vm, cmd); @@ -651,8 +706,10 @@ int virSecurityManagerSetChildProcessLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerVerify(virSecurityManagerPtr mgr, - virDomainDefPtr def) + +int +virSecurityManagerVerify(virSecurityManagerPtr mgr, + virDomainDefPtr def) { virSecurityLabelDefPtr secdef; @@ -679,9 +736,11 @@ int virSecurityManagerVerify(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - int fd) + +int +virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + int fd) { if (mgr->drv->domainSetSecurityImageFDLabel) { int ret; @@ -695,9 +754,11 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - int fd) + +int +virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + int fd) { if (mgr->drv->domainSetSecurityTapFDLabel) { int ret; @@ -711,8 +772,10 @@ int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, return -1; } -char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, - virDomainDefPtr vm) + +char * +virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, + virDomainDefPtr vm) { if (mgr->drv->domainGetSecurityMountOptions) { char *ret; @@ -726,6 +789,7 @@ char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, return NULL; } + virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr) { @@ -743,9 +807,11 @@ virSecurityManagerGetNested(virSecurityManagerPtr mgr) return list; } -int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - const char *path) + +int +virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) { if (mgr->drv->domainSetSecurityHugepages) { int ret; -- 1.9.3

virSecurityManagerSetDiskLabel and virSecurityManagerRestoreDiskLabel don't have complementary semantics. Document the semantics to avoid possible problems. --- src/security/security_manager.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index bb12e8e..06e5123 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -331,6 +331,17 @@ virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) } +/** + * virSecurityManagerRestoreDiskLabel: + * @mgr: security manager object + * @vm: domain definition object + * @disk: disk definition to operate on + * + * Removes security label from the source image of the disk. Note that this + * function doesn't restore labels on backing chain elements of @disk. + * + * Returns: 0 on success, -1 on error. + */ int virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, @@ -400,6 +411,17 @@ virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, } +/** + * virSecurityManagerSetDiskLabel: + * @mgr: security manager object + * @vm: domain definition object + * @disk: disk definition to operate on + * + * Labels the disk image and all images in the backing chain with the configured + * security label. + * + * Returns: 0 on success, -1 on error. + */ int virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, -- 1.9.3

Some of the functions in the storage driver had their headers formatted incorrectly. --- src/security/security_driver.h | 6 +++--- src/security/security_manager.h | 9 ++++----- src/security/security_stack.c | 4 ++-- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 05d612a..062dc8f 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -108,10 +108,10 @@ typedef int (*virSecurityDomainSetTapFDLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, int fd); typedef char *(*virSecurityDomainGetMountOptions) (virSecurityManagerPtr mgr, - virDomainDefPtr def); + virDomainDefPtr def); typedef int (*virSecurityDomainSetHugepages) (virSecurityManagerPtr mgr, - virDomainDefPtr def, - const char *path); + virDomainDefPtr def, + const char *path); struct _virSecurityDriver { size_t privateDataLen; diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 307e1c2..8a5fcfb 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -118,11 +118,10 @@ int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, int fd); char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, - virDomainDefPtr vm); -virSecurityManagerPtr* -virSecurityManagerGetNested(virSecurityManagerPtr mgr); + virDomainDefPtr vm); +virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr); int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, - virDomainDefPtr sec, - const char *hugepages_path); + virDomainDefPtr sec, + const char *hugepages_path); #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 7f210b2..e3e9c85 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -513,8 +513,8 @@ virSecurityStackSetTapFDLabel(virSecurityManagerPtr mgr, static int virSecurityStackSetHugepages(virSecurityManagerPtr mgr, - virDomainDefPtr vm, - const char *path) + virDomainDefPtr vm, + const char *path) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityStackItemPtr item = priv->itemsHead; -- 1.9.3

The function headers contain type on the same line as the name. When combined with usage of ATTRIBUTE_UNUSED, the function headers were very long. Shorten them by breaking the line after the type. --- src/security/security_nop.c | 147 ++++++++++++++++++++++++++------------------ 1 file changed, 87 insertions(+), 60 deletions(-) diff --git a/src/security/security_nop.c b/src/security/security_nop.c index ac9ceae..b57bf05 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -25,160 +25,187 @@ #define VIR_FROM_THIS VIR_FROM_SECURITY -static virSecurityDriverStatus virSecurityDriverProbeNop(const char *virtDriver ATTRIBUTE_UNUSED) +static virSecurityDriverStatus +virSecurityDriverProbeNop(const char *virtDriver ATTRIBUTE_UNUSED) { return SECURITY_DRIVER_ENABLE; } -static int virSecurityDriverOpenNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +static int +virSecurityDriverOpenNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDriverCloseNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +static int +virSecurityDriverCloseNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { return 0; } -static const char * virSecurityDriverGetModelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +static const char * +virSecurityDriverGetModelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { return "none"; } -static const char * virSecurityDriverGetDOINop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +static const char * +virSecurityDriverGetDOINop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { return "0"; } -static int virSecurityDomainRestoreDiskLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) +static int +virSecurityDomainRestoreDiskLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetDaemonSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetDaemonSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainClearSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED) +static int +virSecurityDomainClearSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetDiskLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetDiskLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainRestoreHostdevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED, - const char *vroot ATTRIBUTE_UNUSED) +static int +virSecurityDomainRestoreHostdevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED, + const char *vroot ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetHostdevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED, - const char *vroot ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetHostdevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED, + const char *vroot ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetSavedStateLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - const char *savefile ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetSavedStateLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + const char *savefile ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainRestoreSavedStateLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - const char *savefile ATTRIBUTE_UNUSED) + +static int +virSecurityDomainRestoreSavedStateLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + const char *savefile ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainGenLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr sec ATTRIBUTE_UNUSED) +static int +virSecurityDomainGenLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr sec ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainReserveLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr sec ATTRIBUTE_UNUSED, - pid_t pid ATTRIBUTE_UNUSED) +static int +virSecurityDomainReserveLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr sec ATTRIBUTE_UNUSED, + pid_t pid ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainReleaseLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr sec ATTRIBUTE_UNUSED) +static int +virSecurityDomainReleaseLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr sec ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr sec ATTRIBUTE_UNUSED, - const char *stdin_path ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr sec ATTRIBUTE_UNUSED, + const char *stdin_path ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainRestoreAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - bool migrated ATTRIBUTE_UNUSED) +static int +virSecurityDomainRestoreAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + bool migrated ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainGetProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - pid_t pid ATTRIBUTE_UNUSED, - virSecurityLabelPtr sec ATTRIBUTE_UNUSED) + +static int +virSecurityDomainGetProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + pid_t pid ATTRIBUTE_UNUSED, + virSecurityLabelPtr sec ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetChildProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED, - virCommandPtr cmd ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetChildProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED, + virCommandPtr cmd ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainVerifyNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr def ATTRIBUTE_UNUSED) +static int +virSecurityDomainVerifyNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED) { return 0; } -static int virSecurityDomainSetFDLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr sec ATTRIBUTE_UNUSED, - int fd ATTRIBUTE_UNUSED) +static int +virSecurityDomainSetFDLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr sec ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED) { return 0; } -static char *virSecurityDomainGetMountOptionsNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr vm ATTRIBUTE_UNUSED) +static char * +virSecurityDomainGetMountOptionsNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr vm ATTRIBUTE_UNUSED) { char *opts; -- 1.9.3

In the future we might need to track state of individual images. Move the readonly and shared flags to the virStorageSource struct so that we can keep them in a per-image basis. --- src/conf/domain_conf.c | 18 ++++++++++-------- src/conf/domain_conf.h | 2 -- src/libxl/libxl_conf.c | 2 +- src/locking/domain_lock.c | 4 ++-- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_command.c | 14 +++++++------- src/qemu/qemu_conf.c | 4 ++-- src/qemu/qemu_driver.c | 8 ++++---- src/qemu/qemu_migration.c | 16 ++++++++++------ src/security/security_dac.c | 2 +- src/security/security_selinux.c | 6 +++--- src/security/virt-aa-helper.c | 2 +- src/util/virstoragefile.h | 6 ++++++ src/vbox/vbox_tmpl.c | 30 +++++++++++++++--------------- src/xenxs/xen_sxpr.c | 10 +++++----- src/xenxs/xen_xm.c | 10 +++++----- 19 files changed, 77 insertions(+), 67 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 02c394f..44903b0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5549,9 +5549,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { - def->readonly = true; + def->src->readonly = true; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { - def->shared = true; + def->src->shared = true; } else if (xmlStrEqual(cur->name, BAD_CAST "transient")) { def->transient = true; } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && @@ -5678,7 +5678,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, /* Force CDROM to be listed as read only */ if (def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - def->readonly = true; + def->src->readonly = true; if ((def->device == VIR_DOMAIN_DISK_DEVICE_DISK || def->device == VIR_DOMAIN_DISK_DEVICE_LUN) && @@ -5700,7 +5700,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, snapshot); goto error; } - } else if (def->readonly) { + } else if (def->src->readonly) { def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; } @@ -13390,7 +13390,8 @@ virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, return false; } - if (src->readonly != dst->readonly || src->shared != dst->shared) { + if (src->src->readonly != dst->src->readonly || + src->src->shared != dst->src->shared) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target disk access mode does not match source")); return false; @@ -15114,7 +15115,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " sgio='%s'", sgio); if (def->snapshot && - !(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE && def->readonly)) + !(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE && + def->src->readonly)) virBufferAsprintf(buf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(def->snapshot)); virBufferAddLit(buf, ">\n"); @@ -15270,9 +15272,9 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, "</iotune>\n"); } - if (def->readonly) + if (def->src->readonly) virBufferAddLit(buf, "<readonly/>\n"); - if (def->shared) + if (def->src->shared) virBufferAddLit(buf, "<shareable/>\n"); if (def->transient) virBufferAddLit(buf, "<transient/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1122eb2..bd85514 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -636,8 +636,6 @@ struct _virDomainDiskDef { int copy_on_read; /* enum virDomainDiskCopyOnRead */ int snapshot; /* virDomainSnapshotLocation, snapshot_conf.h */ int startupPolicy; /* enum virDomainStartupPolicy */ - bool readonly; - bool shared; bool transient; virDomainDeviceInfo info; bool rawio_specified; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4b6b5c0..ea2b21b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -827,7 +827,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) /* XXX is this right? */ x_disk->removable = 1; - x_disk->readwrite = !l_disk->readonly; + x_disk->readwrite = !l_disk->src->readonly; x_disk->is_cdrom = l_disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0; /* An empty CDROM must have the empty format, otherwise libxl fails. */ if (x_disk->is_cdrom && !x_disk->pdev_path) diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index 4b3f4d4..78acaa6 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -83,9 +83,9 @@ static int virDomainLockManagerAddDisk(virLockManagerPtr lock, type == VIR_STORAGE_TYPE_DIR)) return 0; - if (disk->readonly) + if (disk->src->readonly) diskFlags |= VIR_LOCK_MANAGER_RESOURCE_READONLY; - if (disk->shared) + if (disk->src->shared) diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED; VIR_DEBUG("Add disk %s", src); diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 39e30ad..00ff807 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -380,7 +380,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, if (virCgroupAllowDevicePath(cgroup, virDomainDiskGetSource(def->disks[i]), - (def->disks[i]->readonly ? + (def->disks[i]->src->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW) | VIR_CGROUP_DEVICE_MKNOD) < 0) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 38acdff..2866869 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -455,7 +455,7 @@ static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) if (virFileNBDDeviceAssociate(src, format, - disk->readonly, + disk->src->readonly, &dev) < 0) return -1; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3875bf3..257303d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4068,7 +4068,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, goto cleanup; } - perms = (def->readonly ? + perms = (def->src->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW) | VIR_CGROUP_DEVICE_MKNOD; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index a31558f..3394c68 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -61,10 +61,10 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, VIR_DEBUG("Process path %s for disk", path); ret = virCgroupAllowDevicePath(priv->cgroup, path, - (disk->readonly ? VIR_CGROUP_DEVICE_READ + (disk->src->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW)); virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, - disk->readonly ? "r" : "rw", ret == 0); + disk->src->readonly ? "r" : "rw", ret == 0); /* Get this for root squash NFS */ if (ret < 0 && diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 93d303e..58ba214 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3316,7 +3316,7 @@ qemuBuildDriveStr(virConnectPtr conn, goto error; } - if (!disk->readonly) { + if (!disk->src->readonly) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot create virtual FAT disks in read-write mode")); goto error; @@ -3384,7 +3384,7 @@ qemuBuildDriveStr(virConnectPtr conn, disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) && disk->bus != VIR_DOMAIN_DISK_BUS_IDE) virBufferAddLit(&opt, ",boot=on"); - if (disk->readonly && + if (disk->src->readonly && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) virBufferAddLit(&opt, ",readonly=on"); if (disk->transient) { @@ -3443,7 +3443,7 @@ qemuBuildDriveStr(virConnectPtr conn, } virBufferAsprintf(&opt, ",cache=%s", mode); - } else if (disk->shared && !disk->readonly) { + } else if (disk->src->shared && !disk->src->readonly) { virBufferAddLit(&opt, ",cache=off"); } @@ -8019,7 +8019,7 @@ qemuBuildCommandLine(virConnectPtr conn, virStorageFileFormatTypeToString(disk->src->format)); goto error; } - if (!disk->readonly) { + if (!disk->src->readonly) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot create virtual FAT disks in read-write mode")); goto error; @@ -9649,7 +9649,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } else if (STREQ(keywords[i], "media")) { if (STREQ(values[i], "cdrom")) { def->device = VIR_DOMAIN_DISK_DEVICE_CDROM; - def->readonly = true; + def->src->readonly = true; } else if (STREQ(values[i], "floppy")) def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; } else if (STREQ(keywords[i], "format")) { @@ -9705,7 +9705,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } } else if (STREQ(keywords[i], "readonly")) { if ((values[i] == NULL) || STREQ(values[i], "on")) - def->readonly = true; + def->src->readonly = true; } else if (STREQ(keywords[i], "aio")) { if ((def->iomode = virDomainDiskIoTypeFromString(values[i])) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -10873,7 +10873,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; if (VIR_STRDUP(disk->dst, "hdc") < 0) goto error; - disk->readonly = true; + disk->src->readonly = true; } else { if (STRPREFIX(arg, "-fd")) { disk->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8a3bdef..c9ca17c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -906,7 +906,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1013,7 +1013,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ce57542..552e595 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12069,7 +12069,7 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, * permissions it would have as if part of the disk chain is to * temporarily modify the disk in place. */ virStorageSource origdisk; - bool origreadonly = disk->readonly; + bool origreadonly = disk->src->readonly; virQEMUDriverConfigPtr cfg = NULL; int ret = -1; @@ -12084,7 +12084,7 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, * than a full virDomainDiskDef. */ memcpy(&origdisk, disk->src, sizeof(origdisk)); memcpy(disk->src, elem, sizeof(*elem)); - disk->readonly = mode == VIR_DISK_CHAIN_READ_ONLY; + disk->src->readonly = mode == VIR_DISK_CHAIN_READ_ONLY; if (mode == VIR_DISK_CHAIN_NO_ACCESS) { if (virSecurityManagerRestoreDiskLabel(driver->securityManager, @@ -12107,7 +12107,7 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, cleanup: memcpy(disk->src, &origdisk, sizeof(origdisk)); - disk->readonly = origreadonly; + disk->src->readonly = origreadonly; virObjectUnref(cfg); return ret; } @@ -12759,7 +12759,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, case VIR_DOMAIN_SNAPSHOT_LOCATION_NONE: /* Remember seeing a disk that has snapshot disabled */ - if (!dom_disk->readonly) + if (!dom_disk->src->readonly) forbid_internal = true; break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7684aec..8a20bf8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1159,7 +1159,8 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, virDomainDiskDefPtr disk = vm->def->disks[i]; /* skip shared, RO and source-less disks */ - if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1265,7 +1266,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, virDomainBlockJobInfo info; /* skip shared, RO and source-less disks */ - if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1351,7 +1353,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, virDomainDiskDefPtr disk = vm->def->disks[--lastGood]; /* skip shared, RO disks */ - if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1414,7 +1417,8 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, virDomainDiskDefPtr disk = vm->def->disks[i]; /* skip shared, RO and source-less disks */ - if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1528,8 +1532,8 @@ qemuMigrationIsSafe(virDomainDefPtr def) /* Our code elsewhere guarantees shared disks are either readonly (in * which case cache mode doesn't matter) or used with cache=none */ if (src && - !disk->shared && - !disk->readonly && + !disk->src->shared && + !disk->src->readonly && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { int rc; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 639f9b0..38cb47f 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -383,7 +383,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * we can't see running VMs using the file on other nodes * Safest bet is thus to skip the restore step. */ - if (disk->readonly || disk->shared) + if (disk->src->readonly || disk->src->shared) return 0; if (!src) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 572f8a1..7740e69 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1155,7 +1155,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * we can't see running VMs using the file on other nodes * Safest bet is thus to skip the restore step. */ - if (disk->readonly || disk->shared) + if (disk->src->readonly || disk->src->shared) return 0; if (!src || virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) @@ -1213,9 +1213,9 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, ret = virSecuritySELinuxSetFilecon(path, disk_seclabel->label); } else if (depth == 0) { - if (disk->shared) { + if (disk->src->shared) { ret = virSecuritySELinuxSetFileconOptional(path, data->file_context); - } else if (disk->readonly) { + } else if (disk->src->readonly) { ret = virSecuritySELinuxSetFileconOptional(path, data->content_context); } else if (secdef->imagelabel) { ret = virSecuritySELinuxSetFileconOptional(path, secdef->imagelabel); diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e54f73f..b5f66f3 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -907,7 +907,7 @@ add_file_path(virDomainDiskDefPtr disk, int ret; if (depth == 0) { - if (disk->readonly) + if (disk->src->readonly) ret = vah_add_file(buf, path, "r"); else ret = vah_add_file(buf, path, "rw"); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 48c7e02..fe17b0b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -240,6 +240,12 @@ struct _virStorageSource { size_t nseclabels; virSecurityDeviceLabelDefPtr *seclabels; + /* Don't ever write to the image */ + bool readonly; + + /* image is shared across hosts */ + bool shared; + /* backing chain of the storage source */ virStorageSourcePtr backingStore; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 4ba9ad7..91e9482 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2791,7 +2791,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { hardDiskPM->vtbl->GetType(hardDiskPM, &hddType); if (hddType == HardDiskType_Immutable) - def->disks[hddNum]->readonly = true; + def->disks[hddNum]->src->readonly = true; ignore_value(virDomainDiskSetSource(def->disks[hddNum], hddlocation)); ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hda")); @@ -2813,7 +2813,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { hardDiskPS->vtbl->GetType(hardDiskPS, &hddType); if (hddType == HardDiskType_Immutable) - def->disks[hddNum]->readonly = true; + def->disks[hddNum]->src->readonly = true; ignore_value(virDomainDiskSetSource(def->disks[hddNum], hddlocation)); ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hdb")); @@ -2835,7 +2835,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { hardDiskSS->vtbl->GetType(hardDiskSS, &hddType); if (hddType == HardDiskType_Immutable) - def->disks[hddNum]->readonly = true; + def->disks[hddNum]->src->readonly = true; ignore_value(virDomainDiskSetSource(def->disks[hddNum], hddlocation)); ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hdd")); @@ -2977,7 +2977,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { medium->vtbl->GetReadOnly(medium, &readOnly); if (readOnly == PR_TRUE) - def->disks[diskCount]->readonly = true; + def->disks[diskCount]->src->readonly = true; virDomainDiskSetType(def->disks[diskCount], VIR_STORAGE_TYPE_FILE); @@ -3257,7 +3257,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { def->disks[def->ndisks - 1]->bus = VIR_DOMAIN_DISK_BUS_IDE; virDomainDiskSetType(def->disks[def->ndisks - 1], VIR_STORAGE_TYPE_FILE); - def->disks[def->ndisks - 1]->readonly = true; + def->disks[def->ndisks - 1]->src->readonly = true; ignore_value(virDomainDiskSetSource(def->disks[def->ndisks - 1], location)); ignore_value(VIR_STRDUP(def->disks[def->ndisks - 1]->dst, "hdc")); def->ndisks--; @@ -3304,7 +3304,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { def->disks[def->ndisks - 1]->bus = VIR_DOMAIN_DISK_BUS_FDC; virDomainDiskSetType(def->disks[def->ndisks - 1], VIR_STORAGE_TYPE_FILE); - def->disks[def->ndisks - 1]->readonly = false; + def->disks[def->ndisks - 1]->src->readonly = false; ignore_value(virDomainDiskSetSource(def->disks[def->ndisks - 1], location)); ignore_value(VIR_STRDUP(def->disks[def->ndisks - 1]->dst, "fda")); def->ndisks--; @@ -3910,9 +3910,9 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VIR_DEBUG("disk(%zu) driverType: %s", i, virStorageFileFormatTypeToString(format)); VIR_DEBUG("disk(%zu) cachemode: %d", i, def->disks[i]->cachemode); - VIR_DEBUG("disk(%zu) readonly: %s", i, (def->disks[i]->readonly + VIR_DEBUG("disk(%zu) readonly: %s", i, (def->disks[i]->src->readonly ? "True" : "False")); - VIR_DEBUG("disk(%zu) shared: %s", i, (def->disks[i]->shared + VIR_DEBUG("disk(%zu) shared: %s", i, (def->disks[i]->src->shared ? "True" : "False")); if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { @@ -4014,11 +4014,11 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) "attached as harddisk: %s, rc=%08x"), src, (unsigned)rc); } else { - if (def->disks[i]->readonly) { + if (def->disks[i]->src->readonly) { hardDisk->vtbl->SetType(hardDisk, HardDiskType_Immutable); VIR_DEBUG("setting harddisk to readonly"); - } else if (!def->disks[i]->readonly) { + } else if (!def->disks[i]->src->readonly) { hardDisk->vtbl->SetType(hardDisk, HardDiskType_Normal); VIR_DEBUG("setting harddisk type to normal"); @@ -4193,9 +4193,9 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VIR_DEBUG("disk(%zu) driverType: %s", i, virStorageFileFormatTypeToString(format)); VIR_DEBUG("disk(%zu) cachemode: %d", i, def->disks[i]->cachemode); - VIR_DEBUG("disk(%zu) readonly: %s", i, (def->disks[i]->readonly + VIR_DEBUG("disk(%zu) readonly: %s", i, (def->disks[i]->src->readonly ? "True" : "False")); - VIR_DEBUG("disk(%zu) shared: %s", i, (def->disks[i]->shared + VIR_DEBUG("disk(%zu) shared: %s", i, (def->disks[i]->src->shared ? "True" : "False")); if (type == VIR_STORAGE_TYPE_FILE && src) { @@ -4317,10 +4317,10 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (def->disks[i]->readonly) { + if (def->disks[i]->src->readonly) { medium->vtbl->SetType(medium, MediumType_Immutable); VIR_DEBUG("setting harddisk to immutable"); - } else if (!def->disks[i]->readonly) { + } else if (!def->disks[i]->src->readonly) { medium->vtbl->SetType(medium, MediumType_Normal); VIR_DEBUG("setting harddisk type to normal"); } @@ -7500,7 +7500,7 @@ int vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot, goto cleanup; } if (readOnly == PR_TRUE) - def->dom->disks[diskCount]->readonly = true; + def->dom->disks[diskCount]->src->readonly = true; def->dom->disks[diskCount]->src->type = VIR_STORAGE_TYPE_FILE; def->dom->disks[diskCount]->dst = vboxGenerateMediumName(storageBus, deviceInst, diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index aacf74c..08a10e7 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -495,10 +495,10 @@ xenParseSxprDisks(virDomainDefPtr def, if (mode && strchr(mode, 'r')) - disk->readonly = true; + disk->src->readonly = true; if (mode && strchr(mode, '!')) - disk->shared = true; + disk->src->shared = true; if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) goto error; @@ -1321,7 +1321,7 @@ xenParseSxpr(const struct sexpr *root, goto error; } disk->bus = VIR_DOMAIN_DISK_BUS_IDE; - disk->readonly = true; + disk->src->readonly = true; if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) { virDomainDiskDefFree(disk); @@ -1818,9 +1818,9 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, } } - if (def->readonly) + if (def->src->readonly) virBufferAddLit(buf, "(mode 'r')"); - else if (def->shared) + else if (def->src->shared) virBufferAddLit(buf, "(mode 'w!')"); else virBufferAddLit(buf, "(mode 'w')"); diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 2cd6d4c..6349d17 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -625,10 +625,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (STREQ(head, "r") || STREQ(head, "ro")) - disk->readonly = true; + disk->src->readonly = true; else if ((STREQ(head, "w!")) || (STREQ(head, "!"))) - disk->shared = true; + disk->src->shared = true; /* Maintain list in sorted order according to target device name */ if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) @@ -656,7 +656,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (VIR_STRDUP(disk->dst, "hdc") < 0) goto cleanup; disk->bus = VIR_DOMAIN_DISK_BUS_IDE; - disk->readonly = true; + disk->src->readonly = true; if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) goto cleanup; @@ -1249,9 +1249,9 @@ xenFormatXMDisk(virConfValuePtr list, if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&buf, ":cdrom"); - if (disk->readonly) + if (disk->src->readonly) virBufferAddLit(&buf, ",r"); - else if (disk->shared) + else if (disk->src->shared) virBufferAddLit(&buf, ",!"); else virBufferAddLit(&buf, ",w"); -- 1.9.3

They will be reused to transfer disk labels from snapshotted disks to the new disk definitions. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 45 ++++++++++++++++++++++++++++++++++++++------- src/util/virstoragefile.h | 3 +++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e1dd84..792754f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1913,6 +1913,7 @@ virStorageSourceNewFromBacking; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourceSeclabelsCopy; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0c50de1..c52206c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1515,6 +1515,31 @@ virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src, } +int +virStorageSourceSeclabelsCopy(virStorageSourcePtr to, + const virStorageSource *from) +{ + size_t i; + + virStorageSourceSeclabelsClear(to); + + if (VIR_ALLOC_N(to->seclabels, from->nseclabels) < 0) + return -1; + to->nseclabels = from->nseclabels; + + for (i = 0; i < to->nseclabels; i++) { + if (!(to->seclabels[i] = virSecurityDeviceLabelDefCopy(from->seclabels[i]))) + goto error; + } + + return 0; + + error: + virStorageSourceSeclabelsClear(to); + return -1; +} + + void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def) { @@ -1573,10 +1598,21 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) void -virStorageSourceClear(virStorageSourcePtr def) +virStorageSourceSeclabelsClear(virStorageSourcePtr def) { size_t i; + if (def->seclabels) { + for (i = 0; i < def->nseclabels; i++) + virSecurityDeviceLabelDefFree(def->seclabels[i]); + VIR_FREE(def->seclabels); + } +} + + +void +virStorageSourceClear(virStorageSourcePtr def) +{ if (!def) return; @@ -1587,12 +1623,7 @@ virStorageSourceClear(virStorageSourcePtr def) virBitmapFree(def->features); VIR_FREE(def->compat); virStorageEncryptionFree(def->encryption); - - if (def->seclabels) { - for (i = 0; i < def->nseclabels; i++) - virSecurityDeviceLabelDefFree(def->seclabels[i]); - VIR_FREE(def->seclabels); - } + virStorageSourceSeclabelsClear(def); virStoragePermsFree(def->perms); VIR_FREE(def->timestamps); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index fe17b0b..176661e 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -322,6 +322,9 @@ void virStorageNetHostDefFree(size_t nhosts, virStorageNetHostDefPtr hosts); virStorageNetHostDefPtr virStorageNetHostDefCopy(size_t nhosts, virStorageNetHostDefPtr hosts); +void virStorageSourceSeclabelsClear(virStorageSourcePtr def); +int virStorageSourceSeclabelsCopy(virStorageSourcePtr to, + const virStorageSource *from); void virStorageSourceAuthClear(virStorageSourcePtr def); void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def); void virStorageSourceClear(virStorageSourcePtr def); -- 1.9.3

Now that we have pointers to store disk source information and thus can easily exchange the structs behind we need a function to copy all the data. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 3 + 3 files changed, 147 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 792754f..bf3a45b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1906,6 +1906,7 @@ virStorageNetProtocolTypeToString; virStorageSourceAuthClear; virStorageSourceBackingStoreClear; virStorageSourceClear; +virStorageSourceCopy; virStorageSourceFree; virStorageSourceGetActualType; virStorageSourceGetSecurityLabelDef; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c52206c..5f8e02d 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1540,6 +1540,149 @@ virStorageSourceSeclabelsCopy(virStorageSourcePtr to, } +static virStorageTimestampsPtr +virStorageTimestampsCopy(const virStorageTimestamps *src) +{ + virStorageTimestampsPtr ret; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + memcpy(ret, src, sizeof(*src)); + + return ret; +} + + +static virStoragePermsPtr +virStoragePermsCopy(const virStoragePerms *src) +{ + virStoragePermsPtr ret; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + ret->mode = src->mode; + ret->uid = src->uid; + ret->gid = src->gid; + + if (VIR_STRDUP(ret->label, src->label)) + goto error; + + return ret; + + error: + virStoragePermsFree(ret); + return NULL; +} + + +static virStorageSourcePoolDefPtr +virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) +{ + virStorageSourcePoolDefPtr ret; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + ret->voltype = src->voltype; + ret->pooltype = src->pooltype; + ret->actualtype = src->actualtype; + ret->mode = src->mode; + + if (VIR_STRDUP(ret->pool, src->pool) < 0 || + VIR_STRDUP(ret->volume, src->volume) < 0) + goto error; + + return ret; + + error: + virStorageSourcePoolDefFree(ret); + return NULL; +} + + +virStorageSourcePtr +virStorageSourceCopy(const virStorageSource *src, + bool backingChain) +{ + virStorageSourcePtr ret = NULL; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + ret->type = src->type; + ret->protocol = src->protocol; + ret->format = src->format; + ret->allocation = src->allocation; + ret->capacity = src->capacity; + ret->readonly = src->readonly; + ret->shared = src->shared; + + /* storage driver metadata are not copied */ + ret->drv = NULL; + + if (VIR_STRDUP(ret->path, src->path) < 0 || + VIR_STRDUP(ret->volume, src->volume) < 0 || + VIR_STRDUP(ret->driverName, src->driverName) < 0 || + VIR_STRDUP(ret->relPath, src->relPath) < 0 || + VIR_STRDUP(ret->backingStoreRaw, src->backingStoreRaw) < 0 || + VIR_STRDUP(ret->compat, src->compat) < 0 || + VIR_STRDUP(ret->auth.username, src->auth.username) < 0) + goto error; + + if (!(ret->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts))) + goto error; + ret->nhosts = src->nhosts; + + if (!(ret->srcpool = virStorageSourcePoolDefCopy(src->srcpool))) + goto error; + + if (!(ret->features = virBitmapNewCopy(src->features))) + goto error; + + if (!(ret->encryption = virStorageEncryptionCopy(src->encryption))) + goto error; + + if (!(ret->perms = virStoragePermsCopy(src->perms))) + goto error; + + if (!(ret->timestamps = virStorageTimestampsCopy(src->timestamps))) + goto error; + + if (virStorageSourceSeclabelsCopy(ret, src) < 0) + goto error; + + ret->auth.secretType = src->auth.secretType; + switch ((virStorageSecretType) src->auth.secretType) { + case VIR_STORAGE_SECRET_TYPE_NONE: + case VIR_STORAGE_SECRET_TYPE_LAST: + break; + + case VIR_STORAGE_SECRET_TYPE_UUID: + memcpy(ret->auth.secret.uuid, src->auth.secret.uuid, VIR_UUID_BUFLEN); + break; + + case VIR_STORAGE_SECRET_TYPE_USAGE: + if (VIR_STRDUP(ret->auth.secret.usage, src->auth.secret.usage) < 0) + goto error; + break; + } + + if (backingChain && src->backingStore) { + if (!(ret->backingStore = virStorageSourceCopy(src->backingStore, + true))) + goto error; + } + + return ret; + + error: + virStorageSourceFree(ret); + return NULL; +} + + void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def) { diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 176661e..3e13071 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -332,6 +332,9 @@ int virStorageSourceGetActualType(virStorageSourcePtr def); void virStorageSourceFree(virStorageSourcePtr def); void virStorageSourceBackingStoreClear(virStorageSourcePtr def); virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent); +virStorageSourcePtr virStorageSourceCopy(const virStorageSource *src, + bool backingChain) + ATTRIBUTE_NONNULL(1); typedef int (*virStorageFileSimplifyPathReadlinkCallback)(const char *path, -- 1.9.3

Cgroups code uses VIR_CGROUP_DEVICE_* flags to specify the mode but in the end it needs to be converted to a string. Add a helper to do it and use it in the cgroup code before introducing it into the rest of the code. --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 62 +++++++++++++++++++++++++++++++++++------------- src/util/vircgroup.h | 2 ++ 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bf3a45b..99e9d52 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1049,6 +1049,7 @@ virCgroupGetCpuCfsQuota; virCgroupGetCpusetCpus; virCgroupGetCpusetMems; virCgroupGetCpuShares; +virCgroupGetDevicePermsString; virCgroupGetDomainTotalCpuStats; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index c578bd0..2eaf265 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2624,6 +2624,44 @@ virCgroupDenyAllDevices(virCgroupPtr group) /** + * virCgroupGetDevicePermsString: + * + * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits + * + * Returns string corresponding to the appropriate bits set. + */ +const char * +virCgroupGetDevicePermsString(int perms) +{ + if (perms & VIR_CGROUP_DEVICE_READ) { + if (perms & VIR_CGROUP_DEVICE_WRITE) { + if (perms & VIR_CGROUP_DEVICE_MKNOD) + return "rwm"; + else + return "rw"; + } else { + if (perms & VIR_CGROUP_DEVICE_MKNOD) + return "rm"; + else + return "r"; + } + } else { + if (perms & VIR_CGROUP_DEVICE_WRITE) { + if (perms & VIR_CGROUP_DEVICE_MKNOD) + return "wm"; + else + return "w"; + } else { + if (perms & VIR_CGROUP_DEVICE_MKNOD) + return "m"; + else + return ""; + } + } +} + + +/** * virCgroupAllowDevice: * * @group: The cgroup to allow a device for @@ -2641,10 +2679,8 @@ virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor, int ret = -1; char *devstr = NULL; - if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor, - perms & VIR_CGROUP_DEVICE_READ ? "r" : "", - perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + if (virAsprintf(&devstr, "%c %i:%i %s", type, major, minor, + virCgroupGetDevicePermsString(perms)) < 0) goto cleanup; if (virCgroupSetValueStr(group, @@ -2678,10 +2714,8 @@ virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major, int ret = -1; char *devstr = NULL; - if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major, - perms & VIR_CGROUP_DEVICE_READ ? "r" : "", - perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + if (virAsprintf(&devstr, "%c %i:* %s", type, major, + virCgroupGetDevicePermsString(perms)) < 0) goto cleanup; if (virCgroupSetValueStr(group, @@ -2752,10 +2786,8 @@ virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor, int ret = -1; char *devstr = NULL; - if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor, - perms & VIR_CGROUP_DEVICE_READ ? "r" : "", - perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + if (virAsprintf(&devstr, "%c %i:%i %s", type, major, minor, + virCgroupGetDevicePermsString(perms)) < 0) goto cleanup; if (virCgroupSetValueStr(group, @@ -2789,10 +2821,8 @@ virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major, int ret = -1; char *devstr = NULL; - if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major, - perms & VIR_CGROUP_DEVICE_READ ? "r" : "", - perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + if (virAsprintf(&devstr, "%c %i:* %s", type, major, + virCgroupGetDevicePermsString(perms)) < 0) goto cleanup; if (virCgroupSetValueStr(group, diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 7bb46bf..3ab9f1c 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -173,6 +173,8 @@ enum { VIR_CGROUP_DEVICE_RWM = VIR_CGROUP_DEVICE_RW | VIR_CGROUP_DEVICE_MKNOD, }; +const char *virCgroupGetDevicePermsString(int perms); + int virCgroupDenyAllDevices(virCgroupPtr group); int virCgroupAllowDevice(virCgroupPtr group, -- 1.9.3

Add functions that will allow to set all the required cgroup stuff on individual images taking a virStorageSourcePtr. Also convert functions designed to setup whole backing chain to take advantage of the chagne. --- src/qemu/qemu_cgroup.c | 104 ++++++++++++++++++++++++------------------------- src/qemu/qemu_cgroup.h | 3 ++ 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3394c68..1deafb7 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -49,30 +49,56 @@ static const char *const defaultDeviceACL[] = { #define DEVICE_PTY_MAJOR 136 #define DEVICE_SND_MAJOR 116 -static int -qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, - const char *path, - size_t depth ATTRIBUTE_UNUSED, - void *opaque) +int +qemuSetImageCgroup(virDomainObjPtr vm, + virStorageSourcePtr src, + bool deny) { - virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; + int perms = VIR_CGROUP_DEVICE_READ; int ret; - VIR_DEBUG("Process path %s for disk", path); - ret = virCgroupAllowDevicePath(priv->cgroup, path, - (disk->src->readonly ? VIR_CGROUP_DEVICE_READ - : VIR_CGROUP_DEVICE_RW)); - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, - disk->src->readonly ? "r" : "rw", ret == 0); + if (!virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; + + if (!src->path || + virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK) { + VIR_DEBUG("Not updating cgroups for disk path '%s', type: %s", + NULLSTR(src->path), virStorageTypeToString(src->type)); + return 0; + } + + if (deny) { + perms |= VIR_CGROUP_DEVICE_WRITE | VIR_CGROUP_DEVICE_MKNOD; + + VIR_DEBUG("Deny path %s", src->path); + + ret = virCgroupDenyDevicePath(priv->cgroup, src->path, perms); + } else { + if (!src->readonly) + perms |= VIR_CGROUP_DEVICE_WRITE; + + VIR_DEBUG("Allow path %s, perms: %s", + src->path, virCgroupGetDevicePermsString(perms)); + + ret = virCgroupAllowDevicePath(priv->cgroup, src->path, perms); + } + + virDomainAuditCgroupPath(vm, priv->cgroup, + deny ? "deny" : "allow", + src->path, + virCgroupGetDevicePermsString(perms), + ret == 0); /* Get this for root squash NFS */ if (ret < 0 && virLastErrorIsSystemErrno(EACCES)) { - VIR_DEBUG("Ignoring EACCES for %s", path); + VIR_DEBUG("Ignoring EACCES for %s", src->path); virResetLastError(); ret = 0; } + return ret; } @@ -81,39 +107,14 @@ int qemuSetupDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk) { - qemuDomainObjPrivatePtr priv = vm->privateData; - - if (!virCgroupHasController(priv->cgroup, - VIR_CGROUP_CONTROLLER_DEVICES)) - return 0; - - return virDomainDiskDefForeachPath(disk, true, qemuSetupDiskPathAllow, vm); -} + virStorageSourcePtr next; - -static int -qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, - const char *path, - size_t depth ATTRIBUTE_UNUSED, - void *opaque) -{ - virDomainObjPtr vm = opaque; - qemuDomainObjPrivatePtr priv = vm->privateData; - int ret; - - VIR_DEBUG("Process path %s for disk", path); - ret = virCgroupDenyDevicePath(priv->cgroup, path, - VIR_CGROUP_DEVICE_RWM); - virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", ret == 0); - - /* Get this for root squash NFS */ - if (ret < 0 && - virLastErrorIsSystemErrno(EACCES)) { - VIR_DEBUG("Ignoring EACCES for %s", path); - virResetLastError(); - ret = 0; + for (next = disk->src; next; next = next->backingStore) { + if (qemuSetImageCgroup(vm, next, false) < 0) + return -1; } - return ret; + + return 0; } @@ -121,18 +122,17 @@ int qemuTeardownDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk) { - qemuDomainObjPrivatePtr priv = vm->privateData; + virStorageSourcePtr next; - if (!virCgroupHasController(priv->cgroup, - VIR_CGROUP_CONTROLLER_DEVICES)) - return 0; + for (next = disk->src; next; next = next->backingStore) { + if (qemuSetImageCgroup(vm, next, true) < 0) + return -1; + } - return virDomainDiskDefForeachPath(disk, - true, - qemuTeardownDiskPathDeny, - vm); + return 0; } + static int qemuSetupChrSourceCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrSourceDefPtr dev, diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 14404d1..732860e 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -29,6 +29,9 @@ # include "domain_conf.h" # include "qemu_conf.h" +int qemuSetImageCgroup(virDomainObjPtr vm, + virStorageSourcePtr src, + bool deny); int qemuSetupDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk); int qemuTeardownDiskCgroup(virDomainObjPtr vm, -- 1.9.3

Only the top level gets writes, so the rest of the backing chain requires only read-only access. --- src/qemu/qemu_cgroup.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1deafb7..97229c4 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -49,10 +49,11 @@ static const char *const defaultDeviceACL[] = { #define DEVICE_PTY_MAJOR 136 #define DEVICE_SND_MAJOR 116 -int -qemuSetImageCgroup(virDomainObjPtr vm, - virStorageSourcePtr src, - bool deny) +static int +qemuSetImageCgroupInternal(virDomainObjPtr vm, + virStorageSourcePtr src, + bool deny, + bool forceReadonly) { qemuDomainObjPrivatePtr priv = vm->privateData; int perms = VIR_CGROUP_DEVICE_READ; @@ -76,7 +77,7 @@ qemuSetImageCgroup(virDomainObjPtr vm, ret = virCgroupDenyDevicePath(priv->cgroup, src->path, perms); } else { - if (!src->readonly) + if (!src->readonly && !forceReadonly) perms |= VIR_CGROUP_DEVICE_WRITE; VIR_DEBUG("Allow path %s, perms: %s", @@ -104,14 +105,27 @@ qemuSetImageCgroup(virDomainObjPtr vm, int +qemuSetImageCgroup(virDomainObjPtr vm, + virStorageSourcePtr src, + bool deny) +{ + return qemuSetImageCgroupInternal(vm, src, deny, false); +} + + +int qemuSetupDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk) { virStorageSourcePtr next; + bool forceReadonly = false; for (next = disk->src; next; next = next->backingStore) { - if (qemuSetImageCgroup(vm, next, false) < 0) + if (qemuSetImageCgroupInternal(vm, next, false, forceReadonly) < 0) return -1; + + /* setup only the top level image for read-write */ + forceReadonly = true; } return 0; -- 1.9.3

Add helper APIs to manage individual image files rather than disks. To simplify the addition some parts of the code were refactored in this patch. --- src/libvirt_private.syms | 2 ++ src/locking/domain_lock.c | 65 ++++++++++++++++++++++++++++++----------------- src/locking/domain_lock.h | 8 ++++++ 3 files changed, 52 insertions(+), 23 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 99e9d52..78d6e3c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -853,6 +853,8 @@ virRegisterStorageDriver; # locking/domain_lock.h virDomainLockDiskAttach; virDomainLockDiskDetach; +virDomainLockImageAttach; +virDomainLockImageDetach; virDomainLockLeaseAttach; virDomainLockLeaseDetach; virDomainLockProcessInquire; diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index 78acaa6..d7b681e 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -68,14 +68,13 @@ static int virDomainLockManagerAddLease(virLockManagerPtr lock, } -static int virDomainLockManagerAddDisk(virLockManagerPtr lock, - virDomainDiskDefPtr disk) +static int virDomainLockManagerAddImage(virLockManagerPtr lock, + virStorageSourcePtr src) { unsigned int diskFlags = 0; - const char *src = virDomainDiskGetSource(disk); - int type = virDomainDiskGetType(disk); + int type = virStorageSourceGetActualType(src); - if (!src) + if (!src->path) return 0; if (!(type == VIR_STORAGE_TYPE_BLOCK || @@ -83,24 +82,25 @@ static int virDomainLockManagerAddDisk(virLockManagerPtr lock, type == VIR_STORAGE_TYPE_DIR)) return 0; - if (disk->src->readonly) + if (src->readonly) diskFlags |= VIR_LOCK_MANAGER_RESOURCE_READONLY; - if (disk->src->shared) + if (src->shared) diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED; - VIR_DEBUG("Add disk %s", src); + VIR_DEBUG("Add disk %s", src->path); if (virLockManagerAddResource(lock, VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, - src, + src->path, 0, NULL, diskFlags) < 0) { - VIR_DEBUG("Failed add disk %s", src); + VIR_DEBUG("Failed add disk %s", src->path); return -1; } return 0; } + static virLockManagerPtr virDomainLockManagerNew(virLockManagerPluginPtr plugin, const char *uri, virDomainObjPtr dom, @@ -148,9 +148,12 @@ static virLockManagerPtr virDomainLockManagerNew(virLockManagerPluginPtr plugin, goto error; VIR_DEBUG("Adding disks"); - for (i = 0; i < dom->def->ndisks; i++) - if (virDomainLockManagerAddDisk(lock, dom->def->disks[i]) < 0) + for (i = 0; i < dom->def->ndisks; i++) { + virDomainDiskDefPtr disk = dom->def->disks[i]; + + if (virDomainLockManagerAddImage(lock, disk->src) < 0) goto error; + } } return lock; @@ -247,21 +250,20 @@ int virDomainLockProcessInquire(virLockManagerPluginPtr plugin, } -int virDomainLockDiskAttach(virLockManagerPluginPtr plugin, - const char *uri, - virDomainObjPtr dom, - virDomainDiskDefPtr disk) +int virDomainLockImageAttach(virLockManagerPluginPtr plugin, + const char *uri, + virDomainObjPtr dom, + virStorageSourcePtr src) { virLockManagerPtr lock; int ret = -1; - VIR_DEBUG("plugin=%p dom=%p disk=%p", - plugin, dom, disk); + VIR_DEBUG("plugin=%p dom=%p src=%p", plugin, dom, src); if (!(lock = virDomainLockManagerNew(plugin, uri, dom, false))) return -1; - if (virDomainLockManagerAddDisk(lock, disk) < 0) + if (virDomainLockManagerAddImage(lock, src) < 0) goto cleanup; if (virLockManagerAcquire(lock, NULL, 0, @@ -276,20 +278,29 @@ int virDomainLockDiskAttach(virLockManagerPluginPtr plugin, return ret; } -int virDomainLockDiskDetach(virLockManagerPluginPtr plugin, + +int virDomainLockDiskAttach(virLockManagerPluginPtr plugin, + const char *uri, virDomainObjPtr dom, virDomainDiskDefPtr disk) { + return virDomainLockImageAttach(plugin, uri, dom, disk->src); +} + + +int virDomainLockImageDetach(virLockManagerPluginPtr plugin, + virDomainObjPtr dom, + virStorageSourcePtr src) +{ virLockManagerPtr lock; int ret = -1; - VIR_DEBUG("plugin=%p dom=%p disk=%p", - plugin, dom, disk); + VIR_DEBUG("plugin=%p dom=%p src=%p", plugin, dom, src); if (!(lock = virDomainLockManagerNew(plugin, NULL, dom, false))) return -1; - if (virDomainLockManagerAddDisk(lock, disk) < 0) + if (virDomainLockManagerAddImage(lock, src) < 0) goto cleanup; if (virLockManagerRelease(lock, NULL, 0) < 0) @@ -304,6 +315,14 @@ int virDomainLockDiskDetach(virLockManagerPluginPtr plugin, } +int virDomainLockDiskDetach(virLockManagerPluginPtr plugin, + virDomainObjPtr dom, + virDomainDiskDefPtr disk) +{ + return virDomainLockImageDetach(plugin, dom, disk->src); +} + + int virDomainLockLeaseAttach(virLockManagerPluginPtr plugin, const char *uri, virDomainObjPtr dom, diff --git a/src/locking/domain_lock.h b/src/locking/domain_lock.h index a9b19c8..fb49102 100644 --- a/src/locking/domain_lock.h +++ b/src/locking/domain_lock.h @@ -50,6 +50,14 @@ int virDomainLockDiskDetach(virLockManagerPluginPtr plugin, virDomainObjPtr dom, virDomainDiskDefPtr disk); +int virDomainLockImageAttach(virLockManagerPluginPtr plugin, + const char *uri, + virDomainObjPtr dom, + virStorageSourcePtr src); +int virDomainLockImageDetach(virLockManagerPluginPtr plugin, + virDomainObjPtr dom, + virStorageSourcePtr src); + int virDomainLockLeaseAttach(virLockManagerPluginPtr plugin, const char *uri, virDomainObjPtr dom, -- 1.9.3

Add security driver functions to label separate storage images using the virStorageSource definition. This will help to avoid the need to do ugly changes to the disk struct and use the source directly. --- src/libvirt_private.syms | 2 ++ src/security/security_driver.h | 10 ++++++++ src/security/security_manager.c | 56 +++++++++++++++++++++++++++++++++++++++++ src/security/security_manager.h | 7 ++++++ src/security/security_nop.c | 19 ++++++++++++++ src/security/security_stack.c | 38 ++++++++++++++++++++++++++++ 6 files changed, 132 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 78d6e3c..98595e1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -915,6 +915,7 @@ virSecurityManagerReserveLabel; virSecurityManagerRestoreAllLabel; virSecurityManagerRestoreDiskLabel; virSecurityManagerRestoreHostdevLabel; +virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreSavedStateLabel; virSecurityManagerSetAllLabel; virSecurityManagerSetChildProcessLabel; @@ -923,6 +924,7 @@ virSecurityManagerSetDiskLabel; virSecurityManagerSetHostdevLabel; virSecurityManagerSetHugepages; virSecurityManagerSetImageFDLabel; +virSecurityManagerSetImageLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 062dc8f..f0dca09 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -112,6 +112,13 @@ typedef char *(*virSecurityDomainGetMountOptions) (virSecurityManagerPtr mgr, typedef int (*virSecurityDomainSetHugepages) (virSecurityManagerPtr mgr, virDomainDefPtr def, const char *path); +typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src); +typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src); + struct _virSecurityDriver { size_t privateDataLen; @@ -130,6 +137,9 @@ struct _virSecurityDriver { virSecurityDomainSetDiskLabel domainSetSecurityDiskLabel; virSecurityDomainRestoreDiskLabel domainRestoreSecurityDiskLabel; + virSecurityDomainSetImageLabel domainSetSecurityImageLabel; + virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel; + virSecurityDomainSetDaemonSocketLabel domainSetSecurityDaemonSocketLabel; virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; virSecurityDomainClearSocketLabel domainClearSecuritySocketLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 06e5123..16bec5c 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -360,6 +360,34 @@ virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, } +/** + * virSecurityManagerRestoreImageLabel: + * @mgr: security manager object + * @vm: domain definition object + * @src: disk source definition to operate on + * + * Removes security label from a single storage image. + * + * Returns: 0 on success, -1 on error. + */ +int +virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virStorageSourcePtr src) +{ + if (mgr->drv->domainRestoreSecurityImageLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src); + virObjectUnlock(mgr); + return ret; + } + + virReportUnsupportedError(); + return -1; +} + + int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) @@ -440,6 +468,34 @@ virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr, } +/** + * virSecurityManagerSetImageLabel: + * @mgr: security manager object + * @vm: domain definition object + * @src: disk source definition to operate on + * + * Labels a single storage image with the configured security label. + * + * Returns: 0 on success, -1 on error. + */ +int +virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virStorageSourcePtr src) +{ + if (mgr->drv->domainSetSecurityImageLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, src); + virObjectUnlock(mgr); + return ret; + } + + virReportUnsupportedError(); + return -1; +} + + int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 8a5fcfb..97b6a2e 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -124,4 +124,11 @@ int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, virDomainDefPtr sec, const char *hugepages_path); +int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virStorageSourcePtr src); +int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virStorageSourcePtr src); + #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_nop.c b/src/security/security_nop.c index b57bf05..951125d 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -220,6 +220,22 @@ virSecurityGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return NULL; } +static int +virSecurityDomainRestoreImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virStorageSourcePtr src ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityDomainSetImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virStorageSourcePtr src ATTRIBUTE_UNUSED) +{ + return 0; +} + virSecurityDriver virSecurityDriverNop = { .privateDataLen = 0, @@ -236,6 +252,9 @@ virSecurityDriver virSecurityDriverNop = { .domainSetSecurityDiskLabel = virSecurityDomainSetDiskLabelNop, .domainRestoreSecurityDiskLabel = virSecurityDomainRestoreDiskLabelNop, + .domainSetSecurityImageLabel = virSecurityDomainSetImageLabelNop, + .domainRestoreSecurityImageLabel = virSecurityDomainRestoreImageLabelNop, + .domainSetSecurityDaemonSocketLabel = virSecurityDomainSetDaemonSocketLabelNop, .domainSetSecuritySocketLabel = virSecurityDomainSetSocketLabelNop, .domainClearSecuritySocketLabel = virSecurityDomainClearSocketLabelNop, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index e3e9c85..1ded57b 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -564,6 +564,41 @@ virSecurityStackGetBaseLabel(virSecurityManagerPtr mgr, int virtType) virtType); } +static int +virSecurityStackSetSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virStorageSourcePtr src) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetImageLabel(item->securityManager, vm, src) < 0) + rc = -1; + } + + return rc; +} + +static int +virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virStorageSourcePtr src) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerRestoreImageLabel(item->securityManager, + vm, src) < 0) + rc = -1; + } + + return rc; +} + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -581,6 +616,9 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityDiskLabel = virSecurityStackSetSecurityDiskLabel, .domainRestoreSecurityDiskLabel = virSecurityStackRestoreSecurityDiskLabel, + .domainSetSecurityImageLabel = virSecurityStackSetSecurityImageLabel, + .domainRestoreSecurityImageLabel = virSecurityStackRestoreSecurityImageLabel, + .domainSetSecurityDaemonSocketLabel = virSecurityStackSetDaemonSocketLabel, .domainSetSecuritySocketLabel = virSecurityStackSetSocketLabel, .domainClearSecuritySocketLabel = virSecurityStackClearSocketLabel, -- 1.9.3

Refactor the code and reuse it to implement the functionality. --- src/security/security_selinux.c | 92 ++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 38 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 7b534b2..97f91f7 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -56,9 +56,6 @@ VIR_LOG_INIT("security.security_selinux"); typedef struct _virSecuritySELinuxData virSecuritySELinuxData; typedef virSecuritySELinuxData *virSecuritySELinuxDataPtr; -typedef struct _virSecuritySELinuxCallbackData virSecuritySELinuxCallbackData; -typedef virSecuritySELinuxCallbackData *virSecuritySELinuxCallbackDataPtr; - struct _virSecuritySELinuxData { char *domain_context; char *alt_domain_context; @@ -71,11 +68,6 @@ struct _virSecuritySELinuxData { #endif }; -struct _virSecuritySELinuxCallbackData { - virSecurityManagerPtr manager; - virSecurityLabelDefPtr secdef; -}; - #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux" @@ -1199,40 +1191,50 @@ virSecuritySELinuxRestoreSecurityImageLabel(virSecurityManagerPtr mgr, static int -virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, - const char *path, - size_t depth, - void *opaque) +virSecuritySELinuxSetSecurityImageLabelInternal(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src, + bool first) { - int ret; + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; virSecurityDeviceLabelDefPtr disk_seclabel; - virSecuritySELinuxCallbackDataPtr cbdata = opaque; - virSecurityLabelDefPtr secdef = cbdata->secdef; - virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(cbdata->manager); + int ret; + + if (!src->path || + virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK) + return 0; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (!secdef || secdef->norelabel) + return 0; - disk_seclabel = virStorageSourceGetSecurityLabelDef(disk->src, + disk_seclabel = virStorageSourceGetSecurityLabelDef(src, SECURITY_SELINUX_NAME); if (disk_seclabel && disk_seclabel->norelabel) return 0; - if (disk_seclabel && !disk_seclabel->norelabel && - disk_seclabel->label) { - ret = virSecuritySELinuxSetFilecon(path, disk_seclabel->label); - } else if (depth == 0) { - - if (disk->src->shared) { - ret = virSecuritySELinuxSetFileconOptional(path, data->file_context); - } else if (disk->src->readonly) { - ret = virSecuritySELinuxSetFileconOptional(path, data->content_context); + if (disk_seclabel && !disk_seclabel->norelabel && disk_seclabel->label) { + ret = virSecuritySELinuxSetFilecon(src->path, disk_seclabel->label); + } else if (first) { + if (src->shared) { + ret = virSecuritySELinuxSetFileconOptional(src->path, + data->file_context); + } else if (src->readonly) { + ret = virSecuritySELinuxSetFileconOptional(src->path, + data->content_context); } else if (secdef->imagelabel) { - ret = virSecuritySELinuxSetFileconOptional(path, secdef->imagelabel); + ret = virSecuritySELinuxSetFileconOptional(src->path, + secdef->imagelabel); } else { ret = 0; } } else { - ret = virSecuritySELinuxSetFileconOptional(path, data->content_context); + ret = virSecuritySELinuxSetFileconOptional(src->path, + data->content_context); } + if (ret == 1 && !disk_seclabel) { /* If we failed to set a label, but virt_use_nfs let us * proceed anyway, then we don't need to relabel later. */ @@ -1240,35 +1242,48 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, if (!disk_seclabel) return -1; disk_seclabel->labelskip = true; - if (VIR_APPEND_ELEMENT(disk->src->seclabels, disk->src->nseclabels, + if (VIR_APPEND_ELEMENT(src->seclabels, src->nseclabels, disk_seclabel) < 0) { virSecurityDeviceLabelDefFree(disk_seclabel); return -1; } ret = 0; } + return ret; } + +static int +virSecuritySELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src) +{ + return virSecuritySELinuxSetSecurityImageLabelInternal(mgr, def, src, true); +} + + static int virSecuritySELinuxSetSecurityDiskLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk) { - virSecuritySELinuxCallbackData cbdata; - cbdata.manager = mgr; - cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + bool first = true; + virStorageSourcePtr next; - if (!cbdata.secdef || cbdata.secdef->norelabel) - return 0; + for (next = disk->src; next; next = next->backingStore) { + if (virSecuritySELinuxSetSecurityImageLabelInternal(mgr, def, next, + first) < 0) + return -1; - return virDomainDiskDefForeachPath(disk, - true, - virSecuritySELinuxSetSecurityFileLabel, - &cbdata); + first = false; + } + + return 0; } + static int virSecuritySELinuxSetSecurityHostdevLabelHelper(const char *file, void *opaque) { @@ -2437,6 +2452,7 @@ virSecurityDriver virSecurityDriverSELinux = { .domainSetSecurityDiskLabel = virSecuritySELinuxSetSecurityDiskLabel, .domainRestoreSecurityDiskLabel = virSecuritySELinuxRestoreSecurityDiskLabel, + .domainSetSecurityImageLabel = virSecuritySELinuxSetSecurityImageLabel, .domainRestoreSecurityImageLabel = virSecuritySELinuxRestoreSecurityImageLabel, .domainSetSecurityDaemonSocketLabel = virSecuritySELinuxSetSecurityDaemonSocketLabel, -- 1.9.3

Refactor the code and reuse it to implement the functionality. --- src/security/security_dac.c | 53 ++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 69b51c1..3ff7817 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -289,22 +289,30 @@ virSecurityDACRestoreSecurityFileLabel(const char *path) static int -virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk, - const char *path, - size_t depth ATTRIBUTE_UNUSED, - void *opaque) +virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src) { - virSecurityDACCallbackDataPtr cbdata = opaque; - virSecurityManagerPtr mgr = cbdata->manager; - virSecurityLabelDefPtr secdef = cbdata->secdef; - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; virSecurityDeviceLabelDefPtr disk_seclabel; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); uid_t user; gid_t group; - disk_seclabel = virStorageSourceGetSecurityLabelDef(disk->src, - SECURITY_DAC_NAME); + if (!priv->dynamicOwnership) + return 0; + + /* XXX: Add support for gluster DAC permissions */ + if (!src->path || + virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK) + return 0; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (secdef && secdef->norelabel) + return 0; + disk_seclabel = virStorageSourceGetSecurityLabelDef(src, + SECURITY_DAC_NAME); if (disk_seclabel && disk_seclabel->norelabel) return 0; @@ -316,7 +324,7 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk, return -1; } - return virSecurityDACSetOwnership(path, user, group); + return virSecurityDACSetOwnership(src->path, user, group); } @@ -326,24 +334,14 @@ virSecurityDACSetSecurityDiskLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - virSecurityDACCallbackData cbdata; - virSecurityLabelDefPtr secdef; + virStorageSourcePtr next; - if (!priv->dynamicOwnership) - return 0; - - secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - - if (secdef && secdef->norelabel) - return 0; + for (next = disk->src; next; next = next->backingStore) { + if (virSecurityDACSetSecurityImageLabel(mgr, def, next) < 0) + return -1; + } - cbdata.manager = mgr; - cbdata.secdef = secdef; - return virDomainDiskDefForeachPath(disk, - false, - virSecurityDACSetSecurityFileLabel, - &cbdata); + return 0; } @@ -1278,6 +1276,7 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityDiskLabel = virSecurityDACSetSecurityDiskLabel, .domainRestoreSecurityDiskLabel = virSecurityDACRestoreSecurityDiskLabel, + .domainSetSecurityImageLabel = virSecurityDACSetSecurityImageLabel, .domainRestoreSecurityImageLabel = virSecurityDACRestoreSecurityImageLabel, .domainSetSecurityDaemonSocketLabel = virSecurityDACSetDaemonSocketLabel, -- 1.9.3

Refactor the code and reuse it to implement the functionality. --- src/security/security_apparmor.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 72d1e16..fb41c5a 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -704,41 +704,40 @@ AppArmorRestoreSecurityDiskLabel(virSecurityManagerPtr mgr, /* Called when hotplugging */ static int -AppArmorSetSecurityDiskLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def, virDomainDiskDefPtr disk) +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src) { int rc = -1; char *profile_name = NULL; - virSecurityLabelDefPtr secdef = - virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + virSecurityLabelDefPtr secdef; - if (!secdef) + if (!src->path || + virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK) + return 0; + + if (!(secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME))) return -1; if (secdef->norelabel) return 0; - if (!virDomainDiskGetSource(disk) || - virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) - return 0; - if (secdef->imagelabel) { /* if the device doesn't exist, error out */ - if (!virFileExists(virDomainDiskGetSource(disk))) { + if (!virFileExists(src->path)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("\'%s\' does not exist"), - virDomainDiskGetSource(disk)); - return rc; + src->path); + return -1; } if ((profile_name = get_profile_name(def)) == NULL) - return rc; + return -1; /* update the profile only if it is loaded */ if (profile_loaded(secdef->imagelabel) >= 0) { if (load_profile(mgr, secdef->imagelabel, def, - virDomainDiskGetSource(disk), - false) < 0) { + src->path, false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " "\'%s\'"), @@ -756,6 +755,14 @@ AppArmorSetSecurityDiskLabel(virSecurityManagerPtr mgr, } static int +AppArmorSetSecurityDiskLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainDiskDefPtr disk) +{ + return AppArmorSetSecurityImageLabel(mgr, def, disk->src); +} + +static int AppArmorSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { @@ -983,6 +990,7 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityDiskLabel = AppArmorSetSecurityDiskLabel, .domainRestoreSecurityDiskLabel = AppArmorRestoreSecurityDiskLabel, + .domainSetSecurityImageLabel = AppArmorSetSecurityImageLabel, .domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel, .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, -- 1.9.3

Instead of just returning the parent path, return the complete parent source structure. --- src/qemu/qemu_driver.c | 16 ++++----- src/util/virstoragefile.c | 17 ++++------ src/util/virstoragefile.h | 2 +- tests/virstoragetest.c | 86 ++++++++++++++++++++++------------------------- 4 files changed, 56 insertions(+), 65 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 552e595..4546862 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15488,7 +15488,7 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned int topIndex = 0; virStorageSourcePtr baseSource; unsigned int baseIndex = 0; - const char *top_parent = NULL; + virStorageSourcePtr top_parent = NULL; bool clean_access = false; /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */ @@ -15596,10 +15596,9 @@ qemuDomainBlockCommit(virDomainPtr dom, clean_access = true; if (qemuDomainPrepareDiskChainElement(driver, vm, disk, baseSource, VIR_DISK_CHAIN_READ_WRITE) < 0 || - (top_parent && top_parent != disk->src->path && - qemuDomainPrepareDiskChainElementPath(driver, vm, disk, - top_parent, - VIR_DISK_CHAIN_READ_WRITE) < 0)) + (top_parent != disk->src && + qemuDomainPrepareDiskChainElement(driver, vm, disk, top_parent, + VIR_DISK_CHAIN_READ_WRITE) < 0)) goto endjob; /* Start the commit operation. Pass the user's original spelling, @@ -15619,10 +15618,9 @@ qemuDomainBlockCommit(virDomainPtr dom, /* Revert access to read-only, if possible. */ qemuDomainPrepareDiskChainElement(driver, vm, disk, baseSource, VIR_DISK_CHAIN_READ_ONLY); - if (top_parent && top_parent != disk->src->path) - qemuDomainPrepareDiskChainElementPath(driver, vm, disk, - top_parent, - VIR_DISK_CHAIN_READ_ONLY); + if (top_parent != disk->src) + qemuDomainPrepareDiskChainElement(driver, vm, disk, top_parent, + VIR_DISK_CHAIN_READ_ONLY); } if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index e154f92..9a7f997 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1337,17 +1337,16 @@ virStorageFileChainLookup(virStorageSourcePtr chain, virStorageSourcePtr startFrom, const char *name, unsigned int idx, - const char **parent) + virStorageSourcePtr *parent) { - virStorageSourcePtr prev = NULL; + virStorageSourcePtr prev; const char *start = chain->path; - const char *tmp; char *parentDir = NULL; bool nameIsFile = virStorageIsFile(name); size_t i = 0; if (!parent) - parent = &tmp; + parent = &prev; *parent = NULL; if (startFrom) { @@ -1355,7 +1354,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain, chain = chain->backingStore; i++; } - *parent = startFrom->path; + *parent = startFrom; } while (chain) { @@ -1372,8 +1371,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain, break; if (nameIsFile && virStorageSourceIsLocalStorage(chain)) { - if (prev && virStorageSourceIsLocalStorage(prev)) - parentDir = mdir_name(prev->path); + if (*parent && virStorageSourceIsLocalStorage(*parent)) + parentDir = mdir_name((*parent)->path); else ignore_value(VIR_STRDUP(parentDir, ".")); @@ -1382,7 +1381,6 @@ virStorageFileChainLookup(virStorageSourcePtr chain, goto error; } - int result = virFileRelLinkPointsTo(parentDir, name, chain->path); @@ -1395,8 +1393,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain, break; } } - *parent = chain->path; - prev = chain; + *parent = chain; chain = chain->backingStore; i++; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index ee85ca5..6cbcd6b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -297,7 +297,7 @@ virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain, virStorageSourcePtr startFrom, const char *name, unsigned int idx, - const char **parent) + virStorageSourcePtr *parent) ATTRIBUTE_NONNULL(1); int virStorageFileResize(const char *path, diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index fb2837f..e2ee3ff 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -415,7 +415,7 @@ struct testLookupData unsigned int expIndex; const char *expResult; virStorageSourcePtr expMeta; - const char *expParent; + virStorageSourcePtr expParent; }; static int @@ -424,7 +424,7 @@ testStorageLookup(const void *args) const struct testLookupData *data = args; int ret = 0; virStorageSourcePtr result; - const char *actualParent; + virStorageSourcePtr actualParent; unsigned int idx; if (virStorageFileParseChainIndex(data->target, data->name, &idx) < 0 && @@ -488,9 +488,10 @@ testStorageLookup(const void *args) data->expMeta, result); ret = -1; } - if (STRNEQ_NULLABLE(data->expParent, actualParent)) { + if (data->expParent != actualParent) { fprintf(stderr, "parent: expected %s, got %s\n", - NULLSTR(data->expParent), NULLSTR(actualParent)); + NULLSTR(data->expParent ? data->expParent->path : NULL), + NULLSTR(actualParent ? actualParent->path : NULL)); ret = -1; } @@ -974,25 +975,25 @@ mymain(void) TEST_LOOKUP(5, NULL, abswrap, chain->path, chain, NULL); TEST_LOOKUP(6, chain, abswrap, NULL, NULL, NULL); TEST_LOOKUP(7, chain2, abswrap, NULL, NULL, NULL); - TEST_LOOKUP(8, NULL, "qcow2", chain2->path, chain2, chain->path); - TEST_LOOKUP(9, chain, "qcow2", chain2->path, chain2, chain->path); + TEST_LOOKUP(8, NULL, "qcow2", chain2->path, chain2, chain); + TEST_LOOKUP(9, chain, "qcow2", chain2->path, chain2, chain); TEST_LOOKUP(10, chain2, "qcow2", NULL, NULL, NULL); TEST_LOOKUP(11, chain3, "qcow2", NULL, NULL, NULL); - TEST_LOOKUP(12, NULL, absqcow2, chain2->path, chain2, chain->path); - TEST_LOOKUP(13, chain, absqcow2, chain2->path, chain2, chain->path); + TEST_LOOKUP(12, NULL, absqcow2, chain2->path, chain2, chain); + TEST_LOOKUP(13, chain, absqcow2, chain2->path, chain2, chain); TEST_LOOKUP(14, chain2, absqcow2, NULL, NULL, NULL); TEST_LOOKUP(15, chain3, absqcow2, NULL, NULL, NULL); - TEST_LOOKUP(16, NULL, "raw", chain3->path, chain3, chain2->path); - TEST_LOOKUP(17, chain, "raw", chain3->path, chain3, chain2->path); - TEST_LOOKUP(18, chain2, "raw", chain3->path, chain3, chain2->path); + TEST_LOOKUP(16, NULL, "raw", chain3->path, chain3, chain2); + TEST_LOOKUP(17, chain, "raw", chain3->path, chain3, chain2); + TEST_LOOKUP(18, chain2, "raw", chain3->path, chain3, chain2); TEST_LOOKUP(19, chain3, "raw", NULL, NULL, NULL); - TEST_LOOKUP(20, NULL, absraw, chain3->path, chain3, chain2->path); - TEST_LOOKUP(21, chain, absraw, chain3->path, chain3, chain2->path); - TEST_LOOKUP(22, chain2, absraw, chain3->path, chain3, chain2->path); + TEST_LOOKUP(20, NULL, absraw, chain3->path, chain3, chain2); + TEST_LOOKUP(21, chain, absraw, chain3->path, chain3, chain2); + TEST_LOOKUP(22, chain2, absraw, chain3->path, chain3, chain2); TEST_LOOKUP(23, chain3, absraw, NULL, NULL, NULL); - TEST_LOOKUP(24, NULL, NULL, chain3->path, chain3, chain2->path); - TEST_LOOKUP(25, chain, NULL, chain3->path, chain3, chain2->path); - TEST_LOOKUP(26, chain2, NULL, chain3->path, chain3, chain2->path); + TEST_LOOKUP(24, NULL, NULL, chain3->path, chain3, chain2); + TEST_LOOKUP(25, chain, NULL, chain3->path, chain3, chain2); + TEST_LOOKUP(26, chain2, NULL, chain3->path, chain3, chain2); TEST_LOOKUP(27, chain3, NULL, NULL, NULL, NULL); /* Rewrite wrap and qcow2 back to 3-deep chain, relative backing */ @@ -1027,25 +1028,25 @@ mymain(void) TEST_LOOKUP(33, NULL, abswrap, chain->path, chain, NULL); TEST_LOOKUP(34, chain, abswrap, NULL, NULL, NULL); TEST_LOOKUP(35, chain2, abswrap, NULL, NULL, NULL); - TEST_LOOKUP(36, NULL, "qcow2", chain2->path, chain2, chain->path); - TEST_LOOKUP(37, chain, "qcow2", chain2->path, chain2, chain->path); + TEST_LOOKUP(36, NULL, "qcow2", chain2->path, chain2, chain); + TEST_LOOKUP(37, chain, "qcow2", chain2->path, chain2, chain); TEST_LOOKUP(38, chain2, "qcow2", NULL, NULL, NULL); TEST_LOOKUP(39, chain3, "qcow2", NULL, NULL, NULL); - TEST_LOOKUP(40, NULL, absqcow2, chain2->path, chain2, chain->path); - TEST_LOOKUP(41, chain, absqcow2, chain2->path, chain2, chain->path); + TEST_LOOKUP(40, NULL, absqcow2, chain2->path, chain2, chain); + TEST_LOOKUP(41, chain, absqcow2, chain2->path, chain2, chain); TEST_LOOKUP(42, chain2, absqcow2, NULL, NULL, NULL); TEST_LOOKUP(43, chain3, absqcow2, NULL, NULL, NULL); - TEST_LOOKUP(44, NULL, "raw", chain3->path, chain3, chain2->path); - TEST_LOOKUP(45, chain, "raw", chain3->path, chain3, chain2->path); - TEST_LOOKUP(46, chain2, "raw", chain3->path, chain3, chain2->path); + TEST_LOOKUP(44, NULL, "raw", chain3->path, chain3, chain2); + TEST_LOOKUP(45, chain, "raw", chain3->path, chain3, chain2); + TEST_LOOKUP(46, chain2, "raw", chain3->path, chain3, chain2); TEST_LOOKUP(47, chain3, "raw", NULL, NULL, NULL); - TEST_LOOKUP(48, NULL, absraw, chain3->path, chain3, chain2->path); - TEST_LOOKUP(49, chain, absraw, chain3->path, chain3, chain2->path); - TEST_LOOKUP(50, chain2, absraw, chain3->path, chain3, chain2->path); + TEST_LOOKUP(48, NULL, absraw, chain3->path, chain3, chain2); + TEST_LOOKUP(49, chain, absraw, chain3->path, chain3, chain2); + TEST_LOOKUP(50, chain2, absraw, chain3->path, chain3, chain2); TEST_LOOKUP(51, chain3, absraw, NULL, NULL, NULL); - TEST_LOOKUP(52, NULL, NULL, chain3->path, chain3, chain2->path); - TEST_LOOKUP(53, chain, NULL, chain3->path, chain3, chain2->path); - TEST_LOOKUP(54, chain2, NULL, chain3->path, chain3, chain2->path); + TEST_LOOKUP(52, NULL, NULL, chain3->path, chain3, chain2); + TEST_LOOKUP(53, chain, NULL, chain3->path, chain3, chain2); + TEST_LOOKUP(54, chain2, NULL, chain3->path, chain3, chain2); TEST_LOOKUP(55, chain3, NULL, NULL, NULL, NULL); /* Use link to wrap with cross-directory relative backing */ @@ -1070,12 +1071,12 @@ mymain(void) TEST_LOOKUP(57, NULL, "sub/link2", chain->path, chain, NULL); TEST_LOOKUP(58, NULL, "wrap", chain->path, chain, NULL); TEST_LOOKUP(59, NULL, abswrap, chain->path, chain, NULL); - TEST_LOOKUP(60, NULL, "../qcow2", chain2->path, chain2, chain->path); + TEST_LOOKUP(60, NULL, "../qcow2", chain2->path, chain2, chain); TEST_LOOKUP(61, NULL, "qcow2", NULL, NULL, NULL); - TEST_LOOKUP(62, NULL, absqcow2, chain2->path, chain2, chain->path); - TEST_LOOKUP(63, NULL, "raw", chain3->path, chain3, chain2->path); - TEST_LOOKUP(64, NULL, absraw, chain3->path, chain3, chain2->path); - TEST_LOOKUP(65, NULL, NULL, chain3->path, chain3, chain2->path); + TEST_LOOKUP(62, NULL, absqcow2, chain2->path, chain2, chain); + TEST_LOOKUP(63, NULL, "raw", chain3->path, chain3, chain2); + TEST_LOOKUP(64, NULL, absraw, chain3->path, chain3, chain2); + TEST_LOOKUP(65, NULL, NULL, chain3->path, chain3, chain2); TEST_LOOKUP_TARGET(66, "vda", NULL, "bogus[1]", 0, NULL, NULL, NULL); TEST_LOOKUP_TARGET(67, "vda", NULL, "vda[-1]", 0, NULL, NULL, NULL); @@ -1084,18 +1085,13 @@ mymain(void) TEST_LOOKUP_TARGET(70, "vda", chain, "wrap", 0, NULL, NULL, NULL); TEST_LOOKUP_TARGET(71, "vda", chain2, "wrap", 0, NULL, NULL, NULL); TEST_LOOKUP_TARGET(72, "vda", NULL, "vda[0]", 0, NULL, NULL, NULL); - TEST_LOOKUP_TARGET(73, "vda", NULL, "vda[1]", 1, chain2->path, chain2, - chain->path); - TEST_LOOKUP_TARGET(74, "vda", chain, "vda[1]", 1, chain2->path, chain2, - chain->path); + TEST_LOOKUP_TARGET(73, "vda", NULL, "vda[1]", 1, chain2->path, chain2, chain); + TEST_LOOKUP_TARGET(74, "vda", chain, "vda[1]", 1, chain2->path, chain2, chain); TEST_LOOKUP_TARGET(75, "vda", chain2, "vda[1]", 1, NULL, NULL, NULL); TEST_LOOKUP_TARGET(76, "vda", chain3, "vda[1]", 1, NULL, NULL, NULL); - TEST_LOOKUP_TARGET(77, "vda", NULL, "vda[2]", 2, chain3->path, chain3, - chain2->path); - TEST_LOOKUP_TARGET(78, "vda", chain, "vda[2]", 2, chain3->path, chain3, - chain2->path); - TEST_LOOKUP_TARGET(79, "vda", chain2, "vda[2]", 2, chain3->path, chain3, - chain2->path); + TEST_LOOKUP_TARGET(77, "vda", NULL, "vda[2]", 2, chain3->path, chain3, chain2); + TEST_LOOKUP_TARGET(78, "vda", chain, "vda[2]", 2, chain3->path, chain3, chain2); + TEST_LOOKUP_TARGET(79, "vda", chain2, "vda[2]", 2, chain3->path, chain3, chain2); TEST_LOOKUP_TARGET(80, "vda", chain3, "vda[2]", 2, NULL, NULL, NULL); TEST_LOOKUP_TARGET(81, "vda", NULL, "vda[3]", 3, NULL, NULL, NULL); -- 1.9.3

When creating a new disk mirror the new struct is stored in a separate variable until everything went well. The removed hunk would actually remove existing mirror information for example when the api would be run if a mirror still exists. --- src/qemu/qemu_driver.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 66752f1..f6f5ace 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15382,10 +15382,6 @@ qemuDomainBlockCopy(virDomainObjPtr vm, endjob: if (need_unlink && unlink(dest)) VIR_WARN("unable to unlink just-created %s", dest); - if (ret < 0 && disk) { - virStorageSourceFree(disk->mirror); - disk->mirror = NULL; - } virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; -- 1.9.3

On 06/26/14 05:29, Eric Blake wrote: I've pushed this one and 1-7 of this series as they are trivial enough to get in during the freeze. I'm not sure though about the other changes. Peter