From 9bbbb9121654ad82931f21ef7f2fb1056670b955 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Thu, 15 Jan 2015 19:12:42 -0500 Subject: [PATCH] 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 's . --- 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 933268cde3..d2e2bb83bd 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -463,7 +463,13 @@
name
Providing a name for the volume which is unique to the pool. - This is mandatory when defining a volume. Since 0.4.1
+ This is mandatory when defining a volume. For a disk pool, the + name must be combination of the source device path + device and next partition number to be created. For example, if + the source 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. + Since 0.4.1
key
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 @@ Since 0.4.1
format
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 300aab3b73..9f4d76a9b3 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 abe80c2714..15913412c1 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2983,7 +2983,11 @@ Create and start a pool object from the XML I. Create and start a pool object I 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. +I. When using B for a pool of I "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 and B 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] [I<--backing-vol-format> I] Create a volume from a set of arguments. I is the name or UUID of the storage pool to create the volume in. -I is the name of the new volume. +I 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 is the size of the volume to be created, as a scaled integer (see B above), defaulting to bytes if there is no suffix. I<--allocation> I is the initial size to be allocated in the volume, also as a scaled integer defaulting to bytes. I<--format> I 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 is the source backing volume to be used if taking a snapshot of an existing volume. I<--backing-vol-format> I is the format of the snapshot backing volume;