[PATCH v1 0/7] xen-livepatch misc fixes/changes

[PATCH v1 0/7] xen-livepatch misc fixes/changes

[Ross Lagerwall]

Hi all,

This series contains a few fixes to the xen-livepatch tool.
It also contains a few changes to make the output more readable.

Ross Lagerwall (7):
  tools/livepatch: Show the correct expected state before action
  tools/livepatch: Set stdout and stderr unbuffered
  tools/livepatch: Improve output
  livepatch: Set timeout unit to nanoseconds
  tools/livepatch: Remove pointless retry loop
  tools/livepatch: Remove unused struct member
  tools/livepatch: Exit with 2 if a timeout occurs

 tools/libxc/include/xenctrl.h |   2 +-
 tools/misc/xen-livepatch.c    | 214 ++++++++++++++++++++++++++----------------
 xen/common/livepatch.c        |   4 +-
 xen/include/public/sysctl.h   |   2 +-
 4 files changed, 138 insertions(+), 84 deletions(-)

-- 
2.7.4


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

[PATCH v1 1/7] tools/livepatch: Show the correct expected state before action

[Ross Lagerwall]

Somewhat confusingly, before the action has been executed the patch is
expected to be in the "allowed" state, not the "expected" state.  The
check for this was correct but the subsequent error message was not.
Fix the error message to show this state correctly.

Before:
    $ xen-livepatch unload test
    test: in wrong state (APPLIED), expected (unknown)
After:
    $ xen-livepatch unload test
    test: in wrong state (APPLIED), expected (CHECKED)

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 tools/misc/xen-livepatch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 2de04c0..f6c7c8a 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -308,7 +308,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
     {
         printf("%s: in wrong state (%s), expected (%s)\n",
                name, state2str(status.state),
-               state2str(action_options[idx].expected));
+               state2str(action_options[idx].allow));
         return -1;
     }
 
-- 
2.7.4


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

[PATCH v1 2/7] tools/livepatch: Set stdout and stderr unbuffered

[Ross Lagerwall]

Using both stdout and stderr interleaved without newlines can result in
strange output when using line buffered mode (e.g. a terminal) or when
fully buffered (e.g. redirected to a file). Set both to unbuffered mode
to fix this.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 tools/misc/xen-livepatch.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index f6c7c8a..2f6721b 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -330,7 +330,6 @@ int action_func(int argc, char *argv[], unsigned int idx)
         }
 
         printf(".");
-        fflush(stdout);
         usleep(DELAY);
     } while ( ++retry < RETRIES );
 
