Re: [libvirt] [PATCH v2 7/12] storage: Refactor and fix virStorageBackendCreateExecCommand
Yes, this should have read 8/12...
On 10/14/2015 02:01 PM, John Ferlan wrote:
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>
---
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);
^^^
Just to be clear - this fixes a bug in the existing code which cannot
create 'qcow2' file in an NFS pool that has root squash enabled. The
failure "currently" is :
virsh vol-create-from bz1253609 qemu-img.xml test.img --inputpool bz1253609
error: Failed to create vol from qemu-img.xml
error: cannot set mode of '/tmp/netfs-pool/qemu-img-test.img' to 0644:
Operation not permitted
Assuming that 'test.img' was created using :
virsh vol-create bz1253609 vol.xml
The qemu-img.xml is:
<volume>
<name>qemu-img-test.img</name>
<source>
</source>
<capacity>4048576000</capacity>
<allocation>0</allocation>
<target>
<format type='qcow2'/>
<permissions>
<mode>0644</mode>
<owner>107</owner>
<group>107</group>
<label>system_u:object_r:nfs_t:s0</label>
</permissions>
</target>
</volume>
it fails it other ways if <mode> and/or <owner>/<group> are not
supplied.
FWIW: The 'vol.xml' file differs from the above by only the
"type='raw'"
John
- 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