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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 31/05/2025 20:45, Tu Dinh 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;
This could be dropped. RebootRequestedBy != NULL could be tested instead.

> +    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);
I'm not seeing where this is freed.

> +        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");
>