Move xenopsd's xenctrl_ext to ocaml/libs/ #6730
Conversation
|
I was under the impression that this was deliberate, so Xapi wasn't linked against libxenctrl. |
|
If we want to expose Numa data in RRDs, access to xenctrl would be convenient. But I am happy to learn why this is a bad idea. |
There's a test in |
But what is the motivation? @robhoes |
|
In any case: |
|
Moving We removed all |
ocaml/xenopsd/c_stubs/dune
Outdated
| (names tuntap_stubs xenctrlext_stubs) | ||
| (names tuntap_stubs) | ||
| ) | ||
| (c_library_flags (-L/lib64 -lxenforeignmemory)) |
There was a problem hiding this comment.
It probably doesn't need this anymore, nor the library dependency on xenctrl? The tuntap_stubs seem to just use some standard linux headers.
Maybe we want to add the test to more daemons as well? xcp-rrdd comes to mind
I wouldn't like to maintain the patchequeue either, having said this, I don't understand how is moving the module to a new library preventing the patchqueue, could you explain which mechanism does this move unlock? |
xcp-rrdd (and its plugins) link to xenctrl purposefully |
My understanding is that the mock xenctrl library's stubs live in xen.spec; (if interfaces change this requires a change to the library+xs-opam update). xenctrlext's stubs live right next to it in the xapi source tree. This speeds up iteration on these stubs. |
While I agree that some plugins need to link to xenctrl, I don't think xcp-rrdd should. There's a single usage still standing, to fetch the active domains' UUIDs. They should be fetched from the plugins collected, IMO. |
lindig
force-pushed
the
private/christianlin/xenctrl
branch
from
3b9bccb to
a6bd28a
Compare
| (language c) | ||
| (names xenctrlext_stubs) | ||
| ) | ||
| (c_library_flags (-L/lib64 -lxenforeignmemory)) |
There was a problem hiding this comment.
-L/lib64 is surely not needed.
ocaml/xenopsd/c_stubs/dune
Outdated
| (names tuntap_stubs) | ||
| ) | ||
| (c_library_flags (-L/lib64 -lxenforeignmemory)) | ||
| (c_library_flags (-L/lib64)) |
There was a problem hiding this comment.
This entire line should be dropped. All it's doing is adding a search path.
xenctrl_ext contains C bindings to xenctrl and Xen. This is a place to add C bindings that maybe are not yet evailable in Xen. Currently their visibility is limited to Xenopsd. This patch moves them into their own library - and hence makes them accessible from Xapi and other code in this repository. Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
lindig
force-pushed
the
private/christianlin/xenctrl
branch
from
b8f57f6 to
7796185
Compare
There was a problem hiding this comment.
LGTM.
Although XAPI itself should never link xenctrl, we got a unit test to ensure that: this is needed to support updating the version of Xen on a system: XAPI must always be able to start up, even if there is a mismatch between the installed xenctrl version and the running Xen.
In fact we should probably move the Xen version safety check to this xenctrlext library, otherwise a new daemon using it might crash or fail in various ways on such a mixed system (where the xenctrl ABI has changed between different releases of Xen)
Nevermind the compatibility check seems to be in XAPI: it disables the host if the installed xenctrl library and running Xen are different, there is nothing the daemons that use the xenctrl library need to do (although they may fail to open the xenctrl handle perhaps). |
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
6 participants
xenctrl_ext contains C bindings to xenctrl and Xen. This is a place to add C bindings that maybe are not yet evailable in Xen.
Currently their visibility is limited to Xenopsd. This patch moves them into their own library - and hence makes them accessible from Xapi and other code in this repository.