On 11/28/19 1:10 PM, Andrew Cooper wrote: > On 28/11/2019 10:17, George Dunlap wrote: >>> On Nov 28, 2019, at 9:55 AM, Jan Beulich <jbeul...@suse.com> wrote: >>>>>> Has anyone actually tried building a livepatch with this change in place? >>>>> Actually - what is your concern here? The exact spelling of symbols >>>>> names should be of no interest to the tool. After all the compiler is >>>>> free to invent all sorts of names for its local symbols, including >>>>> the ones we would produce with this change in place. All the tool >>>>> cares about is that the names be unambiguous. Hence any (theoretical) >>>>> regression here would be a bug in the tools, which imo is no reason >>>>> to delay this change any further. (Granted I should have got to it >>>>> earlier, but it had been continuing to get deferred.) >>>> This might all be true (theoretically). >>>> >>>> The livepatch build tools are fragile and very sensitive to how the >>>> object files are laid out. There is a very real risk that this change >>>> accidentally breaks livepatching totally, even on GCC builds. >>>> >>>> Were this to happen, it would be yet another 4.13 regression. >>> It's perhaps a matter of perception, but I'd still call this a >>> live patching tools bug then, not a 4.13 regression. >> After the discussion yesterday, I was thinking a bit more about this, and >> I’m not happy with the principle Andy seems to be operating on, > > I'm sorry that you feel that way. > >> that it’s reasonable to completely block a bug-fixing patch to Xen, forcing >> a work-around to be used in a release, because it has unknown effects on >> livepatching. > > This is not a fair characterisation of what is going on here. Ignore > the specifics of this patch - they are not relevant to my objection. > > As a maintainer, it is my responsibility to ensure that crap doesn't get > committed.
Jan's patch is not crap; this is out of line. >> And I do not consider it my responsibility to >> do regression tests of the live patching tools. > > Yes. Yes it really is, when you're making a material change > specifically to this area, with a high chance of adverse impact. Then it looks like we need to have a wider discussion about this, because my answer is, "No, not it's really not." And I don't think you would think so either, except that you happen to use livepatching. Imagine if you posted a patch trying to fix nested HVM, and out of the blue Olaf turned up and said, "Have you tested this with hypervisor paging?" And when you said, "No, I have no idea how to test that", Jan simply blocked the patch indefinitely? You would be angry, and rightly so. As a general principle, the cost of features should be borne by the people who use them. That includes the cost of determining authoritatively whether a change is safe or not -- either by review, or by doing manual testing, or by having automated tests in place to flag up issues. If you had questions about the patch, *you*, with your developer hat on, should have either done testing, or arranged for someone who uses it regularly to do testing to make sure it works. -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
