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>
---
V2:
- move to start of series
---
tools/xenstored/lu.c | 95 +++++++++++++++++++++++++++-----------------
1 file changed, 59 insertions(+), 36 deletions(-)
diff --git a/tools/xenstored/lu.c b/tools/xenstored/lu.c
index 77e0d377c5..f2c8b92d07 100644
--- a/tools/xenstored/lu.c
+++ b/tools/xenstored/lu.c
@@ -27,9 +27,11 @@ struct live_update *lu_status;
struct lu_dump_state {
void *buf;
+ unsigned int buf_size;
unsigned int size;
- int fd;
+ unsigned int offset;
char *filename;
+ FILE *fp;
};
static int lu_destroy(void *data)
@@ -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,82 +94,107 @@ 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;
unsigned int version;
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;
version = be32toh(pre->version);
if (memcmp(pre->ident, XS_STATE_IDENT, sizeof(pre->ident)) ||
!version || version > 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;
case XS_STATE_TYPE_DOMAIN:
- read_state_domain(ctx, head + 1, version);
+ read_state_domain(ctx, state.buf, version);
break;
default:
xprintf("live-update: unknown state record %08x\n",
- head->type);
+ head.type);
break;
}
}
@@ -271,11 +299,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.0On 29/07/2025 12:01 pm, Juergen Gross wrote:
> diff --git a/tools/xenstored/lu.c b/tools/xenstored/lu.c
> index 77e0d377c5..f2c8b92d07 100644
> --- a/tools/xenstored/lu.c
> +++ b/tools/xenstored/lu.c
> @@ -27,9 +27,11 @@ struct live_update *lu_status;
>
> struct lu_dump_state {
> void *buf;
> + unsigned int buf_size;
> unsigned int size;
> - int fd;
> + unsigned int offset;
> char *filename;
> + FILE *fp;
I know there's already one unsigned int size here, but life is too short
to not use size_t from the get-go.
~Andrew
On 29/07/2025 8:21 pm, Andrew Cooper wrote:
> On 29/07/2025 12:01 pm, Juergen Gross wrote:
>> diff --git a/tools/xenstored/lu.c b/tools/xenstored/lu.c
>> index 77e0d377c5..f2c8b92d07 100644
>> --- a/tools/xenstored/lu.c
>> +++ b/tools/xenstored/lu.c
>> @@ -27,9 +27,11 @@ struct live_update *lu_status;
>>
>> struct lu_dump_state {
>> void *buf;
>> + unsigned int buf_size;
>> unsigned int size;
>> - int fd;
>> + unsigned int offset;
>> char *filename;
>> + FILE *fp;
> I know there's already one unsigned int size here, but life is too short
> to not use size_t from the get-go.
That said, offset really needs to be an offs64_t if you want 32bit
guests to 9P safely on a modern filesystem.
~Andrew
On 29.07.25 21:25, Andrew Cooper wrote:
> On 29/07/2025 8:21 pm, Andrew Cooper wrote:
>> On 29/07/2025 12:01 pm, Juergen Gross wrote:
>>> diff --git a/tools/xenstored/lu.c b/tools/xenstored/lu.c
>>> index 77e0d377c5..f2c8b92d07 100644
>>> --- a/tools/xenstored/lu.c
>>> +++ b/tools/xenstored/lu.c
>>> @@ -27,9 +27,11 @@ struct live_update *lu_status;
>>>
>>> struct lu_dump_state {
>>> void *buf;
>>> + unsigned int buf_size;
>>> unsigned int size;
>>> - int fd;
>>> + unsigned int offset;
>>> char *filename;
>>> + FILE *fp;
>> I know there's already one unsigned int size here, but life is too short
>> to not use size_t from the get-go.
>
> That said, offset really needs to be an offs64_t if you want 32bit
> guests to 9P safely on a modern filesystem.
For 32bit guests I don't see a problem here, as this would mean the Xenstore
state would exceed 4GB, which seems unlikely using an in-memory data base in
a system supporting less than 2GB of memory (the Mini-OS limit for 32bit is
1GB, while a 32bit daemon can use max. 2GB of virtual memory).
Using unsigned int is a potential problem in 64bit mode, though. So switching
to size_t for size and off64_t for offset seems sensible.
Juergen
On 30.07.25 08:18, Jürgen Groß wrote:
> On 29.07.25 21:25, Andrew Cooper wrote:
>> On 29/07/2025 8:21 pm, Andrew Cooper wrote:
>>> On 29/07/2025 12:01 pm, Juergen Gross wrote:
>>>> diff --git a/tools/xenstored/lu.c b/tools/xenstored/lu.c
>>>> index 77e0d377c5..f2c8b92d07 100644
>>>> --- a/tools/xenstored/lu.c
>>>> +++ b/tools/xenstored/lu.c
>>>> @@ -27,9 +27,11 @@ struct live_update *lu_status;
>>>> struct lu_dump_state {
>>>> void *buf;
>>>> + unsigned int buf_size;
>>>> unsigned int size;
>>>> - int fd;
>>>> + unsigned int offset;
>>>> char *filename;
>>>> + FILE *fp;
>>> I know there's already one unsigned int size here, but life is too short
>>> to not use size_t from the get-go.
>>
>> That said, offset really needs to be an offs64_t if you want 32bit
>> guests to 9P safely on a modern filesystem.
>
> For 32bit guests I don't see a problem here, as this would mean the Xenstore
> state would exceed 4GB, which seems unlikely using an in-memory data base in
> a system supporting less than 2GB of memory (the Mini-OS limit for 32bit is
> 1GB, while a 32bit daemon can use max. 2GB of virtual memory).
>
> Using unsigned int is a potential problem in 64bit mode, though. So switching
> to size_t for size and off64_t for offset seems sensible.
Meh, off64_t seems to be unknown in Mini-OS environment. Will use size_t for
offset, too.
Juergen
© 2016 - 2025 Red Hat, Inc.