storage: Check the partition name against provided name

https://bugzilla.redhat.com/show_bug.cgi?id=1138516

If the provided volume name doesn't match what parted generated as the
partition name, then return a failure.

Update virsh.pod and formatstorage.html.in to describe the 'name' restriction
for disk pools as well as the usage of the <target>'s <format type='value'>.

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 933268cde33effef9815f7e6f4e9b5f26c9009f0..d2e2bb83bd6630ca44c36ced3d3b996f0289636e 100644 (file)
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -463,7 +463,13 @@
     <dl>
       <dt><code>name</code></dt>
       <dd>Providing a name for the volume which is unique to the pool.
-        This is mandatory when defining a volume.  <span class="since">Since 0.4.1</span></dd>
+        This is mandatory when defining a volume. For a disk pool, the
+        name must be combination of the <code>source</code> device path
+        device and next partition number to be created. For example, if
+        the <code>source</code> device path is /dev/sdb and there are no
+        partitions on the disk, then the name must be sdb1 with the next
+        name being sdb2 and so on.
+        <span class="since">Since 0.4.1</span></dd>
       <dt><code>key</code></dt>
       <dd>Providing an identifier for the volume which identifies a
           single volume. In some cases it's possible to have two distinct keys
@@ -553,7 +559,9 @@
         <span class="since">Since 0.4.1</span></dd>
       <dt><code>format</code></dt>
       <dd>Provides information about the pool specific volume format.
-        For disk pools it will provide the partition type. For filesystem
+        For disk pools it will provide the partition table format type, but is
+        not preserved after a pool refresh or libvirtd restart. Use extended
+        in order to create an extended disk extent partition. For filesystem
         or directory pools it will provide the file format type, eg cow,
         qcow, vmdk, raw. If omitted when creating a volume, the pool's
         default format will be used. The actual format is specified via

diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 300aab3b73d67809e7a2acae378e18e4ef63451d..9f4d76a9b3148c43fce0f41efdf42a397a4d5014 100644 (file)
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -47,16 +47,23 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
                                  char **const groups,
                                  virStorageVolDefPtr vol)
 {
-    char *tmp, *devpath;
+    char *tmp, *devpath, *partname;
+
+    /* Prepended path will be same for all partitions, so we can
+     * strip the path to form a reasonable pool-unique name
+     */
+    if ((tmp = strrchr(groups[0], '/')))
+        partname = tmp + 1;
+    else
+        partname = groups[0];
 
     if (vol == NULL) {
+        /* This is typically a reload/restart/refresh path where
+         * we're discovering the existing partitions for the pool
+         */
         if (VIR_ALLOC(vol) < 0)
             return -1;
-        /* Prepended path will be same for all partitions, so we can
-         * strip the path to form a reasonable pool-unique name
-         */
-        tmp = strrchr(groups[0], '/');
-        if (VIR_STRDUP(vol->name, tmp ? tmp + 1 : groups[0]) < 0 ||
+        if (VIR_STRDUP(vol->name, partname) < 0 ||
             VIR_APPEND_ELEMENT_COPY(pool->volumes.objs,
                                     pool->volumes.count, vol) < 0) {
             virStorageVolDefFree(vol);
@@ -80,6 +87,17 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
             return -1;
     }
 
+    /* Enforce provided vol->name is the same as what parted created.
+     * We do this after filling target.path so that we have a chance at
+     * deleting the partition with this failure from CreateVol path
+     */
+    if (STRNEQ(vol->name, partname)) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("invalid partition name '%s', expected '%s'"),
+                       vol->name, partname);
+        return -1;
+    }
+
     if (vol->key == NULL) {
         /* XXX base off a unique key of the underlying disk */
         if (VIR_STRDUP(vol->key, vol->target.path) < 0)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index abe80c2714607898e7f35b7f06084d3f36f0da4f..15913412c1259df0937f09b765c7077676116cbf 100644 (file)
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2983,7 +2983,11 @@ Create and start a pool object from the XML I<file>.
 Create and start a pool object I<name> from the raw parameters.  If
 I<--print-xml> is specified, then print the XML of the pool object
 without creating the pool.  Otherwise, the pool has the specified
-I<type>.
+I<type>. When using B<pool-create-as> for a pool of I<type> "disk",
+the existing partitions found on the I<--source-dev path> will be used
+to populate the disk pool. Therefore, it is suggested to use
+B<pool-define-as> and B<pool-build> with the I<--overwrite> in order
+to properly initialize the disk pool.
 
 [I<--source-host hostname>] provides the source hostname for pools backed
 by storage from a remote server (pool types netfs, iscsi, rbd, sheepdog,
@@ -3175,13 +3179,20 @@ I<vol-name-or-key-or-path>] [I<--backing-vol-format> I<string>]
 Create a volume from a set of arguments.
 I<pool-or-uuid> is the name or UUID of the storage pool to create the volume
 in.
-I<name> is the name of the new volume.
+I<name> is the name of the new volume. For a disk pool, this must match the
+partition name as determined from the pool's source device path and the next
+available partition. For example, a source device path of /dev/sdb and there
+are no partitions on the disk, then the name must be sdb1 with the next
+name being sdb2 and so on.
 I<capacity> is the size of the volume to be created, as a scaled integer
 (see B<NOTES> above), defaulting to bytes if there is no suffix.
 I<--allocation> I<size> is the initial size to be allocated in the volume,
 also as a scaled integer defaulting to bytes.
 I<--format> I<string> is used in file based storage pools to specify the volume
-file format to use; raw, bochs, qcow, qcow2, vmdk, qed.
+file format to use; raw, bochs, qcow, qcow2, vmdk, qed. Use extended for disk
+storage pools in order to create an extended partition (other values are
+validity checked but not preserved when libvirtd is restarted or the pool
+is refreshed).
 I<--backing-vol> I<vol-name-or-key-or-path> is the source backing
 volume to be used if taking a snapshot of an existing volume.
 I<--backing-vol-format> I<string> is the format of the snapshot backing volume;