Use the proper driver functions for the node state shutdown preparation and
wait. In the next patch, these functions will be extended.

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

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 672da8f5a19f..cec7d837c43e 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1736,18 +1736,8 @@ nodeStateCleanup(void)
 
     priv = driver->privateData;
     if (priv) {
-        VIR_WITH_OBJECT_LOCK_GUARD(priv) {
-            priv->udevThreadQuit = true;
-            virCondSignal(&priv->udevThreadCond);
-        }
-        if (priv->initThread) {
-            virThreadJoin(priv->initThread);
-            g_clear_pointer(&priv->initThread, g_free);
-        }
-        if (priv->udevThread) {
-            virThreadJoin(priv->udevThread);
-            g_clear_pointer(&priv->udevThread, g_free);
-        }
+        g_clear_pointer(&priv->initThread, g_free);
+        g_clear_pointer(&priv->udevThread, g_free);
     }
 
     virObjectUnref(priv);
@@ -2440,12 +2430,52 @@ static virConnectDriver udevConnectDriver = {
     .nodeDeviceDriver = &udevNodeDeviceDriver,
 };
 
+static int
+nodeStateShutdownPrepare(void)
+{
+    udevEventData *priv = NULL;
+
+    if (!driver)
+        return 0;
+
+    priv = driver->privateData;
+    if (!priv)
+        return 0;
+
+    VIR_WITH_OBJECT_LOCK_GUARD(priv) {
+        priv->udevThreadQuit = true;
+        virCondSignal(&priv->udevThreadCond);
+    }
+    return 0;
+}
+
+static int
+nodeStateShutdownWait(void)
+{
+    udevEventData *priv = NULL;
+
+    if (!driver)
+        return 0;
+
+    priv = driver->privateData;
+    if (!priv)
+        return 0;
+
+    if (priv->initThread)
+      virThreadJoin(priv->initThread);
+
+    if (priv->udevThread)
+      virThreadJoin(priv->udevThread);
+    return 0;
+}
 
 static virStateDriver udevStateDriver = {
     .name = "udev",
     .stateInitialize = nodeStateInitialize, /* 0.7.3 */
     .stateCleanup = nodeStateCleanup, /* 0.7.3 */
     .stateReload = nodeStateReload, /* 0.7.3 */
+    .stateShutdownPrepare = nodeStateShutdownPrepare,
+    .stateShutdownWait = nodeStateShutdownWait,
 };
 
 
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On 4/12/24 8:36 AM, Marc Hartmayer wrote:
> Use the proper driver functions for the node state shutdown preparation and
> wait. In the next patch, these functions will be extended.
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>   src/node_device/node_device_udev.c | 54 +++++++++++++++++++++++-------
>   1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 672da8f5a19f..cec7d837c43e 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1736,18 +1736,8 @@ nodeStateCleanup(void)
>   
>       priv = driver->privateData;
>       if (priv) {
> -        VIR_WITH_OBJECT_LOCK_GUARD(priv) {
> -            priv->udevThreadQuit = true;
> -            virCondSignal(&priv->udevThreadCond);
> -        }
> -        if (priv->initThread) {
> -            virThreadJoin(priv->initThread);
> -            g_clear_pointer(&priv->initThread, g_free);
> -        }
> -        if (priv->udevThread) {
> -            virThreadJoin(priv->udevThread);
> -            g_clear_pointer(&priv->udevThread, g_free);
> -        }
> +        g_clear_pointer(&priv->initThread, g_free);
> +        g_clear_pointer(&priv->udevThread, g_free);
>       }
>   
>       virObjectUnref(priv);
> @@ -2440,12 +2430,52 @@ static virConnectDriver udevConnectDriver = {
>       .nodeDeviceDriver = &udevNodeDeviceDriver,
>   };
>   
> +static int
> +nodeStateShutdownPrepare(void)
> +{
> +    udevEventData *priv = NULL;
> +
> +    if (!driver)
> +        return 0;
> +
> +    priv = driver->privateData;
> +    if (!priv)
> +        return 0;
> +
> +    VIR_WITH_OBJECT_LOCK_GUARD(priv) {
> +        priv->udevThreadQuit = true;
> +        virCondSignal(&priv->udevThreadCond);
> +    }
> +    return 0;
> +}
> +
> +static int
> +nodeStateShutdownWait(void)
> +{
> +    udevEventData *priv = NULL;
> +
> +    if (!driver)
> +        return 0;
> +
> +    priv = driver->privateData;
> +    if (!priv)
> +        return 0;
> +
> +    if (priv->initThread)
> +      virThreadJoin(priv->initThread);
> +
> +    if (priv->udevThread)
> +      virThreadJoin(priv->udevThread);
> +    return 0;
> +}
>   
>   static virStateDriver udevStateDriver = {
>       .name = "udev",
>       .stateInitialize = nodeStateInitialize, /* 0.7.3 */
>       .stateCleanup = nodeStateCleanup, /* 0.7.3 */
>       .stateReload = nodeStateReload, /* 0.7.3 */
> +    .stateShutdownPrepare = nodeStateShutdownPrepare,
> +    .stateShutdownWait = nodeStateShutdownWait,
>   };
>   
>   


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

On Thu, Apr 18, 2024 at 05:01 PM -0500, Jonathon Jongsma <jjongsma@redhat.com> wrote:
> On 4/12/24 8:36 AM, Marc Hartmayer wrote:
>> Use the proper driver functions for the node state shutdown preparation and
>> wait. In the next patch, these functions will be extended.
>> 
>> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> ---
>>   src/node_device/node_device_udev.c | 54
>> +++++++++++++++++++++++-------

[…snip…]

>
>
> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>

Thanks, but I found some issues/improvements for this patch (or maybe
I’ve to split them into two patches), the new version of the prepare
will do the following:

nodeStateShutdownPrepare:
1. unref all priv->mdevctlMonitors
2. remove priv->watch
3. remove priv->timeout
4. set priv->udevThreadQuit = true and virConSignal...
5. virWorkerPoolStop(...)


In addition, the new functions …ShutdownPrepare and …ShutdownWait must
be called in the cleanup path of nodeStateInitialize for a proper
cleanup, because before this change these things were done in
‘nodeStateCleanup‘ and this function is already called in the cleanup
path of nodeStateInitialize.

Fine with you? If yes, I’ll send a new version.

>
-- 
Kind regards / Beste Grüße
   Marc Hartmayer

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

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>

On 4/12/24 15:36, Marc Hartmayer wrote:
> Use the proper driver functions for the node state shutdown preparation and
> wait. In the next patch, these functions will be extended.
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>   src/node_device/node_device_udev.c | 54 +++++++++++++++++++++++-------
>   1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 672da8f5a19f..cec7d837c43e 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1736,18 +1736,8 @@ nodeStateCleanup(void)
>   
>       priv = driver->privateData;
>       if (priv) {
> -        VIR_WITH_OBJECT_LOCK_GUARD(priv) {
> -            priv->udevThreadQuit = true;
> -            virCondSignal(&priv->udevThreadCond);
> -        }
> -        if (priv->initThread) {
> -            virThreadJoin(priv->initThread);
> -            g_clear_pointer(&priv->initThread, g_free);
> -        }
> -        if (priv->udevThread) {
> -            virThreadJoin(priv->udevThread);
> -            g_clear_pointer(&priv->udevThread, g_free);
> -        }
> +        g_clear_pointer(&priv->initThread, g_free);
> +        g_clear_pointer(&priv->udevThread, g_free);
>       }
>   
>       virObjectUnref(priv);
> @@ -2440,12 +2430,52 @@ static virConnectDriver udevConnectDriver = {
>       .nodeDeviceDriver = &udevNodeDeviceDriver,
>   };
>   
> +static int
> +nodeStateShutdownPrepare(void)
> +{
> +    udevEventData *priv = NULL;
> +
> +    if (!driver)
> +        return 0;
> +
> +    priv = driver->privateData;
> +    if (!priv)
> +        return 0;
> +
> +    VIR_WITH_OBJECT_LOCK_GUARD(priv) {
> +        priv->udevThreadQuit = true;
> +        virCondSignal(&priv->udevThreadCond);
> +    }
> +    return 0;
> +}
> +
> +static int
> +nodeStateShutdownWait(void)
> +{
> +    udevEventData *priv = NULL;
> +
> +    if (!driver)
> +        return 0;
> +
> +    priv = driver->privateData;
> +    if (!priv)
> +        return 0;
> +
> +    if (priv->initThread)
> +      virThreadJoin(priv->initThread);
> +
> +    if (priv->udevThread)
> +      virThreadJoin(priv->udevThread);
> +    return 0;
> +}
>   
>   static virStateDriver udevStateDriver = {
>       .name = "udev",
>       .stateInitialize = nodeStateInitialize, /* 0.7.3 */
>       .stateCleanup = nodeStateCleanup, /* 0.7.3 */
>       .stateReload = nodeStateReload, /* 0.7.3 */
> +    .stateShutdownPrepare = nodeStateShutdownPrepare,
> +    .stateShutdownWait = nodeStateShutdownWait,
>   };
>   
>   

-- 
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