Re: [XENBUS PATCH 1/2] Add autoreboot retry logic

[Owen Smith <owen.smith@xxxxxxxxx>]

Looks good.
The AutoReboot option does require the VM admin to set a registry key, so its opt in and may not need a cancel option with the 'system busy' check.

Reviewed-by: Owen Smith <owen.smith@xxxxxxxxx>

On Sat, May 31, 2025 at 8:46 PM Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> wrote:

Add a new context flag RebootRequested, to be set in PromptForReboot.
Add a timer that calls TryAutoReboot() every minute if reboot has been
requested beforehand.

Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
---
IMO the autoreboot logic needs some changes. At the moment, autoreboot is
triggered without giving the user a chance to cancel the reboot.
---
 src/monitor/monitor.c | 74 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 14 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 185838f..faf1155 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -57,10 +57,13 @@ typedef struct _MONITOR_CONTEXT {
     HANDLE                  EventLog;
     HANDLE                  StopEvent;
     HANDLE                  RequestEvent;
+    HANDLE                  Timer;
     HKEY                    RequestKey;
     PTCHAR                  Title;
     PTCHAR                  Text;
     PTCHAR                  Question;
+    BOOL                    RebootRequested;
+    PTCHAR                  RebootRequestedBy;
     BOOL                    RebootPending;
 } MONITOR_CONTEXT, *PMONITOR_CONTEXT;

@@ -604,17 +607,24 @@ PromptForReboot(

     Log("====> (%s)", DriverName);

+    Context->RebootRequested = TRUE;
+    if (!Context->RebootRequestedBy) {
+        Context->RebootRequestedBy = _tcsdup(DriverName);
+        if (!Context->RebootRequestedBy)
+            goto fail1;
+    }
+
     Title = Context->Title;
     TitleLength = (DWORD)((_tcslen(Context->Title) +
                            1) * sizeof (TCHAR));

     // AutoReboot is set, DoReboot has been called
-    if (TryAutoReboot(DriverName))
+    if (TryAutoReboot(Context->RebootRequestedBy))
         goto done;

-    DisplayName = GetDisplayName(DriverName);
+    DisplayName = GetDisplayName(Context->RebootRequestedBy);
     if (DisplayName == NULL)
-        goto fail1;
+        goto fail2;

     Description = _tcsrchr(DisplayName, ';');
     if (Description == NULL)
@@ -631,7 +641,7 @@ PromptForReboot(

     Text = calloc(1, TextLength);
     if (Text == NULL)
-        goto fail2;
+        goto fail3;

     Result = StringCbPrintf(Text,
                             TextLength,
@@ -647,7 +657,7 @@ PromptForReboot(
                                    &SessionInfo,
                                    &Count);
     if (!Success)
-        goto fail3;
+        goto fail4;

     for (Index = 0; Index < Count; Index++) {
         DWORD                   SessionId = SessionInfo[Index].SessionId;
@@ -678,7 +688,7 @@ PromptForReboot(
                                  TRUE);

         if (!Success)
-            goto fail4;
+            goto fail5;

         Context->RebootPending = TRUE;

@@ -697,28 +707,31 @@ done:

     return;

-fail4:
-    Log("fail4");
+fail5:
+    Log("fail5");

     WTSFreeMemory(SessionInfo);

+fail4:
+    Log("fail4");
+
 fail3:
     Log("fail3");

-fail2:
-    Log("fail2");
-
     free(DisplayName);

-fail1:
+fail2:
     Error = GetLastError();

     {
         PTCHAR  Message;
         Message = GetErrorMessage(Error);
-        Log("fail1 (%s)", Message);
+        Log("fail2 (%s)", Message);
         LocalFree(Message);
     }
+
+fail1:
+    Log("fail1");
 }

 static VOID
@@ -1243,6 +1256,7 @@ MonitorMain(
     PTCHAR              RequestKeyName;
     BOOL                Success;
     HRESULT             Error;
+    LARGE_INTEGER       DueTime;

     UNREFERENCED_PARAMETER(argc);
     UNREFERENCED_PARAMETER(argv);
@@ -1314,16 +1328,32 @@ MonitorMain(
     if (!Success)
         goto fail9;

+    Context->Timer = CreateWaitableTimer(NULL, FALSE, NULL);
+    if (Context->Timer == NULL)
+        goto fail10;
+
+    DueTime.QuadPart = -600000000; // 1 minute
+
+    Success = SetWaitableTimer(Context->Timer,
+                               &DueTime,
+                               60000,
+                               NULL,
+                               NULL,
+                               FALSE);
+    if (!Success)
+        goto fail11;
+
     SetEvent(Context->RequestEvent);

     ReportStatus(SERVICE_RUNNING, NO_ERROR, 0);

     for (;;) {
-        HANDLE  Events[2];
+        HANDLE  Events[3];
         DWORD   Object;

         Events[0] = Context->StopEvent;
         Events[1] = Context->RequestEvent;
+        Events[2] = Context->Timer;

         Log("waiting (%u)...", ARRAYSIZE(Events));
         Object = WaitForMultipleObjects(ARRAYSIZE(Events),
@@ -1342,12 +1372,20 @@ MonitorMain(
             CheckRequestKey();
             break;

+        case WAIT_OBJECT_0 + 2:
+            if (Context->RebootRequested)
+                TryAutoReboot(Context->RebootRequestedBy);
+            break;
+
         default:
             break;
         }
     }

 done:
+    CancelWaitableTimer(Context->Timer);
+    CloseHandle(Context->Timer);
+
     (VOID) RegDeleteTree(Context->RequestKey, NULL);

     free(Context->Question);
@@ -1369,6 +1407,14 @@ done:

     return;

+fail11:
+    Log("fail11");
+
+    CloseHandle(Context->Timer);
+
+fail10:
+    Log("fail10");
+
 fail9:
     Log("fail9");

--
2.49.0.windows.1



Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech