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@xxxxxxxxxx>
> Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
> 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
