* [PATCH v2 1/8] tools/livepatch: Show the correct expected state before action
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
@ 2016-12-14 7:51 ` Ross Lagerwall
2016-12-14 7:51 ` [PATCH v2 2/8] tools/livepatch: Set stdout and stderr unbuffered Ross Lagerwall
` (7 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:51 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
Somewhat confusingly, before the action has been executed the patch is
expected to be in the "allow" 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>
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 2/8] tools/livepatch: Set stdout and stderr unbuffered
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
2016-12-14 7:51 ` [PATCH v2 1/8] tools/livepatch: Show the correct expected state before action Ross Lagerwall
@ 2016-12-14 7:51 ` Ross Lagerwall
2016-12-14 7:54 ` Wei Liu
2016-12-14 7:51 ` [PATCH v2 3/8] tools/livepatch: Improve output Ross Lagerwall
` (6 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:51 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
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 stdout to unbuffered mode
to fix this (stderr is always unbuffered by default).
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
tools/misc/xen-livepatch.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index f6c7c8a..56ae5aa 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,12 @@ int main(int argc, char *argv[])
{
int i, j = 0, ret;
+ /*
+ * Set stdout to be unbuffered to avoid having to fflush when
+ * printing without a newline.
+ */
+ setvbuf(stdout, 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/8] tools/livepatch: Set stdout and stderr unbuffered
2016-12-14 7:51 ` [PATCH v2 2/8] tools/livepatch: Set stdout and stderr unbuffered Ross Lagerwall
@ 2016-12-14 7:54 ` Wei Liu
0 siblings, 0 replies; 16+ messages in thread
From: Wei Liu @ 2016-12-14 7:54 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: Wei Liu, Ian Jackson, xen-devel
On Wed, Dec 14, 2016 at 07:51:54AM +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 stdout to unbuffered mode
> to fix this (stderr is always unbuffered by default).
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> tools/misc/xen-livepatch.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> index f6c7c8a..56ae5aa 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,12 @@ int main(int argc, char *argv[])
> {
> int i, j = 0, ret;
>
> + /*
> + * Set stdout to be unbuffered to avoid having to fflush when
> + * printing without a newline.
> + */
> + setvbuf(stdout, 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/8] tools/livepatch: Improve output
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
2016-12-14 7:51 ` [PATCH v2 1/8] tools/livepatch: Show the correct expected state before action Ross Lagerwall
2016-12-14 7:51 ` [PATCH v2 2/8] tools/livepatch: Set stdout and stderr unbuffered Ross Lagerwall
@ 2016-12-14 7:51 ` Ross Lagerwall
2016-12-14 7:51 ` [PATCH v2 4/8] livepatch: Fix documentation of timeout Ross Lagerwall
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:51 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
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>
---
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 56ae5aa..6e3fcf2 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 4/8] livepatch: Fix documentation of timeout
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
` (2 preceding siblings ...)
2016-12-14 7:51 ` [PATCH v2 3/8] tools/livepatch: Improve output Ross Lagerwall
@ 2016-12-14 7:51 ` Ross Lagerwall
2016-12-14 7:54 ` Wei Liu
2016-12-14 7:51 ` [PATCH v2 5/8] tools/livepatch: Remove pointless retry loop Ross Lagerwall
` (4 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:51 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
The hypervisor expects the timeout from the hypercall to be in
nanoseconds, so document this correctly. Also correctly document
what happens when timeout is set to zero.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
docs/misc/livepatch.markdown | 13 +++++++------
tools/libxc/include/xenctrl.h | 2 +-
xen/common/livepatch.c | 4 ++--
xen/include/public/sysctl.h | 5 +++--
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index 8f0559f..54a6b85 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -689,9 +689,9 @@ The caller provides:
payload. If the operation takes more time than the upper bound of time
the `rc` in `xen_livepatch_status' retrieved via **XEN_SYSCTL_LIVEPATCH_GET**
will be -XEN_EBUSY.
- * `time` the upper bound of time (ms) the cmd should take. Zero means infinite.
- If within the time the operation does not succeed the operation would go in
- error state.
+ * `time` the upper bound of time (ns) the cmd should take. Zero means to use
+ the hypervisor default. If within the time the operation does not succeed
+ the operation would go in error state.
* `pad` - *MUST* be zero.
The return value will be zero unless the provided fields are incorrect.
@@ -706,9 +706,10 @@ The structure is as follow:
struct xen_sysctl_livepatch_action {
xen_livepatch_name_t name; /* IN, name of the patch. */
uint32_t cmd; /* IN: LIVEPATCH_ACTION_* */
- uint32_t time; /* IN: Zero if no timeout. */
- /* Or upper bound of time (ms) */
- /* for operation to take. */
+ uint32_t time; /* IN: If zero then uses */
+ /* hypervisor default. */
+ /* Or upper bound of time (ns) */
+ /* for operation to take. */
};
</pre>
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..00f5e77 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1067,8 +1067,9 @@ struct xen_sysctl_livepatch_action {
#define LIVEPATCH_ACTION_APPLY 3
#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) */
+ uint32_t timeout; /* IN: If zero then uses */
+ /* hypervisor default. */
+ /* 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 5/8] tools/livepatch: Remove pointless retry loop
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
` (3 preceding siblings ...)
2016-12-14 7:51 ` [PATCH v2 4/8] livepatch: Fix documentation of timeout Ross Lagerwall
@ 2016-12-14 7:51 ` Ross Lagerwall
2016-12-14 7:55 ` Wei Liu
2016-12-14 7:51 ` [PATCH v2 6/8] tools/livepatch: Remove unused struct member Ross Lagerwall
` (3 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:51 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
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 6e3fcf2..4ebe95e 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -9,6 +9,7 @@
#include <string.h>
#include <sys/mman.h>
#include <sys/stat.h>
+#include <time.h>
#include <unistd.h>
#include <xenctrl.h>
#include <xenstore.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_NS 30000000
+#define DELAY (2 * HYPERVISOR_TIMEOUT_NS)
+
+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_NS);
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/8] tools/livepatch: Remove pointless retry loop
2016-12-14 7:51 ` [PATCH v2 5/8] tools/livepatch: Remove pointless retry loop Ross Lagerwall
@ 2016-12-14 7:55 ` Wei Liu
0 siblings, 0 replies; 16+ messages in thread
From: Wei Liu @ 2016-12-14 7:55 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: Wei Liu, Ian Jackson, xen-devel
On Wed, Dec 14, 2016 at 07:51:57AM +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>
Acked-by: Wei Liu <wei.liu2@citrix.com>
But I would also like Korand's ack on this one.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 6/8] tools/livepatch: Remove unused struct member
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
` (4 preceding siblings ...)
2016-12-14 7:51 ` [PATCH v2 5/8] tools/livepatch: Remove pointless retry loop Ross Lagerwall
@ 2016-12-14 7:51 ` Ross Lagerwall
2016-12-14 7:51 ` [PATCH v2 7/8] tools/livepatch: Save errno where needed Ross Lagerwall
` (2 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:51 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Wei Liu <wei.liu2@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 4ebe95e..fd2f968 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 7/8] tools/livepatch: Save errno where needed
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
` (5 preceding siblings ...)
2016-12-14 7:51 ` [PATCH v2 6/8] tools/livepatch: Remove unused struct member Ross Lagerwall
@ 2016-12-14 7:51 ` Ross Lagerwall
2016-12-14 7:55 ` Wei Liu
2016-12-14 7:52 ` [PATCH v2 8/8] tools/livepatch: Exit with 2 if a timeout occurs Ross Lagerwall
2016-12-14 18:16 ` [PATCH v2 0/8] xen-livepatch misc fixes/changes Konrad Rzeszutek Wilk
8 siblings, 1 reply; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:51 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
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 | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index fd2f968..140445d 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);
@@ -333,8 +335,10 @@ int action_func(int argc, char *argv[], unsigned int idx)
rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS);
if ( rc )
{
+ int saved_errno = errno;
printf("failed\n");
- fprintf(stderr, "Error %d: %s\n", errno, strerror(errno));
+ fprintf(stderr, "Error %d: %s\n",
+ saved_errno, strerror(saved_errno));
return -1;
}
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 8/8] tools/livepatch: Exit with 2 if a timeout occurs
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
` (6 preceding siblings ...)
2016-12-14 7:51 ` [PATCH v2 7/8] tools/livepatch: Save errno where needed Ross Lagerwall
@ 2016-12-14 7:52 ` Ross Lagerwall
2016-12-14 7:56 ` Wei Liu
2016-12-14 18:16 ` [PATCH v2 0/8] xen-livepatch misc fixes/changes Konrad Rzeszutek Wilk
8 siblings, 1 reply; 16+ messages in thread
From: Ross Lagerwall @ 2016-12-14 7:52 UTC (permalink / raw)
To: xen-devel; +Cc: Ross Lagerwall, Ian Jackson, Wei Liu
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.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
tools/misc/xen-livepatch.c | 37 ++++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 140445d..af9fcd6 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -15,6 +15,7 @@
#include <xenstore.h>
#include <xen/errno.h>
+#include <xen-tools/libs.h>
static xc_interface *xch;
@@ -309,17 +310,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 )
@@ -339,7 +341,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
printf("failed\n");
fprintf(stderr, "Error %d: %s\n",
saved_errno, strerror(saved_errno));
- return -1;
+ return saved_errno;
}
}
else
@@ -364,7 +366,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 )
@@ -376,7 +378,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
{
@@ -488,7 +490,28 @@ 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)
+
+ BUILD_BUG_ON(EXIT_SUCCESS != 0);
+ BUILD_BUG_ON(EXIT_FAILURE != 1);
+ BUILD_BUG_ON(EXIT_TIMEOUT != 2);
+
+ 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 0/8] xen-livepatch misc fixes/changes
2016-12-14 7:51 [PATCH v2 0/8] xen-livepatch misc fixes/changes Ross Lagerwall
` (7 preceding siblings ...)
2016-12-14 7:52 ` [PATCH v2 8/8] tools/livepatch: Exit with 2 if a timeout occurs Ross Lagerwall
@ 2016-12-14 18:16 ` Konrad Rzeszutek Wilk
2016-12-14 20:32 ` Konrad Rzeszutek Wilk
8 siblings, 1 reply; 16+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-12-14 18:16 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: Wei Liu, Ian Jackson, xen-devel
On Wed, Dec 14, 2016 at 07:51:52AM +0000, Ross Lagerwall wrote:
> 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.
Hey Ross,
I've applied and reviewed some of the patches. Testing them right now
and if theere are no issues will push to staging shortly.
Thank you for the fixes!
>
> Changed in v2:
> * Fix minor comments.
> * Split the last patch into two.
>
> Ross Lagerwall (8):
> tools/livepatch: Show the correct expected state before action
> tools/livepatch: Set stdout and stderr unbuffered
> tools/livepatch: Improve output
> livepatch: Fix documentation of timeout
> tools/livepatch: Remove pointless retry loop
> tools/livepatch: Remove unused struct member
> tools/livepatch: Save errno where needed
> tools/livepatch: Exit with 2 if a timeout occurs
>
> docs/misc/livepatch.markdown | 13 +--
> tools/libxc/include/xenctrl.h | 2 +-
> tools/misc/xen-livepatch.c | 218 ++++++++++++++++++++++++++----------------
> xen/common/livepatch.c | 4 +-
> xen/include/public/sysctl.h | 5 +-
> 5 files changed, 151 insertions(+), 91 deletions(-)
>
> --
> 2.7.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 0/8] xen-livepatch misc fixes/changes
2016-12-14 18:16 ` [PATCH v2 0/8] xen-livepatch misc fixes/changes Konrad Rzeszutek Wilk
@ 2016-12-14 20:32 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 16+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-12-14 20:32 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: Wei Liu, Ian Jackson, xen-devel
On Wed, Dec 14, 2016 at 01:16:00PM -0500, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 14, 2016 at 07:51:52AM +0000, Ross Lagerwall wrote:
> > 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.
>
> Hey Ross,
>
> I've applied and reviewed some of the patches. Testing them right now
> and if theere are no issues will push to staging shortly.
Completed. Patches pushed in staging. Thank you again!
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread