conf: tweak volume target struct details

Some preparatory work before consolidating storage volume
structs with the rest of virstoragefile.  Making these
changes allows a volume target to be much closer to (a
subset of) the virStorageSource struct.

Making perms be a pointer allows it to be optional if we
have a storage pool that doesn't expose permissions in a
way we can access.  It also allows future patches to
optionally expose permissions details learned about a disk
image via domain <disk> listings, rather than just
limiting it to storage volume listings.

Disk partition types was only used by internal code to
control what type of partition to create when carving up
an MS-DOS partition table storage pool (and is not used
for GPT partition tables or other storage pools).  It was
not exposed in volume XML, and as it is more closely
related to extent information of the overall block device
than it is to the <target> information describing the host
file.  Besides, if we ever decide to expose it in XML down
the road, we can move it back as needed.

* src/conf/storage_conf.h (_virStorageVolTarget): Change perms to
pointer, enhance comments.  Move partition type...
(_virStorageVolSource): ...here.
* src/conf/storage_conf.c (virStorageVolDefFree)
(virStorageVolDefParseXML, virStorageVolTargetDefFormat): Update
clients.
* src/storage/storage_backend_fs.c (createFileDir): Likewise.
* src/storage/storage_backend.c (virStorageBackendCreateBlockFrom)
(virStorageBackendCreateRaw, virStorageBackendCreateExecCommand)
(virStorageBackendUpdateVolTargetInfoFD): Likewise.
* src/storage/storage_backend_logical.c
(virStorageBackendLogicalCreateVol): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskMakeDataVol)
(virStorageBackendDiskPartTypeToCreate): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 65504b451e1fa7e9f64fb05098e68767327c856d..e4986e686ecf9edd4f642d7980396d4ce66a4a2c 100644 (file)
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -332,11 +332,17 @@ virStorageVolDefFree(virStorageVolDefPtr def)
     VIR_FREE(def->target.compat);
     virBitmapFree(def->target.features);
     VIR_FREE(def->target.path);
-    VIR_FREE(def->target.perms.label);
+    if (def->target.perms) {
+        VIR_FREE(def->target.perms->label);
+        VIR_FREE(def->target.perms);
+    }
     VIR_FREE(def->target.timestamps);
     virStorageEncryptionFree(def->target.encryption);
     VIR_FREE(def->backingStore.path);
-    VIR_FREE(def->backingStore.perms.label);
+    if (def->backingStore.perms) {
+        VIR_FREE(def->backingStore.perms->label);
+        VIR_FREE(def->backingStore.perms);
+    }
     VIR_FREE(def->backingStore.timestamps);
     virStorageEncryptionFree(def->backingStore.encryption);
     VIR_FREE(def);
@@ -1355,7 +1361,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
         VIR_FREE(format);
     }
 
-    if (virStorageDefParsePerms(ctxt, &ret->target.perms,
+    if (VIR_ALLOC(ret->target.perms) < 0)
+        goto error;
+    if (virStorageDefParsePerms(ctxt, ret->target.perms,
                                 "./target/permissions",
                                 DEFAULT_VOL_PERM_MODE) < 0)
         goto error;
@@ -1424,7 +1432,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
         VIR_FREE(nodes);
     }
 
-    if (virStorageDefParsePerms(ctxt, &ret->backingStore.perms,
+    if (VIR_ALLOC(ret->backingStore.perms) < 0)
+        goto error;
+    if (virStorageDefParsePerms(ctxt, ret->backingStore.perms,
                                 "./backingStore/permissions",
                                 DEFAULT_VOL_PERM_MODE) < 0)
         goto error;
@@ -1541,15 +1551,15 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
     virBufferAdjustIndent(buf, 2);
 
     virBufferAsprintf(buf, "<mode>0%o</mode>\n",
-                      def->perms.mode);
+                      def->perms->mode);
     virBufferAsprintf(buf, "<owner>%u</owner>\n",
-                      (unsigned int) def->perms.uid);
+                      (unsigned int) def->perms->uid);
     virBufferAsprintf(buf, "<group>%u</group>\n",
-                      (unsigned int) def->perms.gid);
+                      (unsigned int) def->perms->gid);
 
 
     virBufferEscapeString(buf, "<label>%s</label>\n",
-                          def->perms.label);
+                          def->perms->label);
 
     virBufferAdjustIndent(buf, -2);
     virBufferAddLit(buf, "</permissions>\n");

diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index b8110468488e36a98ff38106728fbac15734111b..abff7ec30984792ec95d2ce74f6d40aba493b372 100644 (file)
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -71,6 +71,9 @@ typedef virStorageVolSource *virStorageVolSourcePtr;
 struct _virStorageVolSource {
     int nextent;
     virStorageVolSourceExtentPtr extents;
+
+    int partType; /* enum virStorageVolTypeDisk, only used by disk
+                   * backend for partition type creation */
 };
 
 
