Re: [Xen-devel] Unable to create VM with nic device on Arndale

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-12-18 at 14:44 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] Unable to create VM with nic device on 
> Arndale"):
> > The confusion arises because our LOG macros include the function name,
> > so checking for nic errors in a vtpm function is always going to produce
> > something confusing.
> >
> > We should introduce a veneer here, e.g. nic_done(), which logs any nic
> > related errors and then calls the next step. Same for each step in the
> > async chain.
> 
> That would be tangling the code just to make the logging output
> right.

I'm not sure of that, I think having paired start+done functions would
actually be clearer in its own right, but it would also make the error
paths less obtuse IMHO, you'd avoid every function being:
        if (some previous failure)
                LOG(something about previous)
                goto out

        setup something completely unrelated to previous

        out:
                next step

In the specific case of nics you would probably build the done function
into the multidev thing with a separate callback for success vs failure.

We already have the start+done pairing in some cases (e.g. the dm
started case).

>   Perhaps we could just fix the logging to DTRT.
> 
> How about a new logging macro which takes an argument for the context
> to print instead of the function name ?
> 
>    static void domcreate_attach_vtpms(libxl__egc *egc,
>                                       libxl__multidev *multidev,
>                                       int ret)
>    {
> ...
>       libxl_domain_config* const d_config = dcs->guest_config;
> 
>       if (ret) {
> -         LOG(ERROR, "unable to add nic devices");
> +         CLOG("domcreate nics", ERROR, "unable to add nic devices");
>           goto error_out;
>       }
> 
> We'd probably only need to change about one call in each callback
> function - the one which prints an error message for the previous
> operation.
> 
> Ian.



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