Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
On Tue, Feb 12, 2019 at 11:20 AM Razvan Cojocaru wrote: > > On 2/12/19 8:13 PM, Tamas K Lengyel wrote: > > On Tue, Jan 8, 2019 at 9:39 AM Razvan Cojocaru > > wrote: > >> > >> On 1/8/19 6:27 PM, Jan Beulich wrote: > >> On 19.12.18 at 19:52, wrote: > Signed-off-by: Petre Pircalabu > >>> > >>> An empty description is not helpful. The immediate question is: Why? > >>> We don't do this for other interface versions. I'm unconvinced a > >>> special purpose piece of information like this one belongs into the > >>> rather generic version hypercall. > >> > >> For an introspection application meant to be deployed on several Xen > >> versions without recompiling, it is important to be able to decide at > >> runtime what size and layout the vm_event struct has. > >> > >> Currently this can somewhat be done by associating the current version > >> with the vm_event version, but that is not ideal for obvious reasons. > > > > We do exactly that in LibVMI and it works fine - care to elaborate > > what problem you have with doing that? There is a 1:1 match between > > any stable Xen version and a vm_event interface version. I don't think > > we will ever have a situation where we bump the vm_event interface > > version but not the Xen release version. > > XenServer. Any given version of XenServer may or may not fit the matrix > you're talking about (because some patches are backported, some are not, > etc.). In which case it all becomes very complicated. Ah yes, if custom patches are applied on top I can see how that could become a problem. Thanks, Tamas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
On 2/12/19 8:13 PM, Tamas K Lengyel wrote: > On Tue, Jan 8, 2019 at 9:39 AM Razvan Cojocaru > wrote: >> >> On 1/8/19 6:27 PM, Jan Beulich wrote: >> On 19.12.18 at 19:52, wrote: Signed-off-by: Petre Pircalabu >>> >>> An empty description is not helpful. The immediate question is: Why? >>> We don't do this for other interface versions. I'm unconvinced a >>> special purpose piece of information like this one belongs into the >>> rather generic version hypercall. >> >> For an introspection application meant to be deployed on several Xen >> versions without recompiling, it is important to be able to decide at >> runtime what size and layout the vm_event struct has. >> >> Currently this can somewhat be done by associating the current version >> with the vm_event version, but that is not ideal for obvious reasons. > > We do exactly that in LibVMI and it works fine - care to elaborate > what problem you have with doing that? There is a 1:1 match between > any stable Xen version and a vm_event interface version. I don't think > we will ever have a situation where we bump the vm_event interface > version but not the Xen release version. XenServer. Any given version of XenServer may or may not fit the matrix you're talking about (because some patches are backported, some are not, etc.). In which case it all becomes very complicated. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
On Tue, Jan 8, 2019 at 9:39 AM Razvan Cojocaru wrote: > > On 1/8/19 6:27 PM, Jan Beulich wrote: > On 19.12.18 at 19:52, wrote: > >> Signed-off-by: Petre Pircalabu > > > > An empty description is not helpful. The immediate question is: Why? > > We don't do this for other interface versions. I'm unconvinced a > > special purpose piece of information like this one belongs into the > > rather generic version hypercall. > > For an introspection application meant to be deployed on several Xen > versions without recompiling, it is important to be able to decide at > runtime what size and layout the vm_event struct has. > > Currently this can somewhat be done by associating the current version > with the vm_event version, but that is not ideal for obvious reasons. We do exactly that in LibVMI and it works fine - care to elaborate what problem you have with doing that? There is a 1:1 match between any stable Xen version and a vm_event interface version. I don't think we will ever have a situation where we bump the vm_event interface version but not the Xen release version. Tamas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
>>> On 12.02.19 at 17:57, wrote: > On Wed, 2019-01-09 at 11:11 +0200, Razvan Cojocaru wrote: >> >> On 1/8/19 6:47 PM, Jan Beulich wrote: >> > > > > On 08.01.19 at 17:37, wrote: >> > > >> > > On 1/8/19 6:27 PM, Jan Beulich wrote: >> > > > > > > On 19.12.18 at 19:52, wrote: >> > > > > >> > > > > Signed-off-by: Petre Pircalabu >> > > > >> > > > An empty description is not helpful. The immediate question is: >> > > > Why? >> > > > We don't do this for other interface versions. I'm unconvinced >> > > > a >> > > > special purpose piece of information like this one belongs into >> > > > the >> > > > rather generic version hypercall. >> > > >> > > For an introspection application meant to be deployed on several >> > > Xen >> > > versions without recompiling, it is important to be able to >> > > decide at >> > > runtime what size and layout the vm_event struct has. >> > > >> > > Currently this can somewhat be done by associating the current >> > > version >> > > with the vm_event version, but that is not ideal for obvious >> > > reasons. >> > > Reading the vm_event version from an actual vm_event is also out >> > > of the >> > > question, because in order to be able to receive the first >> > > vm_event we >> > > have to set the ring buffer up, and that requires knowledge of >> > > the size >> > > of the vm_event. So a run-time mechanism for querying the >> > > vm_event >> > > version is needed. >> > > >> > > We just thought that this was the most flexible place to add it. >> > >> > How about a new XEN_DOMCTL_VM_EVENT_GET_VERSION? >> >> That would work as well, we just thought this was the least >> intrusive >> and most extensible way to do it (other queries could be added >> similarly >> in the future, without needing a new DOMCTL / libxc toolstack >> modifications). >> > Personally, would prefer the xc_version approach because it has a > number of advantages over a creating specific domctl: > > - First, it's a version. In my opinion, if an interface too strongly > coupled with XEN that it cannot be disabled at configure-time, it's > generic enough to be queried by the common version functions. An > example of getting specialized information from XEN is > XENVER_get_features, which is also handled using xc_version. Whether XENVER_get_features is a good fit there is questionable, but it's been there for too long to be (re)moved. > - This interface version is hypervisor specific. A client application > should be able to query this version at startup, even before the > monitor domain is available, and a domctl requires a domain id. The > DOM0 id or DOMID_INVALID can be used, but I find it rather confusing to > query something hypervisor specific and pass a domain related param. Well, I did suggest a domctl because there already is XEN_DOMCTL_vm_event_op, and it could be a sub-op there. Of course you could also add a sysctl just for this, or a whole new major hypercall ... Note how XEN_DOMCTL_createdomain (obviously) also can't possibly be handed a domain ID. > - It's simple and it can be easily extended. As are any other (simple) hypercalls. As you can see I continue to be opposed to add special purpose subsystem information to a general hypercall (of whatever sort). However, as so often I'm not opposed enough to actively nack such an addition, so if other REST maintainers think this is an appropriate thing to do, so be it. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
On Wed, 2019-01-09 at 11:11 +0200, Razvan Cojocaru wrote: > > On 1/8/19 6:47 PM, Jan Beulich wrote: > > > > > On 08.01.19 at 17:37, wrote: > > > > > > On 1/8/19 6:27 PM, Jan Beulich wrote: > > > > > > > On 19.12.18 at 19:52, wrote: > > > > > > > > > > Signed-off-by: Petre Pircalabu > > > > > > > > An empty description is not helpful. The immediate question is: > > > > Why? > > > > We don't do this for other interface versions. I'm unconvinced > > > > a > > > > special purpose piece of information like this one belongs into > > > > the > > > > rather generic version hypercall. > > > > > > For an introspection application meant to be deployed on several > > > Xen > > > versions without recompiling, it is important to be able to > > > decide at > > > runtime what size and layout the vm_event struct has. > > > > > > Currently this can somewhat be done by associating the current > > > version > > > with the vm_event version, but that is not ideal for obvious > > > reasons. > > > Reading the vm_event version from an actual vm_event is also out > > > of the > > > question, because in order to be able to receive the first > > > vm_event we > > > have to set the ring buffer up, and that requires knowledge of > > > the size > > > of the vm_event. So a run-time mechanism for querying the > > > vm_event > > > version is needed. > > > > > > We just thought that this was the most flexible place to add it. > > > > How about a new XEN_DOMCTL_VM_EVENT_GET_VERSION? > > That would work as well, we just thought this was the least > intrusive > and most extensible way to do it (other queries could be added > similarly > in the future, without needing a new DOMCTL / libxc toolstack > modifications). > Personally, would prefer the xc_version approach because it has a number of advantages over a creating specific domctl: - First, it's a version. In my opinion, if an interface too strongly coupled with XEN that it cannot be disabled at configure-time, it's generic enough to be queried by the common version functions. An example of getting specialized information from XEN is XENVER_get_features, which is also handled using xc_version. - This interface version is hypervisor specific. A client application should be able to query this version at startup, even before the monitor domain is available, and a domctl requires a domain id. The DOM0 id or DOMID_INVALID can be used, but I find it rather confusing to query something hypervisor specific and pass a domain related param. - It's simple and it can be easily extended. Many thanks, Petre ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
On 1/8/19 6:47 PM, Jan Beulich wrote: On 08.01.19 at 17:37, wrote: On 1/8/19 6:27 PM, Jan Beulich wrote: On 19.12.18 at 19:52, wrote: Signed-off-by: Petre Pircalabu An empty description is not helpful. The immediate question is: Why? We don't do this for other interface versions. I'm unconvinced a special purpose piece of information like this one belongs into the rather generic version hypercall. For an introspection application meant to be deployed on several Xen versions without recompiling, it is important to be able to decide at runtime what size and layout the vm_event struct has. Currently this can somewhat be done by associating the current version with the vm_event version, but that is not ideal for obvious reasons. Reading the vm_event version from an actual vm_event is also out of the question, because in order to be able to receive the first vm_event we have to set the ring buffer up, and that requires knowledge of the size of the vm_event. So a run-time mechanism for querying the vm_event version is needed. We just thought that this was the most flexible place to add it. How about a new XEN_DOMCTL_VM_EVENT_GET_VERSION? That would work as well, we just thought this was the least intrusive and most extensible way to do it (other queries could be added similarly in the future, without needing a new DOMCTL / libxc toolstack modifications). Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
>>> On 08.01.19 at 17:37, wrote: > On 1/8/19 6:27 PM, Jan Beulich wrote: > On 19.12.18 at 19:52, wrote: >>> Signed-off-by: Petre Pircalabu >> >> An empty description is not helpful. The immediate question is: Why? >> We don't do this for other interface versions. I'm unconvinced a >> special purpose piece of information like this one belongs into the >> rather generic version hypercall. > > For an introspection application meant to be deployed on several Xen > versions without recompiling, it is important to be able to decide at > runtime what size and layout the vm_event struct has. > > Currently this can somewhat be done by associating the current version > with the vm_event version, but that is not ideal for obvious reasons. > Reading the vm_event version from an actual vm_event is also out of the > question, because in order to be able to receive the first vm_event we > have to set the ring buffer up, and that requires knowledge of the size > of the vm_event. So a run-time mechanism for querying the vm_event > version is needed. > > We just thought that this was the most flexible place to add it. How about a new XEN_DOMCTL_VM_EVENT_GET_VERSION? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
On 1/8/19 6:27 PM, Jan Beulich wrote: On 19.12.18 at 19:52, wrote: Signed-off-by: Petre Pircalabu An empty description is not helpful. The immediate question is: Why? We don't do this for other interface versions. I'm unconvinced a special purpose piece of information like this one belongs into the rather generic version hypercall. For an introspection application meant to be deployed on several Xen versions without recompiling, it is important to be able to decide at runtime what size and layout the vm_event struct has. Currently this can somewhat be done by associating the current version with the vm_event version, but that is not ideal for obvious reasons. Reading the vm_event version from an actual vm_event is also out of the question, because in order to be able to receive the first vm_event we have to set the ring buffer up, and that requires knowledge of the size of the vm_event. So a run-time mechanism for querying the vm_event version is needed. We just thought that this was the most flexible place to add it. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
>>> On 19.12.18 at 19:52, wrote: > Signed-off-by: Petre Pircalabu An empty description is not helpful. The immediate question is: Why? We don't do this for other interface versions. I'm unconvinced a special purpose piece of information like this one belongs into the rather generic version hypercall. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version
Signed-off-by: Petre Pircalabu --- tools/libxc/xc_private.c | 3 +++ xen/common/kernel.c | 3 +++ xen/include/public/version.h | 3 +++ 3 files changed, 9 insertions(+) diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index 90974d5..9b983e0 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -497,6 +497,9 @@ int xc_version(xc_interface *xch, int cmd, void *arg) HYPERCALL_BOUNCE_SET_DIR(arg, XC_HYPERCALL_BUFFER_BOUNCE_BOTH); break; } +case XENVER_vm_event_version: +sz = 0; +break; default: ERROR("xc_version: unknown command %d\n", cmd); return -EINVAL; diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 5766a0f..667552c 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -516,6 +516,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return sz; } + +case XENVER_vm_event_version: +return VM_EVENT_INTERFACE_VERSION; } return -ENOSYS; diff --git a/xen/include/public/version.h b/xen/include/public/version.h index 7063e8c..b962386 100644 --- a/xen/include/public/version.h +++ b/xen/include/public/version.h @@ -103,6 +103,9 @@ struct xen_build_id { }; typedef struct xen_build_id xen_build_id_t; +/* arg == NULL; returns the vm_event interface version */ +#define XENVER_vm_event_version 11 + #endif /* __XEN_PUBLIC_VERSION_H__ */ /* -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel