Expose a helper function that can be used by udev and mdevctl to
generate device names for node devices.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
---
 src/node_device/node_device_driver.c | 25 +++++++++++++++++++++++++
 src/node_device/node_device_driver.h |  6 ++++++
 src/node_device/node_device_udev.c   | 19 +++----------------
 3 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 920fd815f2..bada5bbf00 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -978,3 +978,28 @@ nodedevRegister(void)
     return udevNodeRegister();
 #endif
 }
+
+
+void
+nodeDeviceGenerateName(virNodeDeviceDef *def,
+                       const char *subsystem,
+                       const char *sysname,
+                       const char *s)
+{
+    size_t i;
+    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+    virBufferAsprintf(&buf, "%s_%s",
+                      subsystem,
+                      sysname);
+
+    if (s != NULL)
+        virBufferAsprintf(&buf, "_%s", s);
+
+    def->name = virBufferContentAndReset(&buf);
+
+    for (i = 0; i < strlen(def->name); i++) {
+        if (!(g_ascii_isalnum(*(def->name + i))))
+            *(def->name + i) = '_';
+    }
+}
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index 4a40aa51f6..76a40c05ad 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -120,3 +120,9 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
 virCommandPtr
 nodeDeviceGetMdevctlStopCommand(const char *uuid,
                                 char **errmsg);
+
+void
+nodeDeviceGenerateName(virNodeDeviceDef *def,
+                       const char *subsystem,
+                       const char *sysname,
+                       const char *s);
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 373f36c41f..eae301cc95 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -311,22 +311,9 @@ udevGenerateDeviceName(struct udev_device *device,
                        virNodeDeviceDefPtr def,
                        const char *s)
 {
-    size_t i;
-    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-
-    virBufferAsprintf(&buf, "%s_%s",
-                      udev_device_get_subsystem(device),
-                      udev_device_get_sysname(device));
-
-    if (s != NULL)
-        virBufferAsprintf(&buf, "_%s", s);
-
-    def->name = virBufferContentAndReset(&buf);
-
-    for (i = 0; i < strlen(def->name); i++) {
-        if (!(g_ascii_isalnum(*(def->name + i))))
-            *(def->name + i) = '_';
-    }
+    nodeDeviceGenerateName(def,
+                           udev_device_get_subsystem(device),
+                           udev_device_get_sysname(device), s);
 
     return 0;
 }
-- 
2.26.3

On Fri, Mar 26, 2021 at 11:48:02AM -0500, Jonathon Jongsma wrote:
> Expose a helper function that can be used by udev and mdevctl to
> generate device names for node devices.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/node_device/node_device_driver.c | 25 +++++++++++++++++++++++++
>  src/node_device/node_device_driver.h |  6 ++++++
>  src/node_device/node_device_udev.c   | 19 +++----------------
>  3 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 920fd815f2..bada5bbf00 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -978,3 +978,28 @@ nodedevRegister(void)
>      return udevNodeRegister();
>  #endif
>  }
> +
> +
> +void
> +nodeDeviceGenerateName(virNodeDeviceDef *def,
> +                       const char *subsystem,
> +                       const char *sysname,
> +                       const char *s)
> +{
> +    size_t i;
> +    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> +
> +    virBufferAsprintf(&buf, "%s_%s",
> +                      subsystem,
> +                      sysname);
> +
> +    if (s != NULL)
> +        virBufferAsprintf(&buf, "_%s", s);
> +
> +    def->name = virBufferContentAndReset(&buf);

Actually, there's a leak ^here than I only just noticed:

11 bytes in 1 blocks are definitely lost in loss record 222 of 3,093
==629005==    at 0x4C34F0B: malloc (vg_replace_malloc.c:307)
==629005==    by 0x59A92A5: g_malloc (in /usr/lib64/libglib-2.0.so.0.5600.4)
==629005==    by 0x59C2BE2: g_strdup (in /usr/lib64/libglib-2.0.so.0.5600.4)
==629005==    by 0x504B37C: virNodeDeviceDefParseXML (node_device_conf.c:2122)
==629005==    by 0x504BA0E: virNodeDeviceDefParseNode (node_device_conf.c:2242)
==629005==    by 0x504BADD: virNodeDeviceDefParse (node_device_conf.c:2256)
==629005==    by 0x504BB1E: virNodeDeviceDefParseString (node_device_conf.c:2270)
==629005==    by 0x262229FF: nodeDeviceDefineXML (node_device_driver.c:1290)
==629005==    by 0x520D415: virNodeDeviceDefineXML (libvirt-nodedev.c:768)
==629005==    by 0x14AEED: remoteDispatchNodeDeviceDefineXML (remote_daemon_dispatch_stubs.h:1
5032
==629005==    by 0x14AE78: remoteDispatchNodeDeviceDefineXMLHelper (remote_daemon_dispatch_stu
bs.h:15013)

What happens is that we start with something like:
    ...
    def->name = g_strdup("new_device")
    ...
and we never free it before re-writing it.

My R-b still stands.

Erik