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

[John Ferlan]

ping?

Tks -

John

On 11/17/2016 09:55 AM, John Ferlan wrote:
> 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;
>      }
>  
>