Re: [libvirt] [PATCH v2 7/12] storage: Refactor and fix virStorageBackendCreateExecCommand

Refactor the code to handle the NETFS pool separately from other pool types. When the original code was developed (commit id 'e1f27784') the virCommandSetUmask did not exist (commit id '0e1a1a8c4'), so there was no way to set the permissions for the creation. Now that it exists, we can use it. The code currently fails to create a volume in a NETFS pool from some other volume within the pool because the chmod done after file creation fails. So adjust the code to jump to cleanup once the file is successfully created in the NETFS pool. If it fails or for non NETFS files, just perform the create, chown, and chmod in order Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_backend.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index a375fe0..037d6d7 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -678,7 +678,6 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, gid_t gid; uid_t uid; mode_t mode; - bool filecreated = false; int ret = -1; if ((pool->def->type == VIR_STORAGE_POOL_NETFS) @@ -690,26 +689,29 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, virCommandSetUID(cmd, vol->target.perms->uid); virCommandSetGID(cmd, vol->target.perms->gid); + mode = (vol->target.perms->mode == (mode_t) -1 ? + VIR_STORAGE_DEFAULT_VOL_PERM_MODE : vol->target.perms->mode); + virCommandSetUmask(cmd, 0777 - mode); - if (virCommandRun(cmd, NULL) == 0) { - /* command was successfully run, check if the file was created */ - if (stat(vol->target.path, &st) >= 0) - filecreated = true; + if ((ret = virCommandRun(cmd, NULL)) == 0) { + /* command was successfully run, if the file was created + * then we are done */ + if (virFileExists(vol->target.path)) + goto cleanup; } } - /* don't change uid/gid if we retry */ + /* don't change uid/gid/mode if we retry */ virCommandSetUID(cmd, -1); virCommandSetGID(cmd, -1); + virCommandSetUmask(cmd, 0); - if (!filecreated) { - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - if (stat(vol->target.path, &st) < 0) { - virReportSystemError(errno, - _("failed to create %s"), vol->target.path); - goto cleanup; - } + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + if (stat(vol->target.path, &st) < 0) { + virReportSystemError(errno, + _("failed to create %s"), vol->target.path); + goto cleanup; } uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid