[libvirt] [PATCH 0/8] Add support for setting DAC permissions on remote filesystems
Peter Krempa (8): storage: Implement storage driver helper to chown disk images storage: Add witness for checking storage volume use in security driver security: DAC: Remove superfluous link resolution security: DAC: Introduce callback to perform image chown security: DAC: Plumb usage of chown callback qemu: Implement DAC driver chown callback to co-operate with storage drv storage: Implement virStorageFileCreate for local and gluster files qemu: snapshot: Use storage driver to pre-create snapshot file src/qemu/qemu_driver.c | 66 ++++++++++++++++---- src/security/security_dac.c | 109 +++++++++++++++++++++++----------- src/security/security_dac.h | 3 + src/security/security_manager.c | 4 +- src/security/security_manager.h | 19 +++++- src/storage/storage_backend.h | 6 ++ src/storage/storage_backend_fs.c | 29 +++++++++ src/storage/storage_backend_gluster.c | 27 +++++++++ src/storage/storage_driver.c | 58 ++++++++++++++++++ src/storage/storage_driver.h | 3 + 10 files changed, 277 insertions(+), 47 deletions(-) -- 2.0.0
Gluster storage works on a similar principle to NFS where it takes the uid and gid of the actual process and uses it to access the storage volume on the remote server. This introduces a need to chown storage files on gluster via native API. --- src/storage/storage_backend.h | 6 ++++++ src/storage/storage_backend_fs.c | 12 ++++++++++++ src/storage/storage_backend_gluster.c | 12 ++++++++++++ src/storage/storage_driver.c | 28 ++++++++++++++++++++++++++++ src/storage/storage_driver.h | 1 + 5 files changed, 59 insertions(+) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 76c1afa..1a80aef 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -202,6 +202,11 @@ typedef int (*virStorageFileBackendAccess)(virStorageSourcePtr src, int mode); +typedef int +(*virStorageFileBackendChown)(virStorageSourcePtr src, + uid_t uid, + gid_t gid); + virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol); virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type, int protocol, @@ -227,6 +232,7 @@ struct _virStorageFileBackend { virStorageFileBackendUnlink storageFileUnlink; virStorageFileBackendStat storageFileStat; virStorageFileBackendAccess storageFileAccess; + virStorageFileBackendChown storageFileChown; }; #endif /* __VIR_STORAGE_BACKEND_H__ */ diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index c93fc1e..693ebd3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1435,6 +1435,15 @@ virStorageFileBackendFileAccess(virStorageSourcePtr src, } +static int +virStorageFileBackendFileChown(virStorageSourcePtr src, + uid_t uid, + gid_t gid) +{ + return chown(src->path, uid, gid); +} + + virStorageFileBackend virStorageFileBackendFile = { .type = VIR_STORAGE_TYPE_FILE, @@ -1445,6 +1454,7 @@ virStorageFileBackend virStorageFileBackendFile = { .storageFileStat = virStorageFileBackendFileStat, .storageFileReadHeader = virStorageFileBackendFileReadHeader, .storageFileAccess = virStorageFileBackendFileAccess, + .storageFileChown = virStorageFileBackendFileChown, .storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, }; @@ -1459,6 +1469,7 @@ virStorageFileBackend virStorageFileBackendBlock = { .storageFileStat = virStorageFileBackendFileStat, .storageFileReadHeader = virStorageFileBackendFileReadHeader, .storageFileAccess = virStorageFileBackendFileAccess, + .storageFileChown = virStorageFileBackendFileChown, .storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, }; @@ -1471,6 +1482,7 @@ virStorageFileBackend virStorageFileBackendDir = { .backendDeinit = virStorageFileBackendFileDeinit, .storageFileAccess = virStorageFileBackendFileAccess, + .storageFileChown = virStorageFileBackendFileChown, .storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, }; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 76d2461..edb0511 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -754,6 +754,17 @@ virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) } +static int +virStorageFileBackendGlusterChown(virStorageSourcePtr src, + uid_t uid, + gid_t gid) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + + return glfs_chown(priv->vol, src->path, uid, gid); +} + + virStorageFileBackend virStorageFileBackendGluster = { .type = VIR_STORAGE_TYPE_NETWORK, .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER, @@ -765,6 +776,7 @@ virStorageFileBackend virStorageFileBackendGluster = { .storageFileStat = virStorageFileBackendGlusterStat, .storageFileReadHeader = virStorageFileBackendGlusterReadHeader, .storageFileAccess = virStorageFileBackendGlusterAccess, + .storageFileChown = virStorageFileBackendGlusterChown, .storageFileGetUniqueIdentifier = virStorageFileBackendGlusterGetUniqueIdentifier, diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ae86c69..928fadf 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2823,6 +2823,34 @@ virStorageFileAccess(virStorageSourcePtr src, } +/** + * virStorageFileChown: Change owner of a storage file + * + * @src: storage file to change owner of + * @uid: new owner id + * @gid: new group id + * + * Returns 0 on success, -1 on error and sets errno. No libvirt + * error is reported. Returns -2 if the operation isn't supported + * by libvirt storage backend. + */ +int +virStorageFileChown(virStorageSourcePtr src, + uid_t uid, + gid_t gid) +{ + if (!virStorageFileIsInitialized(src) || + !src->drv->backend->storageFileChown) { + errno = ENOSYS; + return -2; + } + + VIR_DEBUG("chown of storage file %p to %d:%d", src, uid, gid); + + return src->drv->backend->storageFileChown(src, uid, gid); +} + + /* Recursive workhorse for virStorageFileGetMetadata. */ static int virStorageFileGetMetadataRecurse(virStorageSourcePtr src, diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index bd9e9ed..eefd766 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -43,6 +43,7 @@ ssize_t virStorageFileReadHeader(virStorageSourcePtr src, char **buf); const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src); int virStorageFileAccess(virStorageSourcePtr src, int mode); +int virStorageFileChown(virStorageSourcePtr src, uid_t uid, gid_t gid); int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, -- 2.0.0
With my intended use of storage driver assist to chown files on remote storage we will need a witness that will tell us whether the given storage volume supports operations needed by the storage driver. --- src/storage/storage_driver.c | 30 ++++++++++++++++++++++++++++++ src/storage/storage_driver.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 928fadf..fc05a08 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2567,6 +2567,36 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) backend->storageFileAccess; } + +/** + * virStorageFileSupportsSecurityDriver: + * + * @src: a storage file structure + * + * Check if a storage file supports operations needed by the security + * driver to perform labelling */ +bool +virStorageFileSupportsSecurityDriver(virStorageSourcePtr src) +{ + int actualType = virStorageSourceGetActualType(src); + virStorageFileBackendPtr backend; + + if (!src) + return false; + + if (src->drv) { + backend = src->drv->backend; + } else { + if (!(backend = virStorageFileBackendForTypeInternal(actualType, + src->protocol, + false))) + return false; + } + + return !!backend->storageFileChown; +} + + void virStorageFileDeinit(virStorageSourcePtr src) { diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index eefd766..9592dd8 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -45,6 +45,8 @@ const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src); int virStorageFileAccess(virStorageSourcePtr src, int mode); int virStorageFileChown(virStorageSourcePtr src, uid_t uid, gid_t gid); +bool virStorageFileSupportsSecurityDriver(virStorageSourcePtr src); + int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, bool allow_probe) -- 2.0.0
When restoring security labels in the dac driver the code would resolve the file path and use the resolved one to be chown-ed. The setting code doesn't do that. Remove the unnecessary code. --- src/security/security_dac.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 26cd615..6821d37 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -264,27 +264,10 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) static int virSecurityDACRestoreSecurityFileLabel(const char *path) { - struct stat buf; - int rc = -1; - char *newpath = NULL; - VIR_INFO("Restoring DAC user and group on '%s'", path); - if (virFileResolveLink(path, &newpath) < 0) { - virReportSystemError(errno, - _("cannot resolve symlink %s"), path); - goto err; - } - - if (stat(newpath, &buf) != 0) - goto err; - /* XXX record previous ownership */ - rc = virSecurityDACSetOwnership(newpath, 0, 0); - - err: - VIR_FREE(newpath); - return rc; + return virSecurityDACSetOwnership(path, 0, 0); } -- 2.0.0
To integrate the security driver with the storage driver we need to pass a callback for a function that will chown storage volumes. Introduce and document the callback prototype. --- src/qemu/qemu_driver.c | 3 ++- src/security/security_dac.c | 9 +++++++++ src/security/security_dac.h | 3 +++ src/security/security_manager.c | 4 +++- src/security/security_manager.h | 19 ++++++++++++++++++- 5 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..b30c504 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -374,7 +374,8 @@ qemuSecurityInit(virQEMUDriverPtr driver) cfg->allowDiskFormatProbing, cfg->securityDefaultConfined, cfg->securityRequireConfined, - cfg->dynamicOwnership))) + cfg->dynamicOwnership, + NULL))) goto error; if (!stack) { if (!(stack = virSecurityManagerNewStack(mgr))) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 6821d37..eafd714 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -51,6 +51,7 @@ struct _virSecurityDACData { int ngroups; bool dynamicOwnership; char *baselabel; + virSecurityManagerDACChownCallback chownCallback; }; typedef struct _virSecurityDACCallbackData virSecurityDACCallbackData; @@ -87,6 +88,14 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, priv->dynamicOwnership = dynamicOwnership; } +void +virSecurityDACSetChownCallback(virSecurityManagerPtr mgr, + virSecurityManagerDACChownCallback chownCallback) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + priv->chownCallback = chownCallback; +} + /* returns 1 if label isn't found, 0 on success, -1 on error */ static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) diff --git a/src/security/security_dac.h b/src/security/security_dac.h index dbcf56f..846cefb 100644 --- a/src/security/security_dac.h +++ b/src/security/security_dac.h @@ -32,4 +32,7 @@ int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, bool dynamic); +void virSecurityDACSetChownCallback(virSecurityManagerPtr mgr, + virSecurityManagerDACChownCallback chownCallback); + #endif /* __VIR_SECURITY_DAC */ diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 16bec5c..320dde4 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -152,7 +152,8 @@ virSecurityManagerNewDAC(const char *virtDriver, bool allowDiskFormatProbing, bool defaultConfined, bool requireConfined, - bool dynamicOwnership) + bool dynamicOwnership, + virSecurityManagerDACChownCallback chownCallback) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC, @@ -170,6 +171,7 @@ virSecurityManagerNewDAC(const char *virtDriver, } virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership); + virSecurityDACSetChownCallback(mgr, chownCallback); return mgr; } diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 97b6a2e..156f882 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -25,6 +25,7 @@ # include "domain_conf.h" # include "vircommand.h" +# include "virstoragefile.h" typedef struct _virSecurityManager virSecurityManager; typedef virSecurityManager *virSecurityManagerPtr; @@ -39,13 +40,29 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary); int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, virSecurityManagerPtr nested); +/** + * virSecurityManagerDACChownCallback: + * @src: Storage file to chown + * @uid: target uid + * @gid: target gid + * + * A function callback to chown image files described by the disk source struct + * @src. The callback shall return 0 on success, -1 on error and errno set (no + * libvirt error reported) OR -2 and a libvirt error reported. */ +typedef int +(*virSecurityManagerDACChownCallback)(virStorageSourcePtr src, + uid_t uid, + gid_t gid); + + virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, uid_t user, gid_t group, bool allowDiskFormatProbing, bool defaultConfined, bool requireConfined, - bool dynamicOwnership); + bool dynamicOwnership, + virSecurityManagerDACChownCallback chownCallback); int virSecurityManagerPreFork(virSecurityManagerPtr mgr); void virSecurityManagerPostFork(virSecurityManagerPtr mgr); -- 2.0.0
Use the callback to set disk and storage image labels by modifying the existing functions and adding wrappers to avoid refactoring a lot of the code. --- src/security/security_dac.c | 89 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 20 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index eafd714..0ad024c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -230,23 +230,54 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr) } static int -virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) +virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv, + virStorageSourcePtr src, + const char *path, + uid_t uid, + gid_t gid) { + int rc; + int chown_errno; + VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", - path, (long) uid, (long) gid); + NULLSTR(src ? src->path : path), (long) uid, (long) gid); + + if (priv && src && priv->chownCallback) { + rc = priv->chownCallback(src, uid, gid); + + /* on -2 returned an error was already reported */ + if (rc == -2) + return -1; - if (chown(path, uid, gid) < 0) { + /* on -1 only errno was set */ + chown_errno = errno; + } else { struct stat sb; - int chown_errno = errno; - if (stat(path, &sb) >= 0) { + if (!path) { + if (!src || !src->path) + return 0; + + if (!virStorageSourceIsLocalStorage(src)) + return 0; + + path = src->path; + } + + rc = chown(path, uid, gid); + chown_errno = errno; + + if (rc < 0 && + stat(path, &sb) >= 0) { if (sb.st_uid == uid && sb.st_gid == gid) { /* It's alright, there's nothing to change anyway. */ return 0; } } + } + if (rc < 0) { if (chown_errno == EOPNOTSUPP || chown_errno == EINVAL) { VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " "supported by filesystem", @@ -270,13 +301,31 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) return 0; } + static int -virSecurityDACRestoreSecurityFileLabel(const char *path) +virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) +{ + return virSecurityDACSetOwnershipInternal(NULL, NULL, path, uid, gid); +} + + +static int +virSecurityDACRestoreSecurityFileLabelInternal(virSecurityDACDataPtr priv, + virStorageSourcePtr src, + const char *path) { - VIR_INFO("Restoring DAC user and group on '%s'", path); + VIR_INFO("Restoring DAC user and group on '%s'", + NULLSTR(src ? src->path : path)); /* XXX record previous ownership */ - return virSecurityDACSetOwnership(path, 0, 0); + return virSecurityDACSetOwnershipInternal(priv, src, path, 0, 0); +} + + +static int +virSecurityDACRestoreSecurityFileLabel(const char *path) +{ + return virSecurityDACRestoreSecurityFileLabelInternal(NULL, NULL, path); } @@ -294,10 +343,6 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!priv->dynamicOwnership) return 0; - /* XXX: Add support for gluster DAC permissions */ - if (!src->path || !virStorageSourceIsLocalStorage(src)) - return 0; - secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); if (secdef && secdef->norelabel) return 0; @@ -315,7 +360,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, return -1; } - return virSecurityDACSetOwnership(src->path, user, group); + return virSecurityDACSetOwnershipInternal(priv, src, NULL, user, group); } @@ -349,9 +394,6 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, if (!priv->dynamicOwnership) return 0; - if (!src->path || !virStorageSourceIsLocalStorage(src)) - return 0; - /* Don't restore labels on readoly/shared disks, because other VMs may * still be accessing these. Alternatively we could iterate over all * running domains and try to figure out if it is in use, but this would @@ -373,9 +415,16 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * ownership, because that kills access on the destination host which is * sub-optimal for the guest VM's I/O attempts :-) */ if (migrated) { - int rc = virFileIsSharedFS(src->path); - if (rc < 0) - return -1; + int rc = 1; + + if (virStorageSourceIsLocalStorage(src)) { + if (!src->path) + return 0; + + if ((rc = virFileIsSharedFS(src->path)) < 0) + return -1; + } + if (rc == 1) { VIR_DEBUG("Skipping image label restore on %s because FS is shared", src->path); @@ -383,7 +432,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, } } - return virSecurityDACRestoreSecurityFileLabel(src->path); + return virSecurityDACRestoreSecurityFileLabelInternal(priv, src, NULL); } -- 2.0.0
Use the storage driver to chown remote images. --- src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b30c504..9b3c695 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -327,6 +327,52 @@ qemuAutostartDomains(virQEMUDriverPtr driver) virObjectUnref(cfg); } + +static int +qemuSecurityChownCallback(virStorageSourcePtr src, + uid_t uid, + gid_t gid) +{ + struct stat sb; + int save_errno = 0; + int ret = -1; + + if (!virStorageFileSupportsSecurityDriver(src)) + return 0; + + if (virStorageSourceIsLocalStorage(src)) { + /* use direct chmod for local files so that the file doesn't + * need to be initialized */ + if (stat(src->path, &sb) >= 0) { + if (sb.st_uid == uid && + sb.st_gid == gid) { + /* It's alright, there's nothing to change anyway. */ + return 0; + } + } + + return chown(src->path, uid, gid); + } + + /* storage file init reports errors, return -2 on failure */ + if (virStorageFileInit(src) < 0) + return -2; + + if (virStorageFileChown(src, uid, gid) < 0) { + save_errno = errno; + goto cleanup; + } + + ret = 0; + + cleanup: + virStorageFileDeinit(src); + errno = save_errno; + + return ret; +} + + static int qemuSecurityInit(virQEMUDriverPtr driver) { @@ -375,7 +421,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) cfg->securityDefaultConfined, cfg->securityRequireConfined, cfg->dynamicOwnership, - NULL))) + qemuSecurityChownCallback))) goto error; if (!stack) { if (!(stack = virSecurityManagerNewStack(mgr))) -- 2.0.0
Add backends for this frontend function so that we can use it in the snapshot creation code. --- src/storage/storage_backend_fs.c | 17 +++++++++++++++++ src/storage/storage_backend_gluster.c | 15 +++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 693ebd3..1705a5a 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1367,6 +1367,22 @@ virStorageFileBackendFileInit(virStorageSourcePtr src) static int +virStorageFileBackendFileCreate(virStorageSourcePtr src) +{ + int fd = -1; + + if ((fd = virFileOpenAs(src->path, O_WRONLY | O_TRUNC | O_CREAT, 0, + src->drv->uid, src->drv->gid, 0)) < 0) { + errno = -fd; + return -1; + } + + VIR_FORCE_CLOSE(fd); + return 0; +} + + +static int virStorageFileBackendFileUnlink(virStorageSourcePtr src) { return unlink(src->path); @@ -1450,6 +1466,7 @@ virStorageFileBackend virStorageFileBackendFile = { .backendInit = virStorageFileBackendFileInit, .backendDeinit = virStorageFileBackendFileDeinit, + .storageFileCreate = virStorageFileBackendFileCreate, .storageFileUnlink = virStorageFileBackendFileUnlink, .storageFileStat = virStorageFileBackendFileStat, .storageFileReadHeader = virStorageFileBackendFileReadHeader, diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index edb0511..c2eafd4 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -626,6 +626,20 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) static int +virStorageFileBackendGlusterCreate(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + glfs_fd_t *fd = NULL; + + if (!(fd = glfs_open(priv->vol, src->path, O_CREAT | O_TRUNC | O_WRONLY))) + return -1; + + ignore_value(glfs_close(fd)); + return 0; +} + + +static int virStorageFileBackendGlusterUnlink(virStorageSourcePtr src) { virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; @@ -772,6 +786,7 @@ virStorageFileBackend virStorageFileBackendGluster = { .backendInit = virStorageFileBackendGlusterInit, .backendDeinit = virStorageFileBackendGlusterDeinit, + .storageFileCreate = virStorageFileBackendGlusterCreate, .storageFileUnlink = virStorageFileBackendGlusterUnlink, .storageFileStat = virStorageFileBackendGlusterStat, .storageFileReadHeader = virStorageFileBackendGlusterReadHeader, -- 2.0.0
Move the last operation done on local files to the storage driver API. --- src/qemu/qemu_driver.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9b3c695..a0f49eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12881,7 +12881,6 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, char *source = NULL; const char *formatStr = NULL; int ret = -1; - int fd = -1; bool need_unlink = false; if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { @@ -12899,7 +12898,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, if (virStorageSourceInitChainElement(newDiskSrc, disk->src, false) < 0) goto cleanup; - if (virStorageFileInit(newDiskSrc) < 0) + if (qemuDomainStorageFileInit(driver, vm, newDiskSrc) < 0) goto cleanup; if (qemuGetDriveSourceString(newDiskSrc, NULL, &source) < 0) @@ -12915,15 +12914,13 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } /* pre-create the image file so that we can label it before handing it to qemu */ - /* XXX we should switch to storage driver based pre-creation of the image */ - if (virStorageSourceIsLocalStorage(newDiskSrc)) { - if (!reuse && newDiskSrc->type != VIR_STORAGE_TYPE_BLOCK) { - fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT, - &need_unlink, NULL); - if (fd < 0) - goto cleanup; - VIR_FORCE_CLOSE(fd); + if (!reuse && newDiskSrc->type != VIR_STORAGE_TYPE_BLOCK) { + if (virStorageFileCreate(newDiskSrc) < 0) { + virReportSystemError(errno, _("failed to create image file '%s'"), + source); + goto cleanup; } + need_unlink = true; } /* set correct security, cgroup and locking options on the new image */ -- 2.0.0
On 07/10/14 16:22, Peter Krempa wrote:
Peter Krempa (8): storage: Implement storage driver helper to chown disk images storage: Add witness for checking storage volume use in security driver security: DAC: Remove superfluous link resolution security: DAC: Introduce callback to perform image chown security: DAC: Plumb usage of chown callback qemu: Implement DAC driver chown callback to co-operate with storage drv storage: Implement virStorageFileCreate for local and gluster files qemu: snapshot: Use storage driver to pre-create snapshot file
Ping, could somebody please have a look :) Peter
participants (1)
-
Peter Krempa