8:21 p.m.
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'>.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatstorage.html.in | 12 ++++++++++--
src/storage/storage_backend_disk.c | 30 ++++++++++++++++++++++++------
tools/virsh.pod | 17 ++++++++++++++---
3 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 933268c..d2e2bb8 100644
--- 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 300aab3..9f4d76a 100644
--- 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 abe80c2..1591341 100644
--- 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;
--
2.1.0