[PATCH 5/9] xenstored: use fread() instead of mmap() for reading live update state

Juergen Gross posted 9 patches 4 months ago
There is a newer version of this series
[PATCH 5/9] xenstored: use fread() instead of mmap() for reading live update state
Posted by Juergen Gross 4 months ago
Mini-OS doesn't support using mmap() for accessing a file. In order
to support reading the live update state from a 9pfs based file, use
fread() instead of mmap().

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstored/lu.c | 93 +++++++++++++++++++++++++++-----------------
 1 file changed, 58 insertions(+), 35 deletions(-)

diff --git a/tools/xenstored/lu.c b/tools/xenstored/lu.c
index 4fccbbc195..029c4fab72 100644
--- a/tools/xenstored/lu.c
+++ b/tools/xenstored/lu.c
@@ -25,9 +25,11 @@
 
 struct lu_dump_state {
 	void *buf;
+	unsigned int buf_size;
 	unsigned int size;
-	int fd;
+	unsigned int offset;
 	char *filename;
+	FILE *fp;
 };
 
 struct live_update *lu_status;
@@ -80,9 +82,10 @@ bool lu_is_pending(void)
 	return lu_status != NULL;
 }
 
-static void lu_get_dump_state(struct lu_dump_state *state)
+static void lu_get_dump_state(void *ctx, struct lu_dump_state *state)
 {
 	struct stat statbuf;
+	int fd;
 
 	state->size = 0;
 
@@ -91,77 +94,102 @@ static void lu_get_dump_state(struct lu_dump_state *state)
 	if (!state->filename)
 		barf("Allocation failure");
 
-	state->fd = open(state->filename, O_RDONLY);
-	if (state->fd < 0)
-		return;
-	if (fstat(state->fd, &statbuf) != 0)
-		goto out_close;
+	fd = open(state->filename, O_RDONLY);
+	if (fd < 0)
+		barf("No state file found");
+	if (fstat(fd, &statbuf) != 0)
+		barf("Could not fstat state file");
 	state->size = statbuf.st_size;
 
-	state->buf = mmap(NULL, state->size, PROT_READ, MAP_PRIVATE,
-			  state->fd, 0);
-	if (state->buf == MAP_FAILED) {
-		state->size = 0;
-		goto out_close;
-	}
+	/* Start with a 4k buffer. If needed we'll reallocate a larger one. */
+	state->buf_size = 4096;
+	state->buf = talloc_size(ctx, state->buf_size);
+	if (!state->buf)
+		barf("Allocation failure");
 
-	return;
+	state->fp = fdopen(fd, "r");
+}
 
- out_close:
-	close(state->fd);
+static void lu_dump_close(FILE *fp)
+{
+	fclose(fp);
 }
 
 static void lu_close_dump_state(struct lu_dump_state *state)
 {
 	assert(state->filename != NULL);
 
-	munmap(state->buf, state->size);
-	close(state->fd);
+	lu_dump_close(state->fp);
 
 	unlink(state->filename);
 	talloc_free(state->filename);
+	talloc_free(state->buf);
+}
+
+static void lu_read_data(void *ctx, struct lu_dump_state *state,
+			 unsigned int size)
+{
+	if (state->offset + size > state->size)
+		barf("Inconsistent state data");
+
+	if (size > state->buf_size) {
+		state->buf = talloc_realloc_size(ctx, state->buf, size);
+		if (!state->buf)
+			barf("Allocation failure");
+		state->buf_size = size;
+	}
+
+	if (fread(state->buf, size, 1, state->fp) != 1)
+		barf("State read error");
+
+	state->offset += size;
 }
 
 void lu_read_state(void)
 {
 	struct lu_dump_state state = {};
-	struct xs_state_record_header *head;
+	struct xs_state_record_header head;
 	void *ctx = talloc_new(NULL); /* Work context for subfunctions. */
 	struct xs_state_preamble *pre;
 
 	syslog(LOG_INFO, "live-update: read state\n");
-	lu_get_dump_state(&state);
+	lu_get_dump_state(ctx, &state);
 	if (state.size == 0)
 		barf_perror("No state found after live-update");
 
+	lu_read_data(ctx, &state, sizeof(*pre));
 	pre = state.buf;
 	if (memcmp(pre->ident, XS_STATE_IDENT, sizeof(pre->ident)) ||
 	    pre->version != htobe32(XS_STATE_VERSION) ||
 	    pre->flags != XS_STATE_FLAGS)
 		barf("Unknown record identifier");
-	for (head = state.buf + sizeof(*pre);
-	     head->type != XS_STATE_TYPE_END &&
-		(void *)head - state.buf < state.size;
-	     head = (void *)head + sizeof(*head) + head->length) {
-		switch (head->type) {
+
+	for (;;) {
+		lu_read_data(ctx, &state, sizeof(head));
+		head = *(struct xs_state_record_header *)(state.buf);
+		if (head.type == XS_STATE_TYPE_END)
+			break;
+		lu_read_data(ctx, &state, head.length);
+
+		switch (head.type) {
 		case XS_STATE_TYPE_GLOBAL:
-			read_state_global(ctx, head + 1);
+			read_state_global(ctx, state.buf);
 			break;
 		case XS_STATE_TYPE_CONN:
-			read_state_connection(ctx, head + 1);
+			read_state_connection(ctx, state.buf);
 			break;
 		case XS_STATE_TYPE_WATCH:
-			read_state_watch(ctx, head + 1);
+			read_state_watch(ctx, state.buf);
 			break;
 		case XS_STATE_TYPE_TA:
 			xprintf("live-update: ignore transaction record\n");
 			break;
 		case XS_STATE_TYPE_NODE:
-			read_state_node(ctx, head + 1);
+			read_state_node(ctx, state.buf);
 			break;
 		default:
 			xprintf("live-update: unknown state record %08x\n",
-				head->type);
+				head.type);
 			break;
 		}
 	}
@@ -266,11 +294,6 @@ static FILE *lu_dump_open(const void *ctx)
 	return fdopen(fd, "w");
 }
 
-static void lu_dump_close(FILE *fp)
-{
-	fclose(fp);
-}
-
 static const char *lu_dump_state(const void *ctx, struct connection *conn)
 {
 	FILE *fp;
-- 
2.43.0
Re: [PATCH 5/9] xenstored: use fread() instead of mmap() for reading live update state
Posted by Jason Andryuk 3 months, 2 weeks ago
On 2025-07-02 04:13, Juergen Gross wrote:
> Mini-OS doesn't support using mmap() for accessing a file. In order
> to support reading the live update state from a 9pfs based file, use
> fread() instead of mmap().
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>