[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 29/31] libxl: ao: Convert libxl_run_bootloader

> +static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op);
> +static void bootloader_keystrokes_copyfail(libxl__egc *egc,
> +       libxl__datacopier_state *dc, int onwrite, int errnoval);
> +static void bootloader_display_copyfail(libxl__egc *egc,
> +       libxl__datacopier_state *dc, int onwrite, int errnoval);
> +static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
> +                                pid_t pid, int status);

I suppose that in here there is a sequence of callbacks which get
chained together? Might be worth pulling those out and ordering them in
the normal expected chain of execution?

> +/*----- synchronous subroutines -----*/
> +
> +static int setup_xenconsoled_pty(libxl__egc *egc, libxl__bootloader_state 
> *bl,
> +                                 char *slave_path, size_t slave_path_len)
>  {
> +    STATE_AO_GC(bl);

I wondered about this in the patch which defined it but didn't really
consider it til I saw this user.

I didn't quite realise back then that STATE_AO_GC takes a pointer to a
struct which is user (a libxl internal user, but still) defined and
requires that it has a member "ao". I don't mind that for cases where
the struct is defined alongside the macro as part of a while API but for
separately defined structs its a bit nasty. Could it be:

I suppose that isn't really a criticism of this patch but rather of the
patch which added the macro, so this patch in itself is:

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.