Re: [Xen-devel] [PATCH] vm_event: Add a new opcode to get VM_EVENT_INTERFACE_VERSION

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.02.19 at 14:25, <ppircalabu@xxxxxxxxxxxxxxx> wrote:
> @@ -592,6 +592,19 @@ int vm_event_domctl(struct domain *d, struct 
> xen_domctl_vm_event_op *vec,
>  {
>      int rc;
>  
> +    if ( vec->op == XEN_VM_EVENT_GET_INTERFACE_VERSION )
> +    {
> +        vec->u.get_interface_version.value = VM_EVENT_INTERFACE_VERSION;
> +        return 0;
> +    }
> +
> +    if ( unlikely(d == NULL) )
> +    {
> +        gdprintk(XENLOG_INFO,
> +                 "Tried to do a memory event op on an invalid domain.\n");
> +        return -EINVAL;
> +    }

To be compatible with previous behavior you want to return
-ESRCH here. I'm also unconvinced of the need to add a log
message here - there was none before in that case. But I'm
not a maintainer of this code.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -778,9 +778,10 @@ struct xen_domctl_gdbsx_domstatus {
>   * to the hypervisor to pull responses (resume) from the given
>   * ring.
>   */
> -#define XEN_VM_EVENT_ENABLE               0
> -#define XEN_VM_EVENT_DISABLE              1
> -#define XEN_VM_EVENT_RESUME               2
> +#define XEN_VM_EVENT_ENABLE                 0
> +#define XEN_VM_EVENT_DISABLE                1
> +#define XEN_VM_EVENT_RESUME                 2
> +#define XEN_VM_EVENT_GET_INTERFACE_VERSION  3

Perhaps just XEN_VM_EVENT_GET_VERSION?

> @@ -843,7 +844,15 @@ struct xen_domctl_vm_event_op {
>      uint32_t       op;           /* XEN_VM_EVENT_* */
>      uint32_t       mode;         /* XEN_DOMCTL_VM_EVENT_OP_* */
>  
> -    uint32_t port;              /* OUT: event channel for ring */
> +    union {
> +        struct {
> +            uint32_t port;       /* OUT: event channel for ring */
> +        } enable;
> +
> +        struct {
> +            uint32_t value;
> +        } get_interface_version;

Why the wrapper structs? Having just a "port" and "version"
field inside the union would be good enough, wouldn't it? But
even if you want to stick to that, the new structure's name
could be simply "version", thus also allowing re-use for a
hypothetical "set-version" operation (in case multiple versions
would want/need to be supported going forward).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel