[PATCH v1 11/20] node_device_udev: Move responsibility to release `(init|udev)Thread` to `udevEventDataDispose`

Marc Hartmayer posted 20 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v1 11/20] node_device_udev: Move responsibility to release `(init|udev)Thread` to `udevEventDataDispose`
Posted by Marc Hartmayer 1 year, 9 months ago
Everything is released in `udevEventDataDispose` except for the threads, change
this as this makes the code easier to read as it can be simplified a little.

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---
 src/node_device/node_device_udev.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 1c8832ebd990..7f233652b461 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -86,12 +86,16 @@ udevEventDataDispose(void *obj)
     struct udev *udev = NULL;
     udevEventData *priv = obj;
 
+    g_clear_pointer(&priv->initThread, g_free);
+
     if (priv->watch != -1)
         virEventRemoveHandle(priv->watch);
 
     if (priv->mdevctlTimeout != -1)
         virEventRemoveTimeout(priv->mdevctlTimeout);
 
+    g_clear_pointer(&priv->udevThread, g_free);
+
     if (!priv->udev_monitor)
         return;
 
@@ -1740,14 +1744,10 @@ nodeStateCleanup(void)
             priv->udevThreadQuit = true;
             virCondSignal(&priv->udevThreadCond);
         }
-        if (priv->initThread) {
+        if (priv->initThread)
             virThreadJoin(priv->initThread);
-            g_clear_pointer(&priv->initThread, g_free);
-        }
-        if (priv->udevThread) {
+        if (priv->udevThread)
             virThreadJoin(priv->udevThread);
-            g_clear_pointer(&priv->udevThread, g_free);
-        }
     }
 
     virObjectUnref(priv);
@@ -2338,7 +2338,6 @@ nodeStateInitialize(bool privileged,
                             "udev-event", false, NULL) < 0) {
         virReportSystemError(errno, "%s",
                              _("failed to create udev handler thread"));
-        g_clear_pointer(&priv->udevThread, g_free);
         goto unlock;
     }
 
@@ -2370,7 +2369,6 @@ nodeStateInitialize(bool privileged,
                             "nodedev-init", false, udev) < 0) {
         virReportSystemError(errno, "%s",
                              _("failed to create udev enumerate thread"));
-        g_clear_pointer(&priv->initThread, g_free);
         goto cleanup;
     }
 
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v1 11/20] node_device_udev: Move responsibility to release `(init|udev)Thread` to `udevEventDataDispose`
Posted by Boris Fiuczynski 1 year, 9 months ago
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>

On 4/19/24 16:49, Marc Hartmayer wrote:
> Everything is released in `udevEventDataDispose` except for the threads, change
> this as this makes the code easier to read as it can be simplified a little.
> 
> Signed-off-by: Marc Hartmayer<mhartmay@linux.ibm.com>
> ---
>   src/node_device/node_device_udev.c | 14 ++++++--------
>   1 file changed, 6 insertions(+), 8 deletions(-)

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v1 11/20] node_device_udev: Move responsibility to release `(init|udev)Thread` to `udevEventDataDispose`
Posted by Jonathon Jongsma 1 year, 9 months ago
On 4/19/24 9:49 AM, Marc Hartmayer wrote:
> Everything is released in `udevEventDataDispose` except for the threads, change
> this as this makes the code easier to read as it can be simplified a little.
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>   src/node_device/node_device_udev.c | 14 ++++++--------
>   1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 1c8832ebd990..7f233652b461 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -86,12 +86,16 @@ udevEventDataDispose(void *obj)
>       struct udev *udev = NULL;
>       udevEventData *priv = obj;
>   
> +    g_clear_pointer(&priv->initThread, g_free);
> +
>       if (priv->watch != -1)
>           virEventRemoveHandle(priv->watch);
>   
>       if (priv->mdevctlTimeout != -1)
>           virEventRemoveTimeout(priv->mdevctlTimeout);
>   
> +    g_clear_pointer(&priv->udevThread, g_free);
> +
>       if (!priv->udev_monitor)
>           return;
>   
> @@ -1740,14 +1744,10 @@ nodeStateCleanup(void)
>               priv->udevThreadQuit = true;
>               virCondSignal(&priv->udevThreadCond);
>           }
> -        if (priv->initThread) {
> +        if (priv->initThread)
>               virThreadJoin(priv->initThread);
> -            g_clear_pointer(&priv->initThread, g_free);
> -        }
> -        if (priv->udevThread) {
> +        if (priv->udevThread)
>               virThreadJoin(priv->udevThread);
> -            g_clear_pointer(&priv->udevThread, g_free);
> -        }
>       }
>   
>       virObjectUnref(priv);
> @@ -2338,7 +2338,6 @@ nodeStateInitialize(bool privileged,
>                               "udev-event", false, NULL) < 0) {
>           virReportSystemError(errno, "%s",
>                                _("failed to create udev handler thread"));
> -        g_clear_pointer(&priv->udevThread, g_free);
>           goto unlock;
>       }
>   
> @@ -2370,7 +2369,6 @@ nodeStateInitialize(bool privileged,
>                               "nodedev-init", false, udev) < 0) {
>           virReportSystemError(errno, "%s",
>                                _("failed to create udev enumerate thread"));
> -        g_clear_pointer(&priv->initThread, g_free);
>           goto cleanup;
>       }
>   

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org