Re: [Xen-devel] preparations for 4.11.2

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Andrew Cooper writes ("Re: [Xen-devel] preparations for 4.11.2"):
> On 16/05/2019 17:17, Ian Jackson wrote:
> > Andrew Cooper writes ("Re: [Xen-devel] preparations for 4.11.2"):
> >> 129025fe3093 "oxenstored: Don't re-open a xenctrl handle for every
> >> domain introduction"
> > Can you justify how this is a bugfix ?  It doesn't seem like backport
> > material to me.
> 
> It was found from strace (while investigating an unrelated issue), but
> given how many issues we've had in the past with {o,}xenstored exceeding
> its FD limit, I'd still put it in the category of bugfix.
> 
> It balloons the worst-case FD requirements by as many concurrent domain
> starts as the rest of dom0 can manage.

Thanks for the clarification.

> >> 7b20a865bc10 "tools/ocaml: Release the global lock before invoking block
> >> syscalls"
> > This *really* doesn't look like a bugfix, let alone a backport
> > candidate !  Removing a lock for performance reasons !
> 
> Of course its a backport candidate, and it is a bugfix even if most of
> the time it is only observed as a perf improvement.
> 
> The Ocaml FFI says "thou shalt not make a syscall holding this lock",
> because while that lock is held, everything is single threaded.

That wasn't mentioned in the commit message, so I assumed that the
reason was the usual one for removing locks, namely simply to increase
cpu concurrency.

So these are bugfixes, but they're not particularly low risk based
just on the code.  How long has XS been running these patches ?  The
answer to that may give me some confidence that for users of Xen
stable branches, the possible reward of fixing a mysterious bad
behaviour is better to take the risk of these patches having bugs.

> >> c393b64dcee6 "tools/libxc: Fix issues with libxc and Xen having
> >> different featureset lengths"
> > The compatibility implications here are not clearly spelled out in the
> > commit message.  AFAICT, after this commit, the effect is:
> >   - new tools will work with old hypervisor
> >   - old tools will not necessariloy work with old hypervisor
> > I assume that we are talking here about old and new code with the same
> > Xen version, eg as a result of a security fix.
> >
> > The previous behaviour, ie, what happens without this patch, is not
> > entirely clear to me.
> 
> This was an unintended consequence of XSA-253 (Spectre/Meltdown) where
> the length of the featureset did increase in a security fix.

Cripes.

OK, so that is definitely wanted.  It applies cleanly back to 4.8, so
I have done that.  I guess it needs to be applied to 4.7 too ?  I get
conflicts.  Would you care to try to fix that up or shall I ?

> >> 82855aba5bf9 "tools/libxc: Fix error handling in get_cpuid_domain_info()"
> > This might break some callers, mightn't it ?  What callers ?  Or is
> > there an argument that there aren't callers which will be broken ?
> 
> This was from the same bit of debugging as above, and ISTR caused some
> error messages in higher callers to print junk instead of the real error.

Right.  I can see that that is annoying.  But given that the existing
behaviour is inconsistent, it is possible that *other* callers
somewhere are relying on the other behaviour and they may even check
errno values or something.  IOW you are changing the API of this
function from "omg what happens depends on what error it even was" to
something sane and uniform.

That is still an API change though, and maybe not sensible to
backport, if the only benefit is improved error messages.  It seems to
me this would depend on how likely it is that there are any callers
that would be broken by the change and in what situations that would
break.

Thanks for the info.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel