This function can be used to translate MAJ:MIN device pair into
/dev/blah name.

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

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1bbff72263..a705bfd5ef 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1787,6 +1787,7 @@ virFileLength;
 virFileLinkPointsTo;
 virFileLock;
 virFileLoopDeviceAssociate;
+virFileMajMinToName;
 virFileMakeParentPath;
 virFileMakePath;
 virFileMakePathWithMode;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index f89e4bd823..62620862a1 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -68,6 +68,12 @@
 # include <libdevmapper.h>
 #endif
 
+#ifdef MAJOR_IN_MKDEV
+# include <sys/mkdev.h>
+#elif MAJOR_IN_SYSMACROS
+# include <sys/sysmacros.h>
+#endif
+
 #include "configmake.h"
 #include "intprops.h"
 #include "viralloc.h"
@@ -4317,3 +4323,50 @@ virFileGetMPathTargets(const char *path ATTRIBUTE_UNUSED,
     return -1;
 }
 #endif /* ! WITH_DEVMAPPER */
+
+
+/**
+ * virFileMajMinToName:
+ * @device: device (rdev) to translate
+ * @name: returned name
+ *
+ * For given MAJ:MIN pair (stored in one integer like in st_rdev)
+ * fetch device name, e.g. 8:0 is translated to "/dev/sda".
+ * Caller is responsible for freeing @name when no longer needed.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+int
+virFileMajMinToName(unsigned long long device,
+                    char **name)
+{
+    struct stat sb;
+    char *sysfsPath = NULL;
+    char *link = NULL;
+    int ret = -1;
+
+    *name = NULL;
+    if (virAsprintf(&sysfsPath, "/sys/dev/block/%u:%u",
+                    major(device), minor(device)) < 0)
+        goto cleanup;
+
+    if (lstat(sysfsPath, &sb) < 0)
+        goto cleanup;
+
+    if (!S_ISLNK(sb.st_mode)) {
+        errno = ENXIO;
+        goto cleanup;
+    }
+
+    if (virFileReadLink(sysfsPath, &link) < 0)
+        goto cleanup;
+
+    if (virAsprintf(name, "/dev/%s", last_component(link)) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(link);
+    VIR_FREE(sysfsPath);
+    return ret;
+}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 0db8bf0b99..390940dc98 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -362,4 +362,7 @@ int virFileGetMPathTargets(const char *path,
                            unsigned long long **devs,
                            size_t *ndevs);
 
+int virFileMajMinToName(unsigned long long device,
+                        char **name);
+
 #endif /* __VIR_FILE_H */
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Mar 26, 2018 at 07:16:44 +0200, Michal Privoznik wrote:
> This function can be used to translate MAJ:MIN device pair into
> /dev/blah name.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virfile.c       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virfile.h       |  3 +++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index f89e4bd823..62620862a1 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -68,6 +68,12 @@
>  # include <libdevmapper.h>
>  #endif
>  
> +#ifdef MAJOR_IN_MKDEV
> +# include <sys/mkdev.h>
> +#elif MAJOR_IN_SYSMACROS
> +# include <sys/sysmacros.h>
> +#endif
> +
>  #include "configmake.h"
>  #include "intprops.h"
>  #include "viralloc.h"
> @@ -4317,3 +4323,50 @@ virFileGetMPathTargets(const char *path ATTRIBUTE_UNUSED,
>      return -1;
>  }
>  #endif /* ! WITH_DEVMAPPER */
> +
> +
> +/**
> + * virFileMajMinToName:
> + * @device: device (rdev) to translate
> + * @name: returned name

I'd prefer if you supply the device identifiers already separated. That
means that the function in the previous patch should preferrably already
split them so that we don't need to do it here.

> + *
> + * For given MAJ:MIN pair (stored in one integer like in st_rdev)
> + * fetch device name, e.g. 8:0 is translated to "/dev/sda".
> + * Caller is responsible for freeing @name when no longer needed.

There is no info on how to treat errors.

> + *
> + * Returns 0 on success, -1 otherwise.

Why isn't the path returned directly?

> + */
> +int
> +virFileMajMinToName(unsigned long long device,
> +                    char **name)
> +{
> +    struct stat sb;
> +    char *sysfsPath = NULL;
> +    char *link = NULL;
> +    int ret = -1;
> +
> +    *name = NULL;
> +    if (virAsprintf(&sysfsPath, "/sys/dev/block/%u:%u",
> +                    major(device), minor(device)) < 0)

So the libdevmapper code is using a similar path to figure out the sysfs
path, but only for devicemapper devices since it accesses
/sys/dev/block/%u:%u/dm/name.

I've also found that '/dev/block/' has the information you might need.

Is there any particular reason to use sysfs? I'm not really persuaded
that the directory name in sysfs is good enough so if you can provide
where you've inspired yourself it will be beneficial.

Additionally virAsprintf reports libvirt errors but this function does
not. 

> +        goto cleanup;
> +
> +    if (lstat(sysfsPath, &sb) < 0)
> +        goto cleanup;
> +
> +    if (!S_ISLNK(sb.st_mode)) {
> +        errno = ENXIO;
> +        goto cleanup;
> +    }
> +
> +    if (virFileReadLink(sysfsPath, &link) < 0)
> +        goto cleanup;
> +
> +    if (virAsprintf(name, "/dev/%s", last_component(link)) < 0)
> +        goto cleanup;

So this is the fishy path. I'd prefer /dev/block since the path already
points to something in the /dev/ filesystem and isn't conjured up from
something that looks the same.

> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(link);
> +    VIR_FREE(sysfsPath);
> +    return ret;
> +}
> diff --git a/src/util/virfile.h b/src/util/virfile.h
> index 0db8bf0b99..390940dc98 100644
> --- a/src/util/virfile.h
> +++ b/src/util/virfile.h
> @@ -362,4 +362,7 @@ int virFileGetMPathTargets(const char *path,
>                             unsigned long long **devs,
>                             size_t *ndevs);
>  
> +int virFileMajMinToName(unsigned long long device,
> +                        char **name);
> +
>  #endif /* __VIR_FILE_H */
> -- 
> 2.16.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 03/26/2018 11:58 AM, Peter Krempa wrote:
> On Mon, Mar 26, 2018 at 07:16:44 +0200, Michal Privoznik wrote:
>> This function can be used to translate MAJ:MIN device pair into
>> /dev/blah name.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/libvirt_private.syms |  1 +
>>  src/util/virfile.c       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  src/util/virfile.h       |  3 +++
>>  3 files changed, 57 insertions(+)
>>
>> diff --git a/src/util/virfile.c b/src/util/virfile.c
>> index f89e4bd823..62620862a1 100644
>> --- a/src/util/virfile.c
>> +++ b/src/util/virfile.c
>> @@ -68,6 +68,12 @@
>>  # include <libdevmapper.h>
>>  #endif
>>  
>> +#ifdef MAJOR_IN_MKDEV
>> +# include <sys/mkdev.h>
>> +#elif MAJOR_IN_SYSMACROS
>> +# include <sys/sysmacros.h>
>> +#endif
>> +
>>  #include "configmake.h"
>>  #include "intprops.h"
>>  #include "viralloc.h"
>> @@ -4317,3 +4323,50 @@ virFileGetMPathTargets(const char *path ATTRIBUTE_UNUSED,
>>      return -1;
>>  }
>>  #endif /* ! WITH_DEVMAPPER */
>> +
>> +
>> +/**
>> + * virFileMajMinToName:
>> + * @device: device (rdev) to translate
>> + * @name: returned name
> 
> I'd prefer if you supply the device identifiers already separated. That
> means that the function in the previous patch should preferrably already
> split them so that we don't need to do it here.

Okay.

> 
>> + *
>> + * For given MAJ:MIN pair (stored in one integer like in st_rdev)
>> + * fetch device name, e.g. 8:0 is translated to "/dev/sda".
>> + * Caller is responsible for freeing @name when no longer needed.
> 
> There is no info on how to treat errors.
> 
>> + *
>> + * Returns 0 on success, -1 otherwise.
> 
> Why isn't the path returned directly?

No reason. It's just that I'm used to functions returning an integer. I
can change it.

> 
>> + */
>> +int
>> +virFileMajMinToName(unsigned long long device,
>> +                    char **name)
>> +{
>> +    struct stat sb;
>> +    char *sysfsPath = NULL;
>> +    char *link = NULL;
>> +    int ret = -1;
>> +
>> +    *name = NULL;
>> +    if (virAsprintf(&sysfsPath, "/sys/dev/block/%u:%u",
>> +                    major(device), minor(device)) < 0)
> 
> So the libdevmapper code is using a similar path to figure out the sysfs
> path, but only for devicemapper devices since it accesses
> /sys/dev/block/%u:%u/dm/name.
> 
> I've also found that '/dev/block/' has the information you might need.
> 
> Is there any particular reason to use sysfs? I'm not really persuaded
> that the directory name in sysfs is good enough so if you can provide
> where you've inspired yourself it will be beneficial.

Okay, I'll switch to /dev/block/... Looks like I took too much
inspiration from other projects O:-)

> 
> Additionally virAsprintf reports libvirt errors but this function does
> not. 
> 
>> +        goto cleanup;
>> +
>> +    if (lstat(sysfsPath, &sb) < 0)
>> +        goto cleanup;
>> +
>> +    if (!S_ISLNK(sb.st_mode)) {
>> +        errno = ENXIO;
>> +        goto cleanup;
>> +    }
>> +
>> +    if (virFileReadLink(sysfsPath, &link) < 0)
>> +        goto cleanup;
>> +
>> +    if (virAsprintf(name, "/dev/%s", last_component(link)) < 0)
>> +        goto cleanup;
> 
> So this is the fishy path. I'd prefer /dev/block since the path already
> points to something in the /dev/ filesystem and isn't conjured up from
> something that looks the same.
> 

Ah, good point. So if I do plain:

virAsprintf(&path, "/dev/block/%u:%u", major(), minor())

I immediately get the correct symlink with both namespace and cgroup
codes know how to deal with. Cool, so this patch might be dropped.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list