@@ -81,10 +84,10 @@ typedef struct _virStorageVolTarget virStorageVolTarget;
 typedef virStorageVolTarget *virStorageVolTargetPtr;
 struct _virStorageVolTarget {
     char *path;
-    int format;
-    virStoragePerms perms;
+    int format; /* enum virStorageFileFormat */
+    virStoragePermsPtr perms;
     virStorageTimestampsPtr timestamps;
-    int type; /* only used by disk backend for partition type */
+
     /* The next three are currently only used in vol->target,
      * not in vol->backingStore. */
     virStorageEncryptionPtr encryption;

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index eedd11b09b762a21c9e46c3bc06be966179ebdc9..c21504d12d295906f7f5bebf20a75fb6c461c190 100644 (file)
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -301,8 +301,10 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED,
                              vol->target.path);
         goto cleanup;
     }
-    uid = (vol->target.perms.uid != st.st_uid) ? vol->target.perms.uid : (uid_t) -1;
-    gid = (vol->target.perms.gid != st.st_gid) ? vol->target.perms.gid : (gid_t) -1;
+    uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid
+        : (uid_t) -1;
+    gid = (vol->target.perms->gid != st.st_gid) ? vol->target.perms->gid
+        : (gid_t) -1;
     if (((uid != (uid_t) -1) || (gid != (gid_t) -1))
         && (fchown(fd, uid, gid) < 0)) {
         virReportSystemError(errno,
@@ -311,10 +313,10 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED,
                              (unsigned int) gid);
         goto cleanup;
     }
-    if (fchmod(fd, vol->target.perms.mode) < 0) {
+    if (fchmod(fd, vol->target.perms->mode) < 0) {
         virReportSystemError(errno,
                              _("cannot set mode of '%s' to %04o"),
-                             vol->target.path, vol->target.perms.mode);
+                             vol->target.path, vol->target.perms->mode);
         goto cleanup;
     }
     if (VIR_CLOSE(fd) < 0) {
@@ -439,9 +441,9 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     if ((fd = virFileOpenAs(vol->target.path,
                             O_RDWR | O_CREAT | O_EXCL,
-                            vol->target.perms.mode,
-                            vol->target.perms.uid,
-                            vol->target.perms.gid,
+                            vol->target.perms->mode,
+                            vol->target.perms->uid,
+                            vol->target.perms->gid,
                             operation_flags)) < 0) {
         virReportSystemError(-fd,
                              _("Failed to create file '%s'"),
@@ -578,13 +580,13 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
 
     if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
         && (((geteuid() == 0)
-             && (vol->target.perms.uid != (uid_t) -1)
-             && (vol->target.perms.uid != 0))
-            || ((vol->target.perms.gid != (gid_t) -1)
-                && (vol->target.perms.gid != getegid())))) {
+             && (vol->target.perms->uid != (uid_t) -1)
+             && (vol->target.perms->uid != 0))
+            || ((vol->target.perms->gid != (gid_t) -1)
+                && (vol->target.perms->gid != getegid())))) {
 
-        virCommandSetUID(cmd, vol->target.perms.uid);
-        virCommandSetGID(cmd, vol->target.perms.gid);
+        virCommandSetUID(cmd, vol->target.perms->uid);
+        virCommandSetGID(cmd, vol->target.perms->gid);
 
         if (virCommandRun(cmd, NULL) == 0) {
             /* command was successfully run, check if the file was created */
@@ -608,8 +610,10 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
         }
     }
 
-    uid = (vol->target.perms.uid != st.st_uid) ? vol->target.perms.uid : (uid_t) -1;
-    gid = (vol->target.perms.gid != st.st_gid) ? vol->target.perms.gid : (gid_t) -1;
+    uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid
+        : (uid_t) -1;
+    gid = (vol->target.perms->gid != st.st_gid) ? vol->target.perms->gid
+        : (gid_t) -1;
     if (((uid != (uid_t) -1) || (gid != (gid_t) -1))
         && (chown(vol->target.path, uid, gid) < 0)) {
         virReportSystemError(errno,
@@ -618,10 +622,10 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
                              (unsigned int) gid);
         return -1;
     }
-    if (chmod(vol->target.path, vol->target.perms.mode) < 0) {
+    if (chmod(vol->target.path, vol->target.perms->mode) < 0) {
         virReportSystemError(errno,
                              _("cannot set mode of '%s' to %04o"),
-                             vol->target.path, vol->target.perms.mode);
+                             vol->target.path, vol->target.perms->mode);
         return -1;
     }
     return 0;
@@ -1495,9 +1499,11 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,
         }
     }
 
-    target->perms.mode = sb->st_mode & S_IRWXUGO;
-    target->perms.uid = sb->st_uid;
-    target->perms.gid = sb->st_gid;
+    if (!target->perms && VIR_ALLOC(target->perms) < 0)
+        return -1;
+    target->perms->mode = sb->st_mode & S_IRWXUGO;
+    target->perms->uid = sb->st_uid;
+    target->perms->gid = sb->st_gid;
 
     if (!target->timestamps && VIR_ALLOC(target->timestamps) < 0)
         return -1;
