Re: [Xen-devel] [PATCH livepatch-python 1/1] livepatch: Add python bindings for livepatch operations

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 12828 bytes --]

On Thu, Aug 15, 2019 at 11:36:46AM +0000, Pawel Wieczorkiewicz wrote:
> Extend the XC python bindings library to support also all common
> livepatch operations and actions.
> 
> Add the python bindings for the following operations:
> - status (pyxc_livepatch_status):
>   Requires a payload name as an input.
>   Returns a status dict containing a state string and a return code
>   integer.
> - action (pyxc_livepatch_action):
>   Requires a payload name and an action id as an input. Timeout and
>   flags are optional parameters.
>   Returns a return code integer.
> - upload (pyxc_livepatch_upload):
>   Requires a payload name and a module's filename as an input.
>   Returns a return code integer.
> - list (pyxc_livepatch_list):
>   Takes no parameters.
>   Returns a list of dicts containing each payload's:
>   * name as a string
>   * state as a string
>   * return code as an integer
>   * list of metadata key=value strings
> 
> Each functions throws an exception error based on the errno value
> received from its corresponding libxc function call.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Martin Mazein <amazein@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Leonard Foerster <foersleo@amazon.de>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
> ---
>  tools/python/xen/lowlevel/xc/xc.c | 273 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 273 insertions(+)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index 5459d6834d..87e3b8cacc 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -2008,6 +2008,230 @@ static PyObject *pyflask_access(PyObject *self, PyObject *args,
>      return Py_BuildValue("i",ret);
>  }
>  
> +static PyObject *pyxc_livepatch_status(XcObject *self,
> +                                       PyObject *args,
> +                                       PyObject *kwds)
> +{
> +    xen_livepatch_status_t status;
> +    PyObject *info_dict = NULL;
> +    char *name;
> +    int rc;
> +
> +    static char *kwd_list[] = { "name", NULL };
> +
> +    if ( !PyArg_ParseTupleAndKeywords(args, kwds, "s", kwd_list, &name) )
> +        goto error;
> +
> +    rc = xc_livepatch_get(self->xc_handle, name, &status);
> +    if ( rc )
> +        goto error;
> +
> +    info_dict = Py_BuildValue(
> +            "{s:i,s:i}",
> +            "state",    status.state,
> +            "rc",       status.rc);
> +
> +error:
> +    return info_dict ?: pyxc_error_to_exception(self->xc_handle);
> +}
> +
> +static PyObject *pyxc_livepatch_action(XcObject *self,
> +                                       PyObject *args,
> +                                       PyObject *kwds)
> +{
> +    int (*action_func)(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);

This makes it dependent on "livepatch: Allow to override inter-modules
buildid dependency" patch. Since it's part of a different series, if
there is going to be v2, please name the dependency explicitly in the
commit message or at least after ---.

> +    char *name;
> +    unsigned int action;
> +    uint32_t timeout;
> +    uint64_t flags;
> +    int rc;
> +
> +    static char *kwd_list[] = { "name", "action", "timeout", "flags", NULL };
> +
> +    if ( !PyArg_ParseTupleAndKeywords(args, kwds, "sI|Ik", kwd_list,
> +                                      &name, &action, &timeout, &flags) )
> +        goto error;
> +
> +    switch (action)
> +    {
> +    case LIVEPATCH_ACTION_UNLOAD:
> +        action_func = xc_livepatch_unload;
> +        break;
> +    case LIVEPATCH_ACTION_REVERT:
> +        action_func = xc_livepatch_revert;
> +        break;
> +    case LIVEPATCH_ACTION_APPLY:
> +        action_func = xc_livepatch_apply;
> +        break;
> +    case LIVEPATCH_ACTION_REPLACE:
> +        action_func = xc_livepatch_replace;
> +        break;
> +    default:
> +        goto error;
> +    }
> +
> +    rc = action_func(self->xc_handle, name, timeout, flags);
> +    if ( rc )
> +        goto error;
> +
> +    return Py_BuildValue("i", rc);
> +error:
> +    return pyxc_error_to_exception(self->xc_handle);
> +}
> +
> +static PyObject *pyxc_livepatch_upload(XcObject *self,
> +                                       PyObject *args,
> +                                       PyObject *kwds)
> +{
> +    unsigned char *fbuf = MAP_FAILED;
> +    char *name, *filename;
> +    struct stat buf;
> +    int fd = 0, rc;
> +    ssize_t len;
> +
> +    static char *kwd_list[] = { "name", "filename", NULL };
> +
> +    if ( !PyArg_ParseTupleAndKeywords(args, kwds, "ss", kwd_list,
> +                                      &name, &filename))

It would be nice to support Path-like objects on python >= 3.6. See
note about "s" format here:
https://docs.python.org/3/c-api/arg.html#strings-and-buffers

But since it's python 3 only, that would need to be under #if
PY_MAJOR_VERSION >= 3.

> +        goto error;
> +
> +    fd = open(filename, O_RDONLY);
> +    if ( fd < 0 )
> +        goto error;
> +
> +    if ( stat(filename, &buf) != 0 )
> +        goto error;
> +
> +    len = buf.st_size;
> +    fbuf = mmap(0, len, PROT_READ, MAP_PRIVATE, fd, 0);
> +    if ( fbuf == MAP_FAILED )
> +        goto error;
> +
> +    rc = xc_livepatch_upload(self->xc_handle, name, fbuf, len);
> +    if ( rc )
> +        goto error;
> +
> +    if ( munmap(fbuf, len) )
> +    {
> +        fbuf = MAP_FAILED;
> +        goto error;
> +    }
> +    close(fd);
> +
> +    return Py_BuildValue("i", rc);;
> +error:
> +    if ( fbuf != MAP_FAILED )
> +        munmap(fbuf, len);
> +    if ( fd >= 0 )
> +        close(fd);
> +    return pyxc_error_to_exception(self->xc_handle);
> +}
> +
> +static PyObject *pyxc_livepatch_list(XcObject *self)
> +{
> +    PyObject *list;
> +    unsigned int nr, done, left, i;
> +    xen_livepatch_status_t *info = NULL;
> +    char *name = NULL;
> +    char *metadata = NULL;
> +    uint32_t *len = NULL;
> +    uint32_t *metadata_len = NULL;
> +    uint64_t name_total_size, metadata_total_size;
> +    off_t name_off, metadata_off;
> +    int rc;
> +
> +    rc = xc_livepatch_list_get_sizes(self->xc_handle, &nr,
> +                                     &name_total_size, &metadata_total_size);

This makes it dependent on lp-metadata series. Same note as previously.
BTW for some reason your patch series are not handled as a mail threads,
which makes it harder to look for other patches in the series. Is it
only me?

> +    if ( rc )
> +        goto error;
> +
> +    if ( nr == 0 )
> +        return PyList_New(0);
> +
> +    rc = ENOMEM;
> +    info = malloc(nr * sizeof(*info));
> +    if ( !info )
> +        goto error;
> +
> +    name = malloc(name_total_size * sizeof(*name));
> +    if ( !name )
> +        goto error;
> +
> +    len = malloc(nr * sizeof(*len));
> +    if ( !len )
> +        goto error;
> +
> +    metadata = malloc(metadata_total_size * sizeof(*metadata));
> +    if ( !metadata )
> +        goto error;
> +
> +    metadata_len = malloc(nr * sizeof(*metadata_len));
> +    if ( !metadata_len )
> +        goto error;
> +
> +    rc = xc_livepatch_list(self->xc_handle, nr, 0, info,
> +                           name, len, name_total_size,
> +                           metadata, metadata_len, metadata_total_size,
> +                           &done, &left);
> +    if ( rc )
> +        goto error;
> +
> +    list = PyList_New(0);

Better use PyList_New(done) and later PyList_SetItem() instead of PyList_Append().

> +    name_off = metadata_off = 0;
> +    for ( i = 0; i < done; i++ )
> +    {
> +        PyObject *info_dict, *metadata_list;
> +        char *name_str, *metadata_str;
> +
> +        name_str = name + name_off;
> +        metadata_str = metadata + metadata_off;
> +
> +        metadata_list = PyList_New(0);
> +        for ( char *s = metadata_str; s < metadata_str + metadata_len[i]; s += strlen(s) + 1 )
> +        {
> +            PyObject *field = Py_BuildValue("s", s);
> +            if ( field == NULL )
> +            {
> +                Py_DECREF(list);
> +                Py_DECREF(metadata_list);
> +                rc = EFAULT;
> +                goto error;
> +            }
> +
> +            PyList_Append(metadata_list, field);
> +            Py_DECREF(field);
> +        }
> +
> +        info_dict = Py_BuildValue(
> +            "{s:s,s:i,s:i,s:N}",
> +            "name",     name_str,
> +            "state",    info[i].state,
> +            "rc",       info[i].rc,
> +            "metadata", metadata_list);
> +
> +        if ( info_dict == NULL )
> +        {
> +            Py_DECREF(list);
> +            Py_DECREF(metadata_list);
> +            rc = EFAULT;
> +            goto error;
> +        }
> +        PyList_Append(list, info_dict);
> +        Py_DECREF(info_dict);
> +
> +        name_off += len[i];
> +        metadata_off += metadata_len[i];
> +    }
> +
> +error:
> +    free(info);
> +    free(name);
> +    free(len);
> +    free(metadata);
> +    free(metadata_len);
> +    return rc ? pyxc_error_to_exception(self->xc_handle) : list;
> +}
> +
>  static PyMethodDef pyxc_methods[] = {
>      { "domain_create", 
>        (PyCFunction)pyxc_domain_create, 
> @@ -2584,6 +2808,44 @@ static PyMethodDef pyxc_methods[] = {
>        "Returns: [int]: 0 on all permission granted; -1 if any permissions are \
>         denied\n" }, 
>  
> +    { "livepatch_status",
> +      (PyCFunction)pyxc_livepatch_status,
> +      METH_KEYWORDS, "\n"
> +      "Gets current state and return code for a specified module.\n"
> +      " name     [str]: Module name to be used\n"
> +      "Returns: [dict] on success; throwing an exception on error\n"
> +      " state    [int]: Module current state: CHECKED or APPLIED\n"
> +      " rc       [int]: Return code of last module's operation\n" },
> +
> +    { "livepatch_upload",
> +      (PyCFunction)pyxc_livepatch_upload,
> +      METH_KEYWORDS, "\n"
> +      "Uploads a module with specified name from filename.\n"
> +      " name     [str]: Module name to be used\n"
> +      " filename [str]: Filename of a module to be uploaded\n"
> +      "Returns: [int] 0 on success; throwing an exception on error\n" },
> +
> +    { "livepatch_action",
> +      (PyCFunction)pyxc_livepatch_action,
> +      METH_KEYWORDS, "\n"
> +      "Performs an action (unload, revert, apply or replace) on a specified \
> +       module.\n"
> +      " name      [str]: Module name to be used\n"
> +      " action   [uint]: Action enum id\n"
> +      " timeout  [uint]: Action scheduled execution timeout\n"
> +      " flags   [ulong]: Flags specifying action's extra parameters\n"
> +      "Returns: [int] 0 on success; throwing an exception on error\n" },
> +
> +    { "livepatch_list",
> +      (PyCFunction)pyxc_livepatch_list,
> +      METH_NOARGS, "\n"
> +      "List all uploaded livepatch modules with their current state and metadata.\n"
> +      "Returns: [list of dicts] on success; throwing an exception on error\n"
> +      " name     [str]: Module name\n"
> +      " state    [int]: Module current state: CHECKED or APPLIED\n"
> +      " rc       [int]: Return code of last module's operation\n"
> +      " metadata [list]: List of module's metadata 'key=value' strings\n" },
> +
>      { NULL, NULL, 0, NULL }
>  };
>  
> @@ -2695,6 +2957,17 @@ PyMODINIT_FUNC initxc(void)
>      PyModule_AddIntConstant(m, "XEN_SCHEDULER_CREDIT", XEN_SCHEDULER_CREDIT);
>      PyModule_AddIntConstant(m, "XEN_SCHEDULER_CREDIT2", XEN_SCHEDULER_CREDIT2);
>  
> +    /* Expose livepatch constants to Python */
> +    PyModule_AddIntConstant(m, "LIVEPATCH_ACTION_UNLOAD", LIVEPATCH_ACTION_UNLOAD);
> +    PyModule_AddIntConstant(m, "LIVEPATCH_ACTION_REVERT", LIVEPATCH_ACTION_REVERT);
> +    PyModule_AddIntConstant(m, "LIVEPATCH_ACTION_APPLY", LIVEPATCH_ACTION_APPLY);
> +    PyModule_AddIntConstant(m, "LIVEPATCH_ACTION_REPLACE", LIVEPATCH_ACTION_REPLACE);
> +
> +    PyModule_AddIntConstant(m, "LIVEPATCH_ACTION_APPLY_NODEPS", LIVEPATCH_ACTION_APPLY_NODEPS);
> +
> +    PyModule_AddIntConstant(m, "LIVEPATCH_STATE_APPLIED", LIVEPATCH_STATE_APPLIED);
> +    PyModule_AddIntConstant(m, "LIVEPATCH_STATE_CHECKED", LIVEPATCH_STATE_CHECKED);
> +
>  #if PY_MAJOR_VERSION >= 3
>      return m;
>  #endif

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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