Re: [PATCH v5 01/11] xen/arm: xc_domain_ioport_permission(..) not supported on ARM.

[Ian Jackson <iwj@xenproject.org>]

Bertrand Marquis writes ("Re: [PATCH v5 01/11] xen/arm: xc_domain_ioport_permission(..) not supported on ARM."):
> I can add something in the commit message about the fact that we improve
> performance and prevent to do a call that is and will not be supported in Xen.

Thanks but I'm afraid I don't think that is a correct summary of the
thread.  Nor would it be an adequate justification for the change.  At
least, not unless you plan to write something considerably longer (and
more precise).

Firstly, I'm not convinced this change would be justified by the
performance impact.  This is a small number of hypercalls during
domain startup.  Usually none, I think ?  If someone wants to optimise
domain startup speed then I am very open to that but I think this
change will make negligible change in practice.  Unless someone wants
to tell me I'm wrong about that ?  And if I am wrong about that then
an explanation of why my suppositions are wrong ought to go in the
commit message.

Secondly, there is no justification there for the change in error
status.

Why is this change needed ?  (What goes wrong if it is omitted ?)
That is what the commit message ought to answer.

Plus, given that it stubs out a function to make it into a no-op, that
itself requires an explanation.  Why is it OK for this function which
is supposed to do a thing, to in fact not do anything at all and
return successfully saying "yes I did that" ?

I think (having read the thread) that I know the answers to these
questions but it needs to be clearly and explicitly written down.

> I saw your change in CODING_STYLE and I understand the request.
> I will try to see if we can handle this change before the feature freeze.

Thanks.  I doubt that this will be hard.  I am more worried about the
commit message.

Indeed, since we haven't had the rationale for this change explicitly
written down, there is a risk that when we do so, we will discover
some problem with the approach that we had previously overlooked.

Discovering that kind of thing is one reason to explicitly write down
why we are doing what we are doing, but this situation does mean we
shouldn't feel we've yet achieved confidence that this patch is right.

Sorry,
Ian.