Re: [Xen-devel] [PATCH v4] libxc: check for various libxc failures and pass them down

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-11-05 at 17:58 +1300, Matthew Daley wrote:
> On Tue, Nov 5, 2013 at 6:31 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Sun, 2013-11-03 at 12:08 +1300, Matthew Daley wrote:
> >>  static int xc_cpuid_policy(
> >>      xc_interface *xch, domid_t domid,
> >>      const unsigned int *input, unsigned int *regs)
> >>  {
> >> +    int                 rc;
> >>      xc_dominfo_t        info;
> >>
> >> -    if ( xc_domain_getinfo(xch, domid, 1, &info) == 0 )
> >> +    rc = xc_domain_getinfo(xch, domid, 1, &info);
> >> +    if ( rc < 0 )
> >> +        return rc;
> >> +    if ( rc == 0 )
> >>          return -EINVAL;
> >
> > I think this means that this function now returns a mixture of -1 (with
> > errno set) and -errno depending on the failure mode. libxc really is a
> > horrible chuffing mess in this regard, sorry.
> 
> I noticed :)
> 
> >
> > Luckily nothing right now looks at the return value (you are adding
> > those checks to the caller in this patch) so I think you can just
> > convert this into { errno=EINVAL; return -1; }
> >
> > Then again, it looks like xc_cpuid_set (the aforementioned called)
> > already returns both styles (explicit rc = -EINVAL, vs returning the
> > result of do_domctl which is -1 and set errno). Gah!
> >
> > Hah, but no one looks at the specific value of the result of
> > xc_cpuid_set on failure: libxl doesn't check it at all, Python and ocaml
> > bindings turn it into a generic exception using xc_get_last_error.
> >
> > WTF, xc_get_last_error is a *third* error handling mechanism in libxc,
> > apparently built into the logging infrastructure. Which isn't used at
> > all on this code path AFAICT.
> >
> > What a terrible mess. I think I should just apply this patch and putthe
> > rug back over the rest of it :-(
> 
> Perhaps this patch should be dropped and the issues put in a "take a
> proper look at later" pile?

I'm ok with that. Sorry it took me until v4 to realise what a rats nest
it would be to fix.

Ian.


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