[libvirt] [PATCH 2/3] storage: Fix algorithm generating path names for devmapper

[John Ferlan]

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

Commit id '020135dc' didn't quite get the algorithm correct when a
device mapper source ended with a non numeric value (e.g. ends with
an alphabet value).

This patch modifies the 'part_separator' logic to add the "p" separator
to the attempted target path name only when specified as part_separator='yes'.

For a source name that already ends with a number, the logic doesn't change
as the part separator would need to be there.

For a source name that ends with something other than a number, this allows
the possibility that a "p" separator can be added. The default for one of
these source devices is to not add the separator.

The key for device mapper and the need for a partition separator "p" is
the presence of a number in the last character of the device name link
in /dev/mapper.  A name such as "/dev/mapper/mpatha1" would generate
a "/dev/mapper/mpatha1p1" partition, while "/dev/mapper/mpatha" would
generate partition "/dev/mapper/mpatha1". Similarly for a device
mapper entry not using friendly names or an alias, a device such as
"/dev/mapper/3600a0b80005b10ca00005ad656fd8d93" would generate a
paritition "/dev/mapper/3600a0b80005b10ca00005ad656fd8d93p1", while
a device such as "/dev/mapper/3600a0b80005b10ca00005e115729093f" would
generate a partition "/dev/mapper/3600a0b80005b10ca00005e115729093f1".
The long number is the WWID of the device. It's also possible to assign
an alias for a device mapper entry, that alias follows the same rules
with respect to ending with a number or not when adding a "p" to create
the target device path.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 docs/formatstorage.html.in         | 18 +++++-------------
 src/storage/parthelper.c           | 11 ++++++-----
 src/storage/storage_backend_disk.c |  9 +++++----
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 0356182..94277a1 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -134,19 +134,11 @@
         <code>path</code> may be supplied. Valid values for the attribute
         may be either "yes" or "no". This attribute is to be used for a
         <code>disk</code> pool type using a <code>path</code> to a
-        device mapper multipath device configured to utilize either
-        'user_friendly_names' or a custom 'alias' name in the
-        /etc/multipath.conf. The attribute directs libvirt to not
-        generate device volume names with the partition character "p".
-        By default, when libvirt generates the partition names for
-        device mapper multipath devices it will add a "p" path separator
-        to the device name before adding the partition number. For example,
-        a <code>device path</code> of '/dev/mapper/mpatha' libvirt would
-        generate partition names of '/dev/mapper/mpathap1',
-        '/dev/mapper/mpathap2', etc. for each partition found. With
-        this attribute set to "no", libvirt will not append the "p" to
-        the name unless it ends with a number thus generating names
-        of '/dev/mapper/mpatha1', '/dev/mapper/mpatha2', etc.
+        device mapper multipath device. Setting the attribute to "yes"
+        causes libvirt to attempt to generate and find target volume path's
+        using a "p" separator. The default algorithm used by device mapper
+        is to add the "p" separator only when the source device path ends
+        with a number.
         <span class="since">Since 1.3.1</span></p></dd>
       <dt><code>dir</code></dt>
       <dd>Provides the source for pools backed by directories (pool
diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c
index 6695f23..d81b177 100644
--- a/src/storage/parthelper.c
+++ b/src/storage/parthelper.c
@@ -69,7 +69,7 @@ int main(int argc, char **argv)
     const char *path;
     char *canonical_path;
     const char *partsep;
-    bool devmap_nopartsep = false;
+    bool devmap_partsep = false;
 
     if (virGettextInitialize() < 0)
         exit(EXIT_FAILURE);
@@ -77,7 +77,7 @@ int main(int argc, char **argv)
     if (argc == 3 && STREQ(argv[2], "-g")) {
         cmd = DISK_GEOMETRY;
     } else if (argc == 3 && STREQ(argv[2], "-p")) {
-        devmap_nopartsep = true;
+        devmap_partsep = true;
     } else if (argc != 2) {
         fprintf(stderr, _("syntax: %s DEVICE [-g]|[-p]\n"), argv[0]);
         return 1;
@@ -85,10 +85,11 @@ int main(int argc, char **argv)
 
     path = argv[1];
     if (virIsDevMapperDevice(path)) {
-        /* The 'devmap_nopartsep' option will not append the partsep of "p"
-         * onto the name unless the 'path' ends in a number
+        /* If the path ends with a number or we explicitly request it for
+         * path, then append the "p" partition separator. Otherwise, if
+         * the path ends with a letter already, then no need for a separator.
          */
-        if (c_isdigit(path[strlen(path)-1]) || !devmap_nopartsep)
+        if (c_isdigit(path[strlen(path)-1]) || devmap_partsep)
             partsep = "p";
         else
             partsep = "";
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 3e0395d..eadf8a3 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -325,13 +325,14 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool,
                                pool->def->source.devices[0].path,
                                NULL);
 
-    /* Check for the presence of the part_separator='no'. Pass this
+    /* Check for the presence of the part_separator='yes'. Pass this
      * along to the libvirt_parthelper as option '-p'. This will cause
-     * libvirt_parthelper to not append the "p" partition separator to
-     * the generated device name, unless the name ends with a number.
+     * libvirt_parthelper to append the "p" partition separator to
+     * the generated device name for a source device which ends with
+     * a non-numeric value (e.g. mpatha would generate mpathap#).
      */
     if (pool->def->source.devices[0].part_separator ==
-        VIR_TRISTATE_BOOL_NO)
+        VIR_TRISTATE_BOOL_YES)
         virCommandAddArg(cmd, "-p");
 
     /* If a volume is passed, virStorageBackendDiskMakeVol only updates the
-- 
2.5.5