[libvirt] [PATCH] disk: Fixup error handling path for devmapper when part_separator='yes'

[John Ferlan]

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

If libvirt_parthelper is erroneously told to append the partition
separator 'p' onto the generated output for a disk pool using device
mapper that has 'user_friendly_names' set to true, then the error
recovery path will fail to find volume resulting in the pool being
in an unusable state.

So, augment the documentation to provide the better hint that the
part_separator='yes' should be set when user_friendly_names are not
being used. Additionally, once we're in the error path where the
returned name doesn't match the expected partition name try to see
if the reason is because the 'p' was erroneosly added. If so alter
the about to be removed vol->target.path so that the DiskDeleteVol
code can find the partition that was created and remove it.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 docs/formatstorage.html.in         |  5 ++++-
 src/storage/storage_backend_disk.c | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index ed1f0d1..7690114 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -138,7 +138,10 @@
         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.
+        with a number; however, it's possible to configure the devmapper
+        device to not use 'user_friendly_names' thus creating partitions
+        with the "p" separator even when the device source path does not
+        end 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/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 666ad03..07a04f1 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -95,6 +95,29 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
         virReportError(VIR_ERR_INVALID_ARG,
                        _("invalid partition name '%s', expected '%s'"),
                        vol->name, partname);
+
+        /* Let's see if we by chance parthelper created a name that won't be
+         * found later when we try to delete. We tell parthelper to add a 'p'
+         * to the output via the part_separator flag, but if devmapper has
+         * user_friendly_names set, the creation won't happen that way, thus
+         * our deletion will fail because the name we generated is wrong.
+         * Check for our conditions and see if the generated name is the
+         * same as StablePath returns and has the 'p' in it */
+        if (pool->def->source.devices[0].part_separator ==
+            VIR_TRISTATE_BOOL_YES &&
+            !virIsDevMapperDevice(vol->target.path) &&
+            STREQ(groups[0], vol->target.path) &&
+            (tmp = strrchr(groups[0], 'p'))) {
+
+            /* If we remove the 'p' from groups[0] and the resulting
+             * device is a devmapper device, then we know parthelper
+             * was told to create the wrong name based on the results.
+             * So just remove the 'p' from the vol->target.path too. */
+            memmove(tmp, tmp + 1, strlen(tmp));
+            if (virIsDevMapperDevice(groups[0]) &&
+                (tmp = strrchr(vol->target.path, 'p')))
+                memmove(tmp, tmp + 1, strlen(tmp));
+        }
         return -1;
     }
 
-- 
2.7.4