Re: REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

["Jan Beulich" <jbeulich@suse.com>]

>>> George Dunlap <george.dunlap@citrix.com> 04/14/16 6:20 PM >>>
>On Thu, Apr 14, 2016 at 4:59 PM, Jan Beulich <jbeulich@suse.com> wrote:
>>>>> George Dunlap <george.dunlap@citrix.com> 04/14/16 5:16 PM >>>
>>>On the other hand, I think there's a bit of a faulty interpretation of
>>>the procedure here.  Jan reviewed the patch thoroughly and then acked
>>>it; on the basis of that, Konrad legitimately checked it in.  After it
>>>was checked in Jan said, "I've changed my mind and withdraw my Ack";
>>>and the assumption of the subsequent conversation was that an ack
>>>*can* be withdrawn after it has been legitimately checked in, and that
>>>if no other Ack is supplied, then it must be reverted.
>>>
>>>I don't think that's a correct interpretation of the rules.  Reviewers
>>>in general, and maintainers in particular, should make reasonably sure
>>>that they mean the Ack before they give it; and if they change their
>>>mind after it has been legitimately checked in, then it's now up to
>>>them to make the change they want to see according to the regular
>>>procedure.  That is, if Jan wants it reverted, he needs to post a
>>>patch reverting it and get Acks from the appropriate maintainers; and
>>>the discussion needs to be around Jan's reversion being accepted, not
>>>about Konrad's original patch continuing to be accepted.  (Obvious
>>>exceptions can be made in the case of emergencies like build
>>>breakages.)
>>
>> Fundamentally I agree, but I think there's more to this than just following
>> a set of rules. For example, please don't forget the time pressure due to
>> the (at that time) rapidly approaching freeze date. And then, mistakes
>> happen, and so I made a mistake here by sending the ack a few hours too
>> early.
>
>Sure, mistakes happen; but I hope it's not being to controversial to
>say that in general, the procedure should be arranged such that the
>person who makes the mistake is the one who has to do deal with the
>most consequences from the mistake, not the people around him.  I
>mean, if you asked a bartender for a bottle of beer, and after he'd
>already opened it you said, "Oh sorry, I actually wanted a cider", it
>would be fair enough for the bartender to ask you to pay for the beer,
>rather than eating it*, wouldn't it? :-)

Sure. And I think that's what I've done. I suggested to revert the thing,
collecting opinions either direction. I just didn't post a revert patch, as I
think that makes little sense - a revert is a mechanical operation, which
doesn't need people looking at the actual patch.

>Well Ian and I have already given our opinions -- Ian thinks moving to
>a clean interface and deprecating the old one is in general a good
>thing, and doesn't look too painful in this case.  I don't really see
>a problem with using a large fixed size, but I don't fundamentally see
>a problem with moving to a new clean interface either.  Given that
>Andy has a strong aversion to the way things are, if you had only a
>mild distaste rather than a  strong objection to the new hypercall, it
>would probably make sense to go with the new hypercall.
>
>If you do have a strong objection, then maybe we could try to convince
>Andy to accept adding subops with different calling semantics to the
>existing hypercall.  But I would still think the burden of persuasion
>is primarily on you.

 I do not have a strong objection, or else I would have converted my ack
into a nak instead of just withdrawing it. I just dislike the duplication, and
hence I'm not happy with me now being the one having approved it to go
in. Hence the request for a replacement ack (or whatever else referee
decision).

And btw., considering that Konrad has already posted a revert patch,
and I have ack-ed that one, this could now go in right away (and the
discussion could either be settled or start over).

Jan


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