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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("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?

Yes.  The two "normal" callbacks are gotptys and finished, which
happen in that order.  The copyfail callbacks may happen in between.
So these are already in execution order.

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

This seemed a fine restriction to me, but I'm not wedded to it.

> 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:
>       STATE_AO_GC(bl->ao)
> instead?

So I will do this.  (I have removed your ack from my copy of that
underlying patch.)

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

Thanks.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel