Re: [PATCH 3/3] tools/xl: move some logic needed only once earlier in

[chunjie.zhu@xxxxxxxxxx]

On Mon, Oct 12, 2026 at 06:03:49 PM, Elliott Mitchell <ehem+xen@xxxxxxx> wrote:

> The value of dom_info->console_autoconnect isn't going to change if a
> domain is rebooted.  Nor does this logic need the lock to be held.  As
> such moving this earlier makes sense.
>
> The policy domain is set once and then never changed.  Similar to above
> simply move before the reboot start point.
>
> Signed-off-by: Elliott Mitchell <ehem+xen@xxxxxxx>
> ---
>  tools/xl/xl_vmcontrol.c | 32 +++++++++++++++-----------------
>  1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 76c2a07ee8..ae7985122f 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -717,7 +717,8 @@ int create_domain(struct domain_create *dom_info)
>      int restore_fd = -1;
>      int restore_fd_to_close = -1;
>      int send_back_fd = -1;
> -    const libxl_asyncprogress_how *autoconnect_console_how;
> +    const libxl_asyncprogress_how *autoconnect_console_how = NULL;
> +    libxl_asyncprogress_how autoconnect_console_how_buf;
>      int notify_pipe[2] = { -1, -1 };
>      struct save_file_header hdr;
>      uint32_t domid_soft_reset = INVALID_DOMID;
> @@ -921,6 +922,19 @@ int create_domain(struct domain_create *dom_info)
>      if (dryrun_only)
>          goto out;
>
> +    if (dom_info->console_autoconnect) {
> +        if (libxl_pipe(ctx, notify_pipe)) {
> +            ret = ERROR_FAIL;
> +            goto error_out;
> +        }

It looks like a functional regression for the reboot path. Right before
"goto start;" on the guest-reboot path there is:

    /*
     * Do not attempt to reconnect if we come round again due to a
     * guest reboot -- the stdin/out will be disconnected by then.
     */
    dom_info->console_autoconnect = 0;

On reboot, dom_info->console_autoconnect is cleared before "goto start",
but autoconnect_console_how now stays non-NULL, since it is no longer
recomputed after "start:". The second create call will invoke
autoconnect_console with the already-closed notify_pipe[1], and the
read(notify_pipe[0]) will fail with EBADF.

> +        autoconnect_console_how_buf.callback = autoconnect_console;
> +        autoconnect_console_how_buf.for_callback = &notify_pipe[1];
> +        autoconnect_console_how = &autoconnect_console_how_buf;
> +    }
> +
> +    if (!libxl_domid_valid_guest(d_config.c_info.domid))
> +        d_config.c_info.domid = domid_policy;
> +
>  start:
>      assert(domid == INVALID_DOMID);
>
> @@ -938,22 +952,6 @@ start:
>          }
>      }
>
> -    libxl_asyncprogress_how autoconnect_console_how_buf;
> -    if ( dom_info->console_autoconnect ) {
> -        if (libxl_pipe(ctx, notify_pipe)) {
> -            ret = ERROR_FAIL;
> -            goto error_out;
> -        }
> -        autoconnect_console_how_buf.callback = autoconnect_console;
> -        autoconnect_console_how_buf.for_callback = &notify_pipe[1];
> -        autoconnect_console_how = &autoconnect_console_how_buf;
> -    }else{
> -        autoconnect_console_how = 0;
> -    }
> -
> -    if (!libxl_domid_valid_guest(d_config.c_info.domid))
> -        d_config.c_info.domid = domid_policy;
> -
>      if ( restoring ) {
>          libxl_domain_restore_params params;
>
> -- 
> 2.47.3
>
>
>