Nothing here is critical enough to go into 4.17 at this juncture. Various notes/observations from having spent a day trying to untangle things.
1) Patches 5/6 are a single bugfix and need merging. Except there was also an error when taking feedback from the list, and the net result regresses the original optimisation. I have a fix sorted in my local queue. 2) The indentation fix (not attached to this series) should scope the logic, not delete a debug line which was presumably added for a good reason. I've got a fix to this effect in my local queue, and we can discuss the pros/cons of the approach in due course. 3) Patch 1, evtchn Ocaml 5.0 compat, is still missing some corrections which I gave on earlier postings. I've fixed it up locally in my queue. I also notice, while reviewing the whole, that stub_eventchn_init() passes NULL as a logger, which has the side effect of libxenevtchn instantiating a default logger which takes control of stdout/stderr. Without starting the fight over toxic library behaviour yet again, it occurs to me in the context of Patch 13, uncaught exception handler, that in oxenstored, any logging from the C level needs to end up elsewhere. While we do have ocaml bindings for xentoollog, nothing uses it, and none of the other libraries (save xl, which isn't used) have a way of passing the Ocaml Xentoollog down. This probably wants rethinking, one way or another. 4) Patches 2/3. All these libraries have far worse problems than evtchn, because they can easily use-after-free. They all need to be Custom with a finaliser. 5) Patch 4. The commit message says "A better solution is being worked on for master", but this is master. Also, it's not a prerequisite for a security fix; merely something to make a developers life easier. 6) The re-indent patch. Policies of when to do it aside, having tried using it, the format adjustment is incomplete (running ocp-indent gets me deltas in files I haven't touched), and there needs to be some .gitignore changes. That said, it is usually frowned upon to have logic depending on being in a git tree. This was perhaps a bigger deal back when we used hg by default and mirrored into multiple SCMs, but it's still expected not to rely on this. 7) Patch 8, evtchn fdopen, is two separate patches. One adding fdopen, and one adding a NOCLOEXEC argument to the existing init. They want splitting in two. fdopen() ought to pass flags so we don't have to break the ABI again when there is a flag needing passing, and cloexec probably shouldn't be a boolean. We should either pass a raw int32, or a list-of-enums like we do in the xenctrl stubs. Also, this patch has inherited errors from patch 1. 9) Patches 8 thru 15 need to be the other side of the intent patch, because they need backporting to branches which will never get it. This is why bugfixes always go at the head of a patch series, and improvements at the tail. 10) Patch 12 talks about default log levels, but that's bogus reasoning. The messages should be warnings because they non-fatal exceptional cases. 11) Patch 14 talks about using caml_stat_strdup(), but doesn't. ~Andrew
