[PATCH 8/8] qemu: Deduplicate code in qemuSecurityChownCallback()

The DAC security driver has an option to register a callback that is called instead of chown(). So far QEMU is the only user of this feature and it's used to set labels on non-local disks (like gluster), where exists notion of owners but regular chown() can't be used. However, this callback (if set) is called always, even for local disks. And thus the QEMU's implementation duplicated parts of the DAC driver to deal with chown(). If the DAC driver would call the callback only for non-local disks then the QEMU's callback can be shorter. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 22 ++-------------------- src/security/security_dac.c | 6 ++++-- src/security/security_manager.h | 13 ++++++++++--- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1ee0e7ebc0..235f575901 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -228,31 +228,13 @@ qemuSecurityChownCallback(const virStorageSource *src, uid_t uid, gid_t gid) { - struct stat sb; int save_errno = 0; int ret = -1; int rv; g_autoptr(virStorageSource) cpy = NULL; - if (virStorageSourceIsLocalStorage(src)) { - /* use direct chown for local files so that the file doesn't - * need to be initialized */ - if (!src->path) - return 0; - - 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; - } - } - - if (chown(src->path, uid, gid) < 0) - return -1; - - return 0; - } + if (virStorageSourceIsLocalStorage(src)) + return -3; if ((rv = virStorageSourceSupportsSecurityDriver(src)) <= 0) return rv; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 7ba367755a..4909107fcc 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -672,7 +672,7 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv, uid_t uid, gid_t gid) { - int rc; + int rc = 0; /* Be aware that this function might run in a separate process. * Therefore, any driver state changes would be thrown away. */ @@ -683,7 +683,9 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv, /* on -2 returned an error was already reported */ if (rc == -2) return -1; - } else { + } + + if (rc == 0 || rc == -3) { struct stat sb; if (!path) diff --git a/src/security/security_manager.h b/src/security/security_manager.h index b5c81e9d98..57047ccb13 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -53,9 +53,16 @@ int virSecurityManagerStackAddNested(virSecurityManager *stack, * @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. */ + * A function callback to chown image files described by the disk + * source struct @src. The callback can decide to skip given @src + * and thus let DAC driver chown the file instead (signalled by + * returning -3). + * + * Returns: 0 on success, + * -1 on error and errno set (no libvirt error reported), + * -2 and a libvirt error reported. + * -3 if callback did not handle chown + */ typedef int (*virSecurityManagerDACChownCallback)(const virStorageSource *src, uid_t uid, -- 2.31.1