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

["Wieczorkiewicz, Pawel" <wipawel@amazon.de>]

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

On 19. Aug 2019, at 23:39, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com<mailto:marmarek@invisiblethingslab.com>> wrote:

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.


…snip...
+
+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 —.

ACK. Will do.

+    char *name;
+    unsigned int action;
+    uint32_t timeout;
+    uint64_t flags;
+    int rc;
+

…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;
+    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.


I’d prefer to add it as a separate commit (adding it to my TODO).

+        goto error;
+
+    fd = open(filename, O_RDONLY);
+    if ( fd < 0 )
+        goto error;
+
+    if ( stat(filename, &buf) != 0 )
+        goto error;
+

…snip...
+
+    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?


ACK, Will do.

No, unfortunately it’s me, who incorrectly sent out the series. I divided them
by functionality instead of per repo. I also did not create a cover letter. But,
I got more than one advice how to do it next time.

+    if ( rc )
+        goto error;
+

…snip...
+
+    list = PyList_New(0);

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


ACK. Will change.

+    name_off = metadata_off = 0;
+    for ( i = 0; i < done; i++ )
+    {
+        PyObject *info_dict, *metadata_list;
+        char *name_str, *metadata_str;
+
…snip...
(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?


Best Regards,
Pawel Wieczorkiewicz



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



[-- Attachment #1.2: Type: text/html, Size: 10869 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