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

2019-11-19 Thread Ross Lagerwall
On 11/14/19 1:06 PM, 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 None or throws an exception.
> - upload (pyxc_livepatch_upload):
>   Requires a payload name and a module's filename as an input.
>   Returns None or throws an exception.
> - 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 
> Reviewed-by: Martin Mazein 
> Reviewed-by: Andra-Irina Paraschiv 
> Reviewed-by: Leonard Foerster 
> Reviewed-by: Norbert Manthey 
> Acked-by: Marek Marczykowski-Górecki 
> ---
snip
> +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 = -1, saved_errno;

Does fd actually need to be initialized here?

Also, initializing it to 0 seems odd because 0 is a valid fd.

> +ssize_t len;
> +
> +static char *kwd_list[] = { "name", "filename", NULL };
> +
> +if ( !PyArg_ParseTupleAndKeywords(args, kwds, "ss", kwd_list,
> +  , ))
> +goto error;
> +
> +fd = open(filename, O_RDONLY);
> +if ( fd < 0 )
> +goto error;
> +
> +if ( fstat(fd, ) != 0 )
> +goto error_fd;
> +
> +len = buf.st_size;
> +fbuf = mmap(0, len, PROT_READ, MAP_PRIVATE, fd, 0);
> +if ( fbuf == MAP_FAILED )
> +goto error_fd;
> +
> +rc = xc_livepatch_upload(self->xc_handle, name, fbuf, len);
> +
> +saved_errno = errno;
> +munmap(fbuf, len);
> +errno = saved_errno;
> +
> +error_fd:
> +close(fd);
> +error:
> +return rc ? pyxc_error_to_exception(self->xc_handle) : Py_None;
> +}
snip>  static PyMethodDef pyxc_methods[] = {
>  { "domain_create", 
>(PyCFunction)pyxc_domain_create, 
> @@ -2542,6 +2761,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: None 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"

Should this be uint and not ulong?

I expect these things could be fixed up on commit.

Reviewed-by: Ross Lagerwall 

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

[Xen-devel] [PATCH v5 12/12] livepatch: Add python bindings for livepatch operations

2019-11-14 Thread Pawel Wieczorkiewicz
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 None or throws an exception.
- upload (pyxc_livepatch_upload):
  Requires a payload name and a module's filename as an input.
  Returns None or throws an exception.
- 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 
Reviewed-by: Martin Mazein 
Reviewed-by: Andra-Irina Paraschiv 
Reviewed-by: Leonard Foerster 
Reviewed-by: Norbert Manthey 
Acked-by: Marek Marczykowski-Górecki 
---
Changed since v4:
  * changed flags field type from uint64_t to uint32_t
  * fixed leaking fd in pyxc_livepatch_upload()

Changed since v3:
  * return None instead of integer 0 from pyxc_livepatch_action()
and pyxc_livepatch_upload()
  * use fstat() instead of stat()
  * simplify error condition handling code for pyxc_livepatch_upload
and also save and restore errno value
  * check done and left values to handle errors in
pyxc_livepatch_list()
  * use PyList_SET_ITEM() to avoid the need for PyDECREF

Changed since v1:
  * changed PyList_Append() with PyList_SetItem() as requested by
Marek
---
 tools/python/xen/lowlevel/xc/xc.c | 268 ++
 1 file changed, 268 insertions(+)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index 44d3606141..9a1d3b6b92 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1979,6 +1979,225 @@ 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, ) )
+goto error;
+
+rc = xc_livepatch_get(self->xc_handle, name, );
+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, 
uint32_t flags);
+char *name;
+unsigned int action;
+uint32_t timeout;
+uint32_t flags;
+int rc = -1;
+
+static char *kwd_list[] = { "name", "action", "timeout", "flags", NULL };
+
+if ( !PyArg_ParseTupleAndKeywords(args, kwds, "sI|Ik", kwd_list,
+  , , , ) )
+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);
+
+error:
+return rc ? pyxc_error_to_exception(self->xc_handle) : Py_None;
+}
+
+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 = -1, saved_errno;
+ssize_t len;
+
+static char *kwd_list[] = { "name", "filename", NULL };
+
+if ( !PyArg_ParseTupleAndKeywords(args, kwds, "ss", kwd_list,
+  , ))
+goto error;
+
+fd = open(filename, O_RDONLY);
+if ( fd < 0 )
+goto error;
+
+if ( fstat(fd, ) != 0 )
+goto error_fd;
+
+len = buf.st_size;
+fbuf = mmap(0, len, PROT_READ, MAP_PRIVATE, fd, 0);
+if ( fbuf == MAP_FAILED )
+goto error_fd;
+
+rc = xc_livepatch_upload(self->xc_handle, name, fbuf, len);
+
+