[libvirt] [PATCH 2/5] utils: Introduce virFileGetMPathTargets

Michal Privoznik posted 5 patches 7 years, 8 months ago
There is a newer version of this series
[libvirt] [PATCH 2/5] utils: Introduce virFileGetMPathTargets
Posted by Michal Privoznik 7 years, 8 months ago
This helper fetches targets for multipath devices.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c       | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/util/virfile.h       |  4 +++
 3 files changed, 95 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 03fe3b315f..1bbff72263 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1773,6 +1773,7 @@ virFileGetACLs;
 virFileGetHugepageSize;
 virFileGetMountReverseSubtree;
 virFileGetMountSubtree;
+virFileGetMPathTargets;
 virFileHasSuffix;
 virFileInData;
 virFileIsAbsPath;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 5e9bd2007a..f89e4bd823 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -64,6 +64,10 @@
 # include <sys/ioctl.h>
 #endif
 
+#ifdef WITH_DEVMAPPER
+# include <libdevmapper.h>
+#endif
+
 #include "configmake.h"
 #include "intprops.h"
 #include "viralloc.h"
@@ -4227,3 +4231,89 @@ virFileWaitForExists(const char *path,
 
     return 0;
 }
+
+
+#ifdef WITH_DEVMAPPER
+/**
+ * virFileGetMPathTargets:
+ * @path: multipath device
+ * @devs: returned array of st_rdevs of @path targets
+ * @ndevs: size of @devs and @devNames
+ *
+ * For given @path figure out its targets, and store them in
+ * @devs. Items from @devs are meant to be used in
+ * minor() and major() to receive device MAJ:MIN pairs.
+ *
+ * If @path is not a multipath device, @ndevs is set to 0 and
+ * success is returned.
+ *
+ * If we don't have permissions to talk to kernel, -1 is returned
+ * and errno is set to EBADF.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+int
+virFileGetMPathTargets(const char *path,
+                       unsigned long long **devs,
+                       size_t *ndevs)
+{
+    struct dm_deps *deps;
+    struct dm_task *dmt;
+    struct dm_info info;
+    size_t i;
+    int ret = -1;
+
+    *ndevs = 0;
+
+    if (!(dmt = dm_task_create(DM_DEVICE_DEPS)))
+        goto cleanup;
+
+    if (!dm_task_set_name(dmt, path)) {
+        if (errno == ENOENT) {
+            /* It's okay, @path is not managed by devmapper =>
+             * not a multipath device. */
+            ret = 0;
+        }
+        goto cleanup;
+    }
+
+    dm_task_no_open_count(dmt);
+
+    if (!dm_task_run(dmt))
+        goto cleanup;
+
+    if (!dm_task_get_info(dmt, &info))
+        goto cleanup;
+
+    if (!info.exists) {
+        ret = 0;
+        goto cleanup;
+    }
+
+    if (!(deps = dm_task_get_deps(dmt)))
+        goto cleanup;
+
+    if (VIR_ALLOC_N(*devs, deps->count) < 0)
+        goto cleanup;
+    *ndevs = deps->count;
+
+    for (i = 0; i < deps->count; i++)
+        (*devs)[i] = deps->device[i];
+
+    ret = 0;
+ cleanup:
+    dm_task_destroy(dmt);
+    return ret;
+}
+
+#else /* ! WITH_DEVMAPPER */
+
+int
+virFileGetMPathTargets(const char *path ATTRIBUTE_UNUSED,
+                       unsigned long long **devs ATTRIBUTE_UNUSED,
+                       size_t *ndevs ATTRIBUTE_UNUSED)
+{
+    errno = ENOSYS;
+    return -1;
+}
+#endif /* ! WITH_DEVMAPPER */
diff --git a/src/util/virfile.h b/src/util/virfile.h
index cd2a3867c2..0db8bf0b99 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -358,4 +358,8 @@ int virFileInData(int fd,
                   int *inData,
                   long long *length);
 
+int virFileGetMPathTargets(const char *path,
+                           unsigned long long **devs,
+                           size_t *ndevs);
+
 #endif /* __VIR_FILE_H */
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] utils: Introduce virFileGetMPathTargets
Posted by Peter Krempa 7 years, 8 months ago
On Mon, Mar 26, 2018 at 07:16:43 +0200, Michal Privoznik wrote:
> This helper fetches targets for multipath devices.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virfile.c       | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virfile.h       |  4 +++
>  3 files changed, 95 insertions(+)

As I've pointed in the conversation regarding patch 1/5, this should be
put into a separate file, so that it's obvious what needs to be removed
if this functionality will need to be put into the storage driver.

Additionally it then will be justified to use a virOnce there to set the
logging function for libdevmapper.

> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 5e9bd2007a..f89e4bd823 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c

> @@ -4227,3 +4231,89 @@ virFileWaitForExists(const char *path,
>  
>      return 0;
>  }
> +
> +
> +#ifdef WITH_DEVMAPPER
> +/**
> + * virFileGetMPathTargets:
> + * @path: multipath device
> + * @devs: returned array of st_rdevs of @path targets
> + * @ndevs: size of @devs and @devNames

There's no 'devNames' parameter.

> + *
> + * For given @path figure out its targets, and store them in
> + * @devs. Items from @devs are meant to be used in
> + * minor() and major() to receive device MAJ:MIN pairs.
> + *
> + * If @path is not a multipath device, @ndevs is set to 0 and
> + * success is returned.
> + *
> + * If we don't have permissions to talk to kernel, -1 is returned
> + * and errno is set to EBADF.

You need to document that this function does not use libvirt errors.

> + *
> + * Returns 0 on success, -1 otherwise.
> + */
> +int
> +virFileGetMPathTargets(const char *path,
> +                       unsigned long long **devs,

struct 'dm_deps' uses uint64_t for this.

> +                       size_t *ndevs)
> +{
> +    struct dm_deps *deps;
> +    struct dm_task *dmt;
> +    struct dm_info info;
> +    size_t i;
> +    int ret = -1;
> +
> +    *ndevs = 0;
> +
> +    if (!(dmt = dm_task_create(DM_DEVICE_DEPS)))
> +        goto cleanup;
> +
> +    if (!dm_task_set_name(dmt, path)) {
> +        if (errno == ENOENT) {
> +            /* It's okay, @path is not managed by devmapper =>
> +             * not a multipath device. */
> +            ret = 0;
> +        }
> +        goto cleanup;
> +    }
> +
> +    dm_task_no_open_count(dmt);
> +
> +    if (!dm_task_run(dmt))
> +        goto cleanup;
> +
> +    if (!dm_task_get_info(dmt, &info))
> +        goto cleanup;
> +
> +    if (!info.exists) {
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    if (!(deps = dm_task_get_deps(dmt)))
> +        goto cleanup;
> +
> +    if (VIR_ALLOC_N(*devs, deps->count) < 0)

VIR_ALLOC_N_QUIET, since we don't do libvirt errors.

> +        goto cleanup;
> +    *ndevs = deps->count;
> +
> +    for (i = 0; i < deps->count; i++)
> +        (*devs)[i] = deps->device[i];
> +
> +    ret = 0;
> + cleanup:
> +    dm_task_destroy(dmt);
> +    return ret;
> +}


Okay this code is stol^W inspired by the 'dmsetup deps' functionality.
You might want to point that fact out, since it took some time to find
it.

The code looks good, but since it requires the virOnce stuff, v2 will be
needed.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list