@@ -416,6 +415,13 @@ int main(int argc, char *argv[])
 {
     int i, j = 0, ret;
 
+    /*
+     * Set stdout and stderr to be unbuffered to avoid having to fflush
+     * when printing without a newline.
+     */
+    setvbuf(stdout, NULL, _IONBF, 0);
+    setvbuf(stderr, NULL, _IONBF, 0);
+
     if ( argc  <= 1 )
     {
         show_help();
-- 
2.7.4


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

[PATCH v1 3/7] tools/livepatch: Improve output

[Ross Lagerwall]

Improving the output of xen-livepatch, which is currently hard to read,
especially when an error occurs.

Some examples of the changes:
Before:
    $ xen-livepatch apply test
    Performing apply:. completed
After:
    $ xen-livepatch apply test
    Applying test:. completed

Before:
    $ xen-livepatch apply test2
    test2 failed with 22(Invalid argument)
    Performing apply: (no newline)
After:
    $ xen-livepatch apply test2
    Applying test2: failed
    Error 22: Invalid argument

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 tools/misc/xen-livepatch.c | 75 +++++++++++++++++++++++++++++++---------------
 1 file changed, 51 insertions(+), 24 deletions(-)

diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 2f6721b..afd8c48 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -100,7 +100,8 @@ static int list_func(int argc, char *argv[])
         rc = xc_livepatch_list(xch, MAX_LEN, idx, info, name, len, &done, &left);
         if ( rc )
         {
-            fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
+            fprintf(stderr, "Failed to list %d/%d.\n"
+                            "Error %d: %s\n",
                     idx, left, errno, strerror(errno));
             break;
         }
@@ -140,7 +141,8 @@ static int get_name(int argc, char *argv[], char *name)
     ssize_t len = strlen(argv[0]);
     if ( len > XEN_LIVEPATCH_NAME_SIZE )
     {
-        fprintf(stderr, "ID MUST be %d characters!\n", XEN_LIVEPATCH_NAME_SIZE);
+        fprintf(stderr, "ID must be no more than %d characters.\n",
+                XEN_LIVEPATCH_NAME_SIZE);
         errno = EINVAL;
         return errno;
     }
@@ -172,13 +174,15 @@ static int upload_func(int argc, char *argv[])
     fd = open(filename, O_RDONLY);
     if ( fd < 0 )
     {
-        fprintf(stderr, "Could not open %s, error: %d(%s)\n",
+        fprintf(stderr, "Could not open %s.\n"
+                        "Error %d: %s\n",
                 filename, errno, strerror(errno));
         return errno;
     }
     if ( stat(filename, &buf) != 0 )
     {
-        fprintf(stderr, "Could not get right size %s, error: %d(%s)\n",
+        fprintf(stderr, "Could not get size of %s.\n"
+                        "Error %d: %s\n",
                 filename, errno, strerror(errno));
         close(fd);
         return errno;
@@ -188,21 +192,29 @@ static int upload_func(int argc, char *argv[])
     fbuf = mmap(0, len, PROT_READ, MAP_PRIVATE, fd, 0);
     if ( fbuf == MAP_FAILED )
     {
-        fprintf(stderr,"Could not map: %s, error: %d(%s)\n",
+        fprintf(stderr, "Could not map %s.\n"
+                        "Error %d: %s\n",
                 filename, errno, strerror(errno));
         close (fd);
         return errno;
     }
-    printf("Uploading %s (%zu bytes)\n", filename, len);
+    printf("Uploading %s... ", filename);
     rc = xc_livepatch_upload(xch, name, fbuf, len);
     if ( rc )
-        fprintf(stderr, "Upload failed: %s, error: %d(%s)!\n",
-                filename, errno, strerror(errno));
+    {
+        printf("failed\n");
+        fprintf(stderr, "Error %d: %s\n",
+                errno, strerror(errno));
+    }
+    else
+        printf("completed\n");
+
 
     if ( munmap( fbuf, len) )
     {
-        fprintf(stderr, "Could not unmap!? error: %d(%s)!\n",
-                errno, strerror(errno));
+        fprintf(stderr, "Could not unmap %s.\n"
+                        "Error %d: %s\n",
+                filename, errno, strerror(errno));
         if ( !rc )
             rc = errno;
     }
@@ -223,27 +235,32 @@ struct {
     int allow; /* State it must be in to call function. */
     int expected; /* The state to be in after the function. */
     const char *name;
+    const char *verb;
     int (*function)(xc_interface *xch, char *name, uint32_t timeout);
     unsigned int executed; /* Has the function been called?. */
 } action_options[] = {
     {   .allow = LIVEPATCH_STATE_CHECKED,
         .expected = LIVEPATCH_STATE_APPLIED,
         .name = "apply",
+        .verb = "Applying",
         .function = xc_livepatch_apply,
     },
     {   .allow = LIVEPATCH_STATE_APPLIED,
         .expected = LIVEPATCH_STATE_CHECKED,
         .name = "revert",
+        .verb = "Reverting",
         .function = xc_livepatch_revert,
     },
     {   .allow = LIVEPATCH_STATE_CHECKED,
         .expected = -XEN_ENOENT,
         .name = "unload",
+        .verb = "Unloading",
         .function = xc_livepatch_unload,
     },
     {   .allow = LIVEPATCH_STATE_CHECKED,
         .expected = LIVEPATCH_STATE_APPLIED,
         .name = "replace",
+        .verb = "Replacing all live patches with",
         .function = xc_livepatch_replace,
     },
 };
@@ -276,39 +293,44 @@ int action_func(int argc, char *argv[], unsigned int idx)
     rc = xc_livepatch_get(xch, name, &status);
     if ( rc )
     {
-        fprintf(stderr, "%s failed to get status %d(%s)!\n",
+        fprintf(stderr, "Failed to get status of %s.\n"
+                        "Error %d: %s\n",
                 name, errno, strerror(errno));
         return -1;
     }
     if ( status.rc == -XEN_EAGAIN )
     {
-        fprintf(stderr, "%s failed. Operation already in progress\n", name);
+        fprintf(stderr,
+                "Cannot execute %s.\n"
+                "Operation already in progress.\n", action_options[idx].name);
         return -1;
     }
 
     if ( status.state == action_options[idx].expected )
     {
-        printf("No action needed\n");
+        printf("No action needed.\n");
         return 0;
     }
 
     /* Perform action. */
     if ( action_options[idx].allow & status.state )
     {
-        printf("Performing %s:", action_options[idx].name);
+        printf("%s %s:", action_options[idx].verb, name);
         rc = action_options[idx].function(xch, name, 0);
         if ( rc )
         {
-            fprintf(stderr, "%s failed with %d(%s)\n", name, errno,
-                    strerror(errno));
+            printf(" failed\n");
+            fprintf(stderr, "Error %d: %s\n", errno, strerror(errno));
             return -1;
         }
     }
     else
     {
-        printf("%s: in wrong state (%s), expected (%s)\n",
-               name, state2str(status.state),
-               state2str(action_options[idx].allow));
+        fprintf(stderr, "%s is in the wrong state.\n"
+                        "Current state: %s\n"
+                        "Expected state: %s\n",
+                name, state2str(status.state),
+                state2str(action_options[idx].allow));
         return -1;
     }
 
@@ -335,7 +357,8 @@ int action_func(int argc, char *argv[], unsigned int idx)
 
     if ( retry >= RETRIES )
     {
-        fprintf(stderr, "%s: Operation didn't complete after 30 seconds.\n", name);
+        printf(" failed\n");
+        fprintf(stderr, "Operation didn't complete after 30 seconds.\n");
         return -1;
     }
     else
@@ -347,14 +370,18 @@ int action_func(int argc, char *argv[], unsigned int idx)
             printf(" completed\n");
         else if ( rc < 0 )
         {
-            fprintf(stderr, "%s failed with %d(%s)\n", name, -rc, strerror(-rc));
+            printf(" failed\n");
+            fprintf(stderr, "Error %d: %s\n", -rc, strerror(-rc));
             return -1;
         }
         else
         {
-            fprintf(stderr, "%s: in wrong state (%s), expected (%s)\n",
-               name, state2str(rc),
-               state2str(action_options[idx].expected));
+            printf(" failed\n");
+            fprintf(stderr, "%s is in the wrong state.\n"
+                            "Current state: %s\n"
+                            "Expected state: %s\n",
+                    name, state2str(rc),
+                    state2str(action_options[idx].expected));
             return -1;
         }
     }
-- 
2.7.4


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

[PATCH v1 4/7] livepatch: Set timeout unit to nanoseconds

[Ross Lagerwall]

The hypervisor already expects the timeout from the hypercall to be in
nanoseconds, so set this expectation everywhere.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 tools/libxc/include/xenctrl.h | 2 +-
 xen/common/livepatch.c        | 4 ++--
 xen/include/public/sysctl.h   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 2c83544..4ab0f57 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2703,7 +2703,7 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
  * The operations are asynchronous and the hypervisor may take a while
  * to complete them. The `timeout` offers an option to expire the
  * operation if it could not be completed within the specified time
- * (in ms). Value of 0 means let hypervisor decide the best timeout.
+ * (in ns). Value of 0 means let hypervisor decide the best timeout.
  */
 int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout);
 int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index fc8ef99..246e673 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1227,8 +1227,8 @@ static int schedule_work(struct payload *data, uint32_t cmd, uint32_t timeout)
     livepatch_work.data = data;
     livepatch_work.timeout = timeout ?: MILLISECS(30);
 
-    dprintk(XENLOG_DEBUG, LIVEPATCH "%s: timeout is %"PRI_stime"ms\n",
-            data->name, livepatch_work.timeout / MILLISECS(1));
+    dprintk(XENLOG_DEBUG, LIVEPATCH "%s: timeout is %"PRIu32"ns\n",
+            data->name, livepatch_work.timeout);
 
     atomic_set(&livepatch_work.semaphore, -1);
 
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 28ac56c..3c67858 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1068,7 +1068,7 @@ struct xen_sysctl_livepatch_action {
 #define LIVEPATCH_ACTION_REPLACE      4
     uint32_t cmd;                           /* IN: LIVEPATCH_ACTION_*. */
     uint32_t timeout;                       /* IN: Zero if no timeout. */
-                                            /* Or upper bound of time (ms) */
+                                            /* Or upper bound of time (ns) */
                                             /* for operation to take. */
 };
 typedef struct xen_sysctl_livepatch_action xen_sysctl_livepatch_action_t;
-- 
2.7.4


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

[PATCH v1 5/7] tools/livepatch: Remove pointless retry loop

[Ross Lagerwall]

The default timeout in the hypervisor for a livepatch operation is 30 ms,
but xen-livepatch currently waits for up to 30 seconds for the operation
to complete. Instead, remove the retry loop and simply wait for 2 * 30 ms
for the operation to complete. The extra period is to account for the
time to actually start the operation.

Furthermore, have xen-livepatch set the hypervisor timeout rather than
relying on the hypervisor default since the tool doesn't know how long
it will be. Use nanosleep rather than usleep since usleep has been
removed from POSIX.1-2008.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 tools/misc/xen-livepatch.c | 102 ++++++++++++++++++++++-----------------------
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index afd8c48..d683860 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -7,6 +7,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <time.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -265,17 +266,31 @@ struct {
     },
 };
 
-/* Go around 300 * 0.1 seconds = 30 seconds. */
-#define RETRIES 300
-/* aka 0.1 second */
-#define DELAY 100000
+/* The hypervisor timeout for the live patching operation is 30 msec,
+ * but it could take some time for the operation to start, so wait twice
+ * that period. */
+#define HYPERVISOR_TIMEOUT 30000000 /* in ns */
+#define DELAY (2 * HYPERVISOR_TIMEOUT)
+
+static void nanosleep_retry(long ns)
+{
+    struct timespec req, rem;
+    int rc;
+
+    rem.tv_sec = 0;
+    rem.tv_nsec = ns;
+
+    do {
+        req = rem;
+        rc = nanosleep(&req, &rem);
+    } while (rc != -1 && errno == EINTR);
+}
 
 int action_func(int argc, char *argv[], unsigned int idx)
 {
     char name[XEN_LIVEPATCH_NAME_SIZE];
-    int rc, original_state;
+    int rc;
     xen_livepatch_status_t status;
-    unsigned int retry = 0;
 
     if ( argc != 1 )
     {
@@ -315,11 +330,11 @@ int action_func(int argc, char *argv[], unsigned int idx)
     /* Perform action. */
     if ( action_options[idx].allow & status.state )
     {
-        printf("%s %s:", action_options[idx].verb, name);
-        rc = action_options[idx].function(xch, name, 0);
+        printf("%s %s... ", action_options[idx].verb, name);
+        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT);
         if ( rc )
         {
-            printf(" failed\n");
+            printf("failed\n");
             fprintf(stderr, "Error %d: %s\n", errno, strerror(errno));
             return -1;
         }
@@ -334,56 +349,41 @@ int action_func(int argc, char *argv[], unsigned int idx)
         return -1;
     }
 
-    original_state = status.state;
-    do {
-        rc = xc_livepatch_get(xch, name, &status);
-        if ( rc )
-        {
-            rc = -errno;
-            break;
-        }
+    nanosleep_retry(DELAY);
+    rc = xc_livepatch_get(xch, name, &status);
 
-        if ( status.state != original_state )
-            break;
-        if ( status.rc && status.rc != -XEN_EAGAIN )
-        {
-            rc = status.rc;
-            break;
-        }
+    if ( rc )
+        rc = -errno;
+    else if ( status.rc )
+        rc = status.rc;
 
-        printf(".");
-        usleep(DELAY);
-    } while ( ++retry < RETRIES );
+    if ( rc == -XEN_EAGAIN )
+    {
+        printf("failed\n");
+        fprintf(stderr, "Operation didn't complete.\n");
+        return -1;
+    }
+
+    if ( rc == 0 )
+        rc = status.state;
 
-    if ( retry >= RETRIES )
+    if ( action_options[idx].expected == rc )
+        printf("completed\n");
+    else if ( rc < 0 )
     {
-        printf(" failed\n");
-        fprintf(stderr, "Operation didn't complete after 30 seconds.\n");
+        printf("failed\n");
+        fprintf(stderr, "Error %d: %s\n", -rc, strerror(-rc));
         return -1;
     }
     else
     {
-        if ( rc == 0 )
-            rc = status.state;
-
-        if ( action_options[idx].expected == rc )
-            printf(" completed\n");
-        else if ( rc < 0 )
-        {
-            printf(" failed\n");
-            fprintf(stderr, "Error %d: %s\n", -rc, strerror(-rc));
-            return -1;
-        }
-        else
-        {
-            printf(" failed\n");
-            fprintf(stderr, "%s is in the wrong state.\n"
-                            "Current state: %s\n"
-                            "Expected state: %s\n",
-                    name, state2str(rc),
-                    state2str(action_options[idx].expected));
-            return -1;
-        }
+        printf("failed\n");
+        fprintf(stderr, "%s is in the wrong state.\n"
+                        "Current state: %s\n"
+                        "Expected state: %s\n",
+                name, state2str(rc),
+                state2str(action_options[idx].expected));
+        return -1;
     }
 
     return 0;
-- 
2.7.4


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

[PATCH v1 6/7] tools/livepatch: Remove unused struct member

[Ross Lagerwall]

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 tools/misc/xen-livepatch.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index d683860..9633c4a 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -238,7 +238,6 @@ struct {
     const char *name;
     const char *verb;
     int (*function)(xc_interface *xch, char *name, uint32_t timeout);
-    unsigned int executed; /* Has the function been called?. */
 } action_options[] = {
     {   .allow = LIVEPATCH_STATE_CHECKED,
         .expected = LIVEPATCH_STATE_APPLIED,
-- 
2.7.4


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

[PATCH v1 7/7] tools/livepatch: Exit with 2 if a timeout occurs

[Ross Lagerwall]

Exit with 0 for success.
Exit with 1 for an error.
Exit with 2 if the operation should be retried for any reason (e.g. a
timeout or because another operation was in progress).

This allows a program or script driving xen-livepatch to determine if
the operation should be retried without parsing the output.

Fix a number of incorrect uses of errno after an operation that could
set it (e.g. fprintf, close).

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 tools/misc/xen-livepatch.c | 60 +++++++++++++++++++++++++++++++---------------
 1 file changed, 41 insertions(+), 19 deletions(-)

diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 9633c4a..370ad60 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -101,9 +101,10 @@ static int list_func(int argc, char *argv[])
         rc = xc_livepatch_list(xch, MAX_LEN, idx, info, name, len, &done, &left);
         if ( rc )
         {
+            rc = errno;
             fprintf(stderr, "Failed to list %d/%d.\n"
                             "Error %d: %s\n",
-                    idx, left, errno, strerror(errno));
+                    idx, left, rc, strerror(rc));
             break;
         }
         if ( !idx )
@@ -175,37 +176,40 @@ static int upload_func(int argc, char *argv[])
     fd = open(filename, O_RDONLY);
     if ( fd < 0 )
     {
+        int saved_errno = errno;
         fprintf(stderr, "Could not open %s.\n"
                         "Error %d: %s\n",
-                filename, errno, strerror(errno));
-        return errno;
+                filename, saved_errno, strerror(saved_errno));
+        return saved_errno;
     }
     if ( stat(filename, &buf) != 0 )
     {
+        int saved_errno = errno;
         fprintf(stderr, "Could not get size of %s.\n"
                         "Error %d: %s\n",
-                filename, errno, strerror(errno));
+                filename, saved_errno, strerror(saved_errno));
         close(fd);
-        return errno;
+        return saved_errno;
     }
 
     len = buf.st_size;
     fbuf = mmap(0, len, PROT_READ, MAP_PRIVATE, fd, 0);
     if ( fbuf == MAP_FAILED )
     {
+        int saved_errno = errno;
         fprintf(stderr, "Could not map %s.\n"
                         "Error %d: %s\n",
-                filename, errno, strerror(errno));
+                filename, saved_errno, strerror(saved_errno));
         close (fd);
-        return errno;
+        return saved_errno;
     }
     printf("Uploading %s... ", filename);
     rc = xc_livepatch_upload(xch, name, fbuf, len);
     if ( rc )
     {
+        rc = errno;
         printf("failed\n");
-        fprintf(stderr, "Error %d: %s\n",
-                errno, strerror(errno));
+        fprintf(stderr, "Error %d: %s\n", rc, strerror(rc));
     }
     else
         printf("completed\n");
@@ -216,8 +220,6 @@ static int upload_func(int argc, char *argv[])
         fprintf(stderr, "Could not unmap %s.\n"
                         "Error %d: %s\n",
                 filename, errno, strerror(errno));
-        if ( !rc )
-            rc = errno;
     }
     close(fd);
 
@@ -307,17 +309,18 @@ int action_func(int argc, char *argv[], unsigned int idx)
     rc = xc_livepatch_get(xch, name, &status);
     if ( rc )
     {
+        int saved_errno = errno;
         fprintf(stderr, "Failed to get status of %s.\n"
                         "Error %d: %s\n",
-                name, errno, strerror(errno));
-        return -1;
+                name, saved_errno, strerror(saved_errno));
+        return saved_errno;
     }
     if ( status.rc == -XEN_EAGAIN )
     {
         fprintf(stderr,
                 "Cannot execute %s.\n"
                 "Operation already in progress.\n", action_options[idx].name);
-        return -1;
+        return EAGAIN;
     }
 
     if ( status.state == action_options[idx].expected )
@@ -333,9 +336,11 @@ int action_func(int argc, char *argv[], unsigned int idx)
         rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT);
         if ( rc )
         {
+            int saved_errno = errno;
             printf("failed\n");
-            fprintf(stderr, "Error %d: %s\n", errno, strerror(errno));
-            return -1;
+            fprintf(stderr, "Error %d: %s\n",
+                    saved_errno, strerror(saved_errno));
+            return saved_errno;
         }
     }
     else
@@ -360,7 +365,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
     {
         printf("failed\n");
         fprintf(stderr, "Operation didn't complete.\n");
-        return -1;
+        return EAGAIN;
     }
 
     if ( rc == 0 )
@@ -372,7 +377,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
     {
         printf("failed\n");
         fprintf(stderr, "Error %d: %s\n", -rc, strerror(-rc));
-        return -1;
+        return -rc;
     }
     else
     {
@@ -485,7 +490,24 @@ int main(int argc, char *argv[])
 
     xc_interface_close(xch);
 
-    return !!ret;
+    /*
+     * Exitcode 0 for success.
+     * Exitcode 1 for an error.
+     * Exitcode 2 if the operation should be retried for any reason (e.g. a
+     * timeout or because another operation was in progress).
+     */
+
+#define EXIT_TIMEOUT (EXIT_FAILURE + 1)
+    switch ( ret )
+    {
+    case 0:
+        return EXIT_SUCCESS;
+    case EAGAIN:
+    case EBUSY:
+        return EXIT_TIMEOUT;
+    default:
+        return EXIT_FAILURE;
+    }
 }
 
 /*
-- 
2.7.4


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

Re: [PATCH v1 7/7] tools/livepatch: Exit with 2 if a timeout occurs

[Wei Liu]

On Mon, Dec 12, 2016 at 04:18:10PM +0000, Ross Lagerwall wrote:
> Exit with 0 for success.
> Exit with 1 for an error.
> Exit with 2 if the operation should be retried for any reason (e.g. a
> timeout or because another operation was in progress).
> 
> This allows a program or script driving xen-livepatch to determine if
> the operation should be retried without parsing the output.
> 
> Fix a number of incorrect uses of errno after an operation that could
> set it (e.g. fprintf, close).

This should probably be split out to another patch so that it can be
backported to 4.7 and 4.8?

>      {
> @@ -485,7 +490,24 @@ int main(int argc, char *argv[])
>  
>      xc_interface_close(xch);
>  
> -    return !!ret;
> +    /*
> +     * Exitcode 0 for success.
> +     * Exitcode 1 for an error.
> +     * Exitcode 2 if the operation should be retried for any reason (e.g. a
> +     * timeout or because another operation was in progress).
> +     */
> +
> +#define EXIT_TIMEOUT (EXIT_FAILURE + 1)

I doubt there would be any other value used for EXIT_FAILURE, but to
make sure the code matches the comment, please:

  BUILD_BUG_ON(EXIT_SUCCESS != 0);
  BUILD_BUG_ON(EXIT_FAILURE != 1);


> +    switch ( ret )
> +    {
> +    case 0:
> +        return EXIT_SUCCESS;
> +    case EAGAIN:
> +    case EBUSY:
> +        return EXIT_TIMEOUT;
> +    default:
> +        return EXIT_FAILURE;
> +    }
>  }
>  
>  /*
> -- 
> 2.7.4
> 

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

Re: [PATCH v1 1/7] tools/livepatch: Show the correct expected state before action

[Wei Liu]

On Mon, Dec 12, 2016 at 04:18:04PM +0000, Ross Lagerwall wrote:
> Somewhat confusingly, before the action has been executed the patch is
> expected to be in the "allowed" state, not the "expected" state.  The

ITYM "allow", according to your code.

> check for this was correct but the subsequent error message was not.
> Fix the error message to show this state correctly.
> 
> Before:
>     $ xen-livepatch unload test
>     test: in wrong state (APPLIED), expected (unknown)
> After:
>     $ xen-livepatch unload test
>     test: in wrong state (APPLIED), expected (CHECKED)
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/misc/xen-livepatch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> index 2de04c0..f6c7c8a 100644
> --- a/tools/misc/xen-livepatch.c
> +++ b/tools/misc/xen-livepatch.c
> @@ -308,7 +308,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
>      {
>          printf("%s: in wrong state (%s), expected (%s)\n",
>                 name, state2str(status.state),
> -               state2str(action_options[idx].expected));
> +               state2str(action_options[idx].allow));
>          return -1;
>      }
>  
> -- 
> 2.7.4
> 

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

Re: [PATCH v1 2/7] tools/livepatch: Set stdout and stderr unbuffered

[Wei Liu]

On Mon, Dec 12, 2016 at 04:18:05PM +0000, Ross Lagerwall wrote:
> Using both stdout and stderr interleaved without newlines can result in
> strange output when using line buffered mode (e.g. a terminal) or when
> fully buffered (e.g. redirected to a file). Set both to unbuffered mode
> to fix this.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  tools/misc/xen-livepatch.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> index f6c7c8a..2f6721b 100644
> --- a/tools/misc/xen-livepatch.c
> +++ b/tools/misc/xen-livepatch.c
> @@ -330,7 +330,6 @@ int action_func(int argc, char *argv[], unsigned int idx)
>          }
>  
>          printf(".");
> -        fflush(stdout);
>          usleep(DELAY);
>      } while ( ++retry < RETRIES );
>  
> @@ -416,6 +415,13 @@ int main(int argc, char *argv[])
>  {
>      int i, j = 0, ret;
>  
> +    /*
> +     * Set stdout and stderr to be unbuffered to avoid having to fflush
> +     * when printing without a newline.
> +     */
> +    setvbuf(stdout, NULL, _IONBF, 0);
> +    setvbuf(stderr, NULL, _IONBF, 0);
> +

