Re: [Xen-devel] [PATCH 11/11] add vtpm support to libxl

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-09-28 at 17:39 +0100, George Dunlap wrote:
> >>> @@ -1084,13 +1090,13 @@ static void domcreate_devmodel_started(libxl__egc 
> >>> *egc,
> >>>      if (d_config->num_nics > 0) {
> >>>          /* Attach nics */
> >>>          libxl__multidev_begin(ao, &dcs->multidev);
> >>> -        dcs->multidev.callback = domcreate_attach_pci;
> >>> +        dcs->multidev.callback = domcreate_attach_vtpms;
> >> Wow -- what a weird convention you've had to try to figure out and
> >> modify here.  Well done. :-)
> > It was really tricky. Is there no better way to handle asynchronous
> > code? This method seems really error prone and almost impossible to follow.
> 
> Well I didn't write it. :-)  I haven't taken the time to figure out
> why it might have been written that way; but at first glance, I tend
> to agree with you.  For about 10 minutes I was convinced you had made
> some kind of weird error, by sprinkling "vtpm" around things that
> obviously were supposed to be about nics and pci devices, until I
> realized you were just following the existing "call chain" convention.

One big improvement would be to stop treating the preamble of "do_bar"
as the error handling of the preceding "do_foo", in the case where bar
happens to follow foo in the async chain.

For example nic attach's completion handler should be called
domcreate_nicattach_done, which then calls the next step instead of
putting the nic error handling at the front of domcreate_attach_pci
because that happens to be the next call in the chain. It's not quite
ideal but it would be substantially less confusing.

If there was further some way to make the what to call next step table
driven then even better.

Ian.


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