Re: [Xen-devel] [PATCH v3 MISSING/23] xsplice: Design document (v7).

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.02.16 at 22:57, <konrad.wilk@xxxxxxxxxx> wrote:
> +struct xsplice_patch_func {  
> +    const char *name;  
> +    Elf64_Xwordnew_addr;  

Missing space.

> +    Elf64_Xword old_addr;  
> +    Elf64_Word new_size;  
> +    Elf64_Word long old_size;  

There are still two types left here.

> +### XEN_SYSCTL_XSPLICE_GET (1)
> +
> +Retrieve an status of an specific payload. This caller provides:
> +
> + * A `struct xen_xsplice_name` called `name` which has the unique name.
> + * A `struct xen_xsplice_status` structure which has all members
> +   set to zero: That is:
> +   * `status` *MUST* be set to zero.
> +   * `rc` *MUST* be set to zero.

Why is this?

> +The structure is as follow:
> +
> +<pre>
> +struct xen_xsplice_status {  
> +#define XSPLICE_STATUS_LOADED       1  
> +#define XSPLICE_STATUS_CHECKED      2  
> +#define XSPLICE_STATUS_APPLIED      3  
> +    int32_t state;                  /* OUT: XSPLICE_STATE_*. IN: MUST be 
> zero. */  
> +    int32_t rc;                     /* OUT: 0 if no error, otherwise 
> -XEN_EXX. */  
> +                                    /* IN: MUST be zero. */
> +};  
> +
> +struct xen_sysctl_xsplice_summary {  
> +    xen_xsplice_name_t name;        /* IN, the name of the payload. */  
> +    xen_xsplice_status_t status;    /* IN/OUT: status of the payload. */  
> +};  

With the operation being named XEN_SYSCTL_XSPLICE_GET, shouldn't
the structure tag be xen_sysctl_xsplice_get?

> +### XEN_SYSCTL_XSPLICE_LIST (2)
> +
> +Retrieve an array of abbreviated status and names of payloads that are 
> loaded in the
> +hypervisor.
> +
> +The caller provides:
> +
> + * `version`. Initially (on first hypercall) *MUST* be zero.
> + * `idx` index iterator. On first call *MUST* be zero, subsequent calls 
> varies.
> + * `nr` the max number of entries to populate.
> + * `pad` - *MUST* be zero.
> + * `status` virtual address of where to write `struct xen_xsplice_status`
> +   structures. Caller *MUST* allocate up to `nr` of them.
> + * `name` - virtual address of where to write the unique name of the payload.
> +   Caller *MUST* allocate up to `nr` of them. Each *MUST* be of
> +   **XEN_XSPLICE_NAME_SIZE** size.
> + * `len` - virtual address of where to write the length of each unique name
> +   of the payload. Caller *MUST* allocate up to `nr` of them. Each *MUST* be
> +   of sizeof(uint32_t) (4 bytes).
> +
> +If the hypercall returns an positive number, it is the number (upto `nr`
> +provided to the hypercall) of the payloads returned, along with `nr` updated
> +with the number of remaining payloads, `version` updated (it may be the same
> +across hypercalls - if it varies the data is stale and further calls could
> +fail). The `status`, `name`, and `len`' are updated at their designed index
> +value (`idx`) with the returned value of data.
> +
> +If the hypercall returns -XEN_E2BIG the `nr` is too big and should be
> +lowered.
> +
> +If the hypercall returns an zero value that means there are no payloads.

Maybe worth changing to "... there are no (more) payloads",
considering the iterative nature of the operation?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel