From: Julien Grall <julien@xen.org>
To: xen-devel@lists.xenproject.org
Cc: raphning@amazon.co.uk, doebel@amazon.de,
Julien Grall <jgrall@amazon.com>,
Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
Juergen Gross <jgross@suse.com>, Julien Grall <julien@xen.org>
Subject: [PATCH 03/10] tools/xenstore: Don't assume conn->in points to the LU request
Date: Wed, 16 Jun 2021 15:43:17 +0100 [thread overview]
Message-ID: <20210616144324.31652-4-julien@xen.org> (raw)
In-Reply-To: <20210616144324.31652-1-julien@xen.org>
From: Julien Grall <jgrall@amazon.com>
call_delayed() is currently assuming that conn->in is NULL when
handling delayed request. However, the connection is not paused.
Therefore new request can be processed and conn->in may be non-NULL
if we have only received a partial request.
Furthermore, as we overwrite conn->in, the current partial request
will not be transferred. This will result to corrupt the connection.
Rather than updating conn->in, stash the LU request in lu_status and
let each callback for delayed request to update conn->in when
necessary.
To keep a sane interface, the code to write the "OK" response the
LU request is moved in xenstored_core.c.
Fixes: c5ca1404b4 ("tools/xenstore: add support for delaying execution of a xenstore request")
Fixes: ed6eebf17d ("tools/xenstore: dump the xenstore state for live update")
Signed-off-by: Julien Grall <jgrall@amazon.com>
----
This is fixing bugs from two separate commits. I couldn't figure out
how to split in two patches without breaking bisection.
---
tools/xenstore/xenstored_control.c | 41 ++++++++++++++++++++++++++++--
tools/xenstore/xenstored_control.h | 3 +++
tools/xenstore/xenstored_core.c | 17 +++----------
3 files changed, 46 insertions(+), 15 deletions(-)
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index d08a2b961432..7acc2d134f9f 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -50,6 +50,9 @@ struct live_update {
/* For verification the correct connection is acting. */
struct connection *conn;
+ /* Pointer to the command used to request LU */
+ struct buffered_data *in;
+
#ifdef __MINIOS__
void *kernel;
unsigned int kernel_size;
@@ -100,6 +103,7 @@ static const char *lu_begin(struct connection *conn)
if (!lu_status)
return "Allocation failure.";
lu_status->conn = conn;
+ lu_status->in = conn->in;
talloc_set_destructor(lu_status, lu_destroy);
return NULL;
@@ -110,11 +114,34 @@ struct connection *lu_get_connection(void)
return lu_status ? lu_status->conn : NULL;
}
+unsigned int lu_write_response(FILE *fp)
+{
+ struct xsd_sockmsg msg;
+
+ assert(lu_status);
+
+ msg = lu_status->in->hdr.msg;
+
+ msg.len = sizeof("OK");
+ if (fp && fwrite(&msg, sizeof(msg), 1, fp) != 1)
+ return 0;
+ if (fp && fwrite("OK", msg.len, 1, fp) != 1)
+ return 0;
+
+ return sizeof(msg) + msg.len;
+}
+
#else
struct connection *lu_get_connection(void)
{
return NULL;
}
+
+unsigned int lu_write_response(FILE *fp)
+{
+ /* Unsupported */
+ return 0;
+}
#endif
struct cmd_s {
@@ -658,6 +685,8 @@ static bool do_lu_start(struct delayed_request *req)
{
time_t now = time(NULL);
const char *ret;
+ struct buffered_data *saved_in;
+ struct connection *conn = lu_status->conn;
if (!lu_check_lu_allowed()) {
if (now < lu_status->started_at + lu_status->timeout)
@@ -668,8 +697,9 @@ static bool do_lu_start(struct delayed_request *req)
}
}
+ assert(req->in == lu_status->in);
/* Dump out internal state, including "OK" for live update. */
- ret = lu_dump_state(req->in, lu_status->conn);
+ ret = lu_dump_state(req->in, conn);
if (!ret) {
/* Perform the activation of new binary. */
ret = lu_activate_binary(req->in);
@@ -677,7 +707,14 @@ static bool do_lu_start(struct delayed_request *req)
/* We will reach this point only in case of failure. */
out:
- send_reply(lu_status->conn, XS_CONTROL, ret, strlen(ret) + 1);
+ /*
+ * send_reply() will send the response for conn->in. Save the current
+ * conn->in and restore it afterwards.
+ */
+ saved_in = conn->in;
+ conn->in = req->in;
+ send_reply(conn, XS_CONTROL, ret, strlen(ret) + 1);
+ conn->in = saved_in;
talloc_free(lu_status);
return true;
diff --git a/tools/xenstore/xenstored_control.h b/tools/xenstore/xenstored_control.h
index 6842b8d88760..27d7f19e4b7f 100644
--- a/tools/xenstore/xenstored_control.h
+++ b/tools/xenstore/xenstored_control.h
@@ -20,3 +20,6 @@ int do_control(struct connection *conn, struct buffered_data *in);
void lu_read_state(void);
struct connection *lu_get_connection(void);
+
+/* Write the "OK" response for the live-update command */
+unsigned int lu_write_response(FILE *fp);
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 607187361d84..41b26d7094c8 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -272,15 +272,10 @@ static int undelay_request(void *_req)
static void call_delayed(struct connection *conn, struct delayed_request *req)
{
- assert(conn->in == NULL);
- conn->in = req->in;
-
if (req->func(req)) {
undelay_request(req);
talloc_set_destructor(req, NULL);
}
-
- conn->in = NULL;
}
int delay_request(struct connection *conn, struct buffered_data *in,
@@ -2375,7 +2370,7 @@ const char *dump_state_buffered_data(FILE *fp, const struct connection *c,
struct buffered_data *out, *in = c->in;
bool partial = true;
- if (in && c != lu_get_connection()) {
+ if (in) {
len = in->inhdr ? in->used : sizeof(in->hdr);
if (fp && fwrite(&in->hdr, len, 1, fp) != 1)
return "Dump read data error";
@@ -2416,16 +2411,12 @@ const char *dump_state_buffered_data(FILE *fp, const struct connection *c,
/* Add "OK" for live-update command. */
if (c == lu_get_connection()) {
- struct xsd_sockmsg msg = c->in->hdr.msg;
+ unsigned int rc = lu_write_response(fp);
- msg.len = sizeof("OK");
- if (fp && fwrite(&msg, sizeof(msg), 1, fp) != 1)
+ if (!rc)
return "Dump buffered data error";
- len += sizeof(msg);
- if (fp && fwrite("OK", msg.len, 1, fp) != 1)
- return "Dump buffered data error";
- len += msg.len;
+ len += rc;
}
if (sc)
--
2.17.1
next prev parent reply other threads:[~2021-06-16 14:43 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-16 14:43 [PATCH 00/10] tools/xenstored: Bug fixes + Improve Live-Update Julien Grall
2021-06-16 14:43 ` [PATCH 01/10] MAINTAINERS: Add myself as reviewers for tools/xenstore Julien Grall
2021-06-16 15:15 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 02/10] tools/xenstored: Introduce lu_get_connection() and use it Julien Grall
2021-06-21 8:21 ` Luca Fancellu
2021-06-24 7:21 ` Juergen Gross
2021-06-16 14:43 ` Julien Grall [this message]
2021-06-21 8:55 ` [PATCH 03/10] tools/xenstore: Don't assume conn->in points to the LU request Luca Fancellu
2021-06-24 8:06 ` Julien Grall
2021-06-24 7:32 ` Juergen Gross
2021-06-24 7:34 ` Juergen Gross
2021-06-24 7:56 ` Luca Fancellu
2021-06-24 8:05 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 04/10] tools/xenstored: Limit the number of requests a connection can delay Julien Grall
2021-06-21 9:02 ` Luca Fancellu
2021-06-24 7:35 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 05/10] tools/xenstored: xenstored_core.h should include fcntl.h Julien Grall
2021-06-21 9:03 ` Luca Fancellu
2021-06-24 7:36 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 06/10] tools/xenstored: Introduce a wrapper for conn->funcs->can_{read, write} Julien Grall
2021-06-21 9:10 ` Luca Fancellu
2021-06-24 7:39 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 07/10] tools/xenstored: delay_request: don't assume conn->in == in Julien Grall
2021-06-21 9:12 ` Luca Fancellu
2021-06-24 7:44 ` Juergen Gross
2021-06-24 7:58 ` Julien Grall
2021-06-16 14:43 ` [PATCH 08/10] tools/xenstored: Extend restore code to handle multiple input buffer Julien Grall
2021-06-21 9:21 ` Luca Fancellu
2021-06-24 8:30 ` Juergen Gross
2021-06-24 8:42 ` Julien Grall
2021-06-24 9:20 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 09/10] tools/xenstored: Dump delayed requests Julien Grall
2021-06-21 9:27 ` Luca Fancellu
2021-06-24 8:41 ` Juergen Gross
2021-06-24 10:28 ` Julien Grall
2021-06-24 10:45 ` Juergen Gross
2021-06-24 10:46 ` Julien Grall
2021-06-24 11:02 ` Juergen Gross
2021-06-24 11:17 ` Julien Grall
2021-06-16 14:43 ` [PATCH 10/10] tools/xenstored: Delay new transaction while Live-Update is pending Julien Grall
2021-06-21 9:30 ` Luca Fancellu
2021-06-24 9:23 ` Juergen Gross
2021-06-24 10:43 ` [PATCH 00/10] tools/xenstored: Bug fixes + Improve Live-Update Julien Grall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210616144324.31652-4-julien@xen.org \
--to=julien@xen.org \
--cc=doebel@amazon.de \
--cc=iwj@xenproject.org \
--cc=jgrall@amazon.com \
--cc=jgross@suse.com \
--cc=raphning@amazon.co.uk \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.