OOI why would you need to set stderr? It is unbuffered by default.

There is no harm in setting it though.

And perhaps you should check the return value of setvbuf?

Wei.

>      if ( argc  <= 1 )
>      {
>          show_help();
> -- 
> 2.7.4
> 

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

Re: [PATCH v1 3/7] tools/livepatch: Improve output

[Wei Liu]

On Mon, Dec 12, 2016 at 04:18:06PM +0000, Ross Lagerwall wrote:
> Improving the output of xen-livepatch, which is currently hard to read,
> especially when an error occurs.
> 
> Some examples of the changes:
> Before:
>     $ xen-livepatch apply test
>     Performing apply:. completed
> After:
>     $ xen-livepatch apply test
>     Applying test:. completed
> 
> Before:
>     $ xen-livepatch apply test2
>     test2 failed with 22(Invalid argument)
>     Performing apply: (no newline)
> After:
>     $ xen-livepatch apply test2
>     Applying test2: failed
>     Error 22: Invalid argument
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

I will let Konrad quibble about the actual text.

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

Re: [PATCH v1 4/7] livepatch: Set timeout unit to nanoseconds

[Wei Liu]

On Mon, Dec 12, 2016 at 04:18:07PM +0000, Ross Lagerwall wrote:
> The hypervisor already expects the timeout from the hypercall to be in
> nanoseconds, so set this expectation everywhere.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  tools/libxc/include/xenctrl.h | 2 +-
>  xen/common/livepatch.c        | 4 ++--
>  xen/include/public/sysctl.h   | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 2c83544..4ab0f57 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2703,7 +2703,7 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
>   * The operations are asynchronous and the hypervisor may take a while
>   * to complete them. The `timeout` offers an option to expire the
>   * operation if it could not be completed within the specified time
> - * (in ms). Value of 0 means let hypervisor decide the best timeout.
> + * (in ns). Value of 0 means let hypervisor decide the best timeout.

Acked-by: Wei Liu <wei.liu2@citrix.com>

>   */
>  int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout);
>  int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index fc8ef99..246e673 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -1227,8 +1227,8 @@ static int schedule_work(struct payload *data, uint32_t cmd, uint32_t timeout)
>      livepatch_work.data = data;
>      livepatch_work.timeout = timeout ?: MILLISECS(30);
>  
> -    dprintk(XENLOG_DEBUG, LIVEPATCH "%s: timeout is %"PRI_stime"ms\n",
> -            data->name, livepatch_work.timeout / MILLISECS(1));
> +    dprintk(XENLOG_DEBUG, LIVEPATCH "%s: timeout is %"PRIu32"ns\n",
> +            data->name, livepatch_work.timeout);
>  
>      atomic_set(&livepatch_work.semaphore, -1);
>  
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 28ac56c..3c67858 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1068,7 +1068,7 @@ struct xen_sysctl_livepatch_action {
>  #define LIVEPATCH_ACTION_REPLACE      4
>      uint32_t cmd;                           /* IN: LIVEPATCH_ACTION_*. */
>      uint32_t timeout;                       /* IN: Zero if no timeout. */
> -                                            /* Or upper bound of time (ms) */
> +                                            /* Or upper bound of time (ns) */

Is this only fixing a typo or does it constitute a change in behaviour?
The latter would result in bumping the sysctl number. But from the look
of the patch I think it is doing the former.

>                                              /* for operation to take. */
>  };
>  typedef struct xen_sysctl_livepatch_action xen_sysctl_livepatch_action_t;
> -- 
> 2.7.4
> 

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

Re: [PATCH v1 5/7] tools/livepatch: Remove pointless retry loop

[Wei Liu]

On Mon, Dec 12, 2016 at 04:18:08PM +0000, Ross Lagerwall wrote:
> The default timeout in the hypervisor for a livepatch operation is 30 ms,
> but xen-livepatch currently waits for up to 30 seconds for the operation
> to complete. Instead, remove the retry loop and simply wait for 2 * 30 ms
> for the operation to complete. The extra period is to account for the
> time to actually start the operation.
> 
> Furthermore, have xen-livepatch set the hypervisor timeout rather than
> relying on the hypervisor default since the tool doesn't know how long
> it will be. Use nanosleep rather than usleep since usleep has been
> removed from POSIX.1-2008.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  tools/misc/xen-livepatch.c | 102 ++++++++++++++++++++++-----------------------
>  1 file changed, 51 insertions(+), 51 deletions(-)
> 
> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> index afd8c48..d683860 100644
> --- a/tools/misc/xen-livepatch.c
> +++ b/tools/misc/xen-livepatch.c
> @@ -7,6 +7,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <time.h>
>  #include <sys/mman.h>
>  #include <sys/stat.h>
>  #include <unistd.h>

Please order the inclusion in alphabetic order.

> @@ -265,17 +266,31 @@ struct {
>      },
>  };
>  
> -/* Go around 300 * 0.1 seconds = 30 seconds. */
> -#define RETRIES 300
> -/* aka 0.1 second */
> -#define DELAY 100000
> +/* The hypervisor timeout for the live patching operation is 30 msec,
> + * but it could take some time for the operation to start, so wait twice
> + * that period. */
> +#define HYPERVISOR_TIMEOUT 30000000 /* in ns */

Use HYPERVISOR_TIMEOUT_NS and remove the comment?

> +#define DELAY (2 * HYPERVISOR_TIMEOUT)
> +
> +static void nanosleep_retry(long ns)
> +{
> +    struct timespec req, rem;
> +    int rc;
> +
> +    rem.tv_sec = 0;
> +    rem.tv_nsec = ns;
> +
> +    do {
> +        req = rem;
> +        rc = nanosleep(&req, &rem);
> +    } while (rc != -1 && errno == EINTR);

Coding style.

And shouldn't it be ( rc == -1 && errno == EINTR ) ?

I don't really have more comment on this approach.

Wei.

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

Re: [PATCH v1 6/7] tools/livepatch: Remove unused struct member

[Wei Liu]

On Mon, Dec 12, 2016 at 04:18:09PM +0000, Ross Lagerwall wrote:
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v1 3/7] tools/livepatch: Improve output

[Konrad Rzeszutek Wilk]

On Mon, Dec 12, 2016 at 05:02:52PM +0000, Wei Liu wrote:
> On Mon, Dec 12, 2016 at 04:18:06PM +0000, Ross Lagerwall wrote:
> > Improving the output of xen-livepatch, which is currently hard to read,
> > especially when an error occurs.
> > 
> > Some examples of the changes:
> > Before:
> >     $ xen-livepatch apply test
> >     Performing apply:. completed
> > After:
> >     $ xen-livepatch apply test
> >     Applying test:. completed
> > 
> > Before:
> >     $ xen-livepatch apply test2
> >     test2 failed with 22(Invalid argument)
> >     Performing apply: (no newline)
> > After:
> >     $ xen-livepatch apply test2
> >     Applying test2: failed
> >     Error 22: Invalid argument
> > 
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> I will let Konrad quibble about the actual text.

No quibbles from me.


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

Re: [PATCH v1 4/7] livepatch: Set timeout unit to nanoseconds

[Konrad Rzeszutek Wilk]

On Mon, Dec 12, 2016 at 05:02:59PM +0000, Wei Liu wrote:
> On Mon, Dec 12, 2016 at 04:18:07PM +0000, Ross Lagerwall wrote:
> > The hypervisor already expects the timeout from the hypercall to be in
> > nanoseconds, so set this expectation everywhere.
> > 
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> > ---
> >  tools/libxc/include/xenctrl.h | 2 +-
> >  xen/common/livepatch.c        | 4 ++--
> >  xen/include/public/sysctl.h   | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> > index 2c83544..4ab0f57 100644
> > --- a/tools/libxc/include/xenctrl.h
> > +++ b/tools/libxc/include/xenctrl.h
> > @@ -2703,7 +2703,7 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
> >   * The operations are asynchronous and the hypervisor may take a while
> >   * to complete them. The `timeout` offers an option to expire the
> >   * operation if it could not be completed within the specified time
> > - * (in ms). Value of 0 means let hypervisor decide the best timeout.
> > + * (in ns). Value of 0 means let hypervisor decide the best timeout.
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> >   */
> >  int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout);
> >  int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
> > diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> > index fc8ef99..246e673 100644
> > --- a/xen/common/livepatch.c
> > +++ b/xen/common/livepatch.c
> > @@ -1227,8 +1227,8 @@ static int schedule_work(struct payload *data, uint32_t cmd, uint32_t timeout)
> >      livepatch_work.data = data;
> >      livepatch_work.timeout = timeout ?: MILLISECS(30);
> >  
> > -    dprintk(XENLOG_DEBUG, LIVEPATCH "%s: timeout is %"PRI_stime"ms\n",
> > -            data->name, livepatch_work.timeout / MILLISECS(1));
> > +    dprintk(XENLOG_DEBUG, LIVEPATCH "%s: timeout is %"PRIu32"ns\n",
> > +            data->name, livepatch_work.timeout);
> >  
> >      atomic_set(&livepatch_work.semaphore, -1);
> >  
> > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> > index 28ac56c..3c67858 100644
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -1068,7 +1068,7 @@ struct xen_sysctl_livepatch_action {
> >  #define LIVEPATCH_ACTION_REPLACE      4
> >      uint32_t cmd;                           /* IN: LIVEPATCH_ACTION_*. */
> >      uint32_t timeout;                       /* IN: Zero if no timeout. */
> > -                                            /* Or upper bound of time (ms) */
> > +                                            /* Or upper bound of time (ns) */
> 
> Is this only fixing a typo or does it constitute a change in behaviour?

Type (and it should also be changed in the docs/misc/livepatch.markdown)

> The latter would result in bumping the sysctl number. But from the look
> of the patch I think it is doing the former.
> 
> >                                              /* for operation to take. */
> >  };
> >  typedef struct xen_sysctl_livepatch_action xen_sysctl_livepatch_action_t;
> > -- 
> > 2.7.4
> > 

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

Re: [PATCH v1 6/7] tools/livepatch: Remove unused struct member

[Konrad Rzeszutek Wilk]

On Mon, Dec 12, 2016 at 04:18:09PM +0000, Ross Lagerwall wrote:
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  tools/misc/xen-livepatch.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> index d683860..9633c4a 100644
> --- a/tools/misc/xen-livepatch.c
> +++ b/tools/misc/xen-livepatch.c
> @@ -238,7 +238,6 @@ struct {
>      const char *name;
>      const char *verb;
>      int (*function)(xc_interface *xch, char *name, uint32_t timeout);
> -    unsigned int executed; /* Has the function been called?. */
>  } action_options[] = {
>      {   .allow = LIVEPATCH_STATE_CHECKED,
>          .expected = LIVEPATCH_STATE_APPLIED,
> -- 
> 2.7.4
> 

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

Re: [PATCH v1 2/7] tools/livepatch: Set stdout and stderr unbuffered

[Ross Lagerwall]

On 12/12/2016 05:02 PM, Wei Liu wrote:
> On Mon, Dec 12, 2016 at 04:18:05PM +0000, Ross Lagerwall wrote:
>> Using both stdout and stderr interleaved without newlines can result in
>> strange output when using line buffered mode (e.g. a terminal) or when
>> fully buffered (e.g. redirected to a file). Set both to unbuffered mode
>> to fix this.
>>
>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>> ---
>>  tools/misc/xen-livepatch.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
>> index f6c7c8a..2f6721b 100644
>> --- a/tools/misc/xen-livepatch.c
>> +++ b/tools/misc/xen-livepatch.c
>> @@ -330,7 +330,6 @@ int action_func(int argc, char *argv[], unsigned int idx)
>>          }
>>
>>          printf(".");
>> -        fflush(stdout);
>>          usleep(DELAY);
>>      } while ( ++retry < RETRIES );
>>
>> @@ -416,6 +415,13 @@ int main(int argc, char *argv[])
>>  {
>>      int i, j = 0, ret;
>>
>> +    /*
>> +     * Set stdout and stderr to be unbuffered to avoid having to fflush
>> +     * when printing without a newline.
>> +     */
>> +    setvbuf(stdout, NULL, _IONBF, 0);
>> +    setvbuf(stderr, NULL, _IONBF, 0);
>> +
>
> OOI why would you need to set stderr? It is unbuffered by default.
>
> There is no harm in setting it though.

Good point. I'll drop it since it is unnecessary.

>
> And perhaps you should check the return value of setvbuf?
>

I don't think there's anything that can be done if it fails, so there's 
not much point in checking the return value.

-- 
Ross Lagerwall

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

Re: [PATCH v1 2/7] tools/livepatch: Set stdout and stderr unbuffered

[Wei Liu]

On Wed, Dec 14, 2016 at 06:44:43AM +0000, Ross Lagerwall wrote:
[...]
> >
> >And perhaps you should check the return value of setvbuf?
> >
> 
> I don't think there's anything that can be done if it fails, so there's not
> much point in checking the return value.
> 

This is a fair point.

Wei.

> -- 
> Ross Lagerwall

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