@@ -1506,7 +1512,7 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,
     target->timestamps->ctime = get_stat_ctime(sb);
     target->timestamps->mtime = get_stat_mtime(sb);
 
-    VIR_FREE(target->perms.label);
+    VIR_FREE(target->perms->label);
 
 #if WITH_SELINUX
     /* XXX: make this a security driver call */
@@ -1519,7 +1525,7 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,
                 return -1;
             }
         } else {
-            if (VIR_STRDUP(target->perms.label, filecon) < 0) {
+            if (VIR_STRDUP(target->perms->label, filecon) < 0) {
                 freecon(filecon);
                 return -1;
             }

diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index fb7a2a48b68b40ad70ef7830d9bdc88714b4e923..01f1b176417d565bb5909bdb9e588ac1fcb6ec0a 100644 (file)
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -119,13 +119,13 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
 
     /* set partition type */
     if (STREQ(groups[1], "normal"))
-       vol->target.type = VIR_STORAGE_VOL_DISK_TYPE_PRIMARY;
+       vol->source.partType = VIR_STORAGE_VOL_DISK_TYPE_PRIMARY;
     else if (STREQ(groups[1], "logical"))
-       vol->target.type = VIR_STORAGE_VOL_DISK_TYPE_LOGICAL;
+       vol->source.partType = VIR_STORAGE_VOL_DISK_TYPE_LOGICAL;
     else if (STREQ(groups[1], "extended"))
-       vol->target.type = VIR_STORAGE_VOL_DISK_TYPE_EXTENDED;
+       vol->source.partType = VIR_STORAGE_VOL_DISK_TYPE_EXTENDED;
     else
-       vol->target.type = VIR_STORAGE_VOL_DISK_TYPE_NONE;
+       vol->source.partType = VIR_STORAGE_VOL_DISK_TYPE_NONE;
 
     vol->type = VIR_STORAGE_VOL_BLOCK;
 
@@ -445,10 +445,10 @@ virStorageBackendDiskPartTypeToCreate(virStoragePoolObjPtr pool)
         size_t i;
         int count = 0;
         for (i = 0; i < pool->volumes.count; i++) {
-             if (pool->volumes.objs[i]->target.type == VIR_STORAGE_VOL_DISK_TYPE_PRIMARY ||
-                 pool->volumes.objs[i]->target.type == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) {
-                     count++;
-             }
+            int partType = pool->volumes.objs[i]->source.partType;
+            if (partType == VIR_STORAGE_VOL_DISK_TYPE_PRIMARY ||
+                partType == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED)
+                count++;
         }
         if (count >= 4) {
             return VIR_STORAGE_VOL_DISK_TYPE_LOGICAL;
@@ -614,7 +614,7 @@ virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool,
         *end -= (*start % cylinderSize);
     }
 
-    /* counting in byte, we want the last byte of the current sector */
+    /* counting in bytes, we want the last byte of the current sector */
     *end -= 1;
     VIR_DEBUG("final aligned start %llu, end %llu", *start, *end);
     return 0;

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index be0659a08fdf86092cfb65d481be5909702fe2ce..b3618048c8b063187e3c9bc8432735e3b22d0ec7 100644 (file)
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1051,9 +1051,9 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
         return -1;
     }
 
-    if ((err = virDirCreate(vol->target.path, vol->target.perms.mode,
-                            vol->target.perms.uid,
-                            vol->target.perms.gid,
+    if ((err = virDirCreate(vol->target.path, vol->target.perms->mode,
+                            vol->target.perms->uid,
+                            vol->target.perms->gid,
                             VIR_DIR_CREATE_FORCE_PERMS |
                             (pool->def->type == VIR_STORAGE_POOL_NETFS
                              ? VIR_DIR_CREATE_AS_UID : 0))) < 0) {

diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 7893626b6f537aa8f9428ccbe8a3323ff2af24c5..aea624ef38ece9d4e684ecba82263a039962ca00 100644 (file)
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -1,7 +1,7 @@
 /*
  * storage_backend_logical.c: storage backend for logical volume handling
  *
- * Copyright (C) 2007-2009, 2011, 2013 Red Hat, Inc.
+ * Copyright (C) 2007-2014 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -767,14 +767,14 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
 
     /* We can only chown/grp if root */
     if (geteuid() == 0) {
-        if (fchown(fd, vol->target.perms.uid, vol->target.perms.gid) < 0) {
+        if (fchown(fd, vol->target.perms->uid, vol->target.perms->gid) < 0) {
             virReportSystemError(errno,
                                  _("cannot set file owner '%s'"),
                                  vol->target.path);
             goto error;
         }
     }
-    if (fchmod(fd, vol->target.perms.mode) < 0) {
+    if (fchmod(fd, vol->target.perms->mode) < 0) {
         virReportSystemError(errno,
                              _("cannot set file mode '%s'"),
                              vol->target.path);