Skip to content

Commit

Permalink
string-output-visitor: Fix (pseudo) struct handling
Browse files Browse the repository at this point in the history
Commit ff32bb5 tried to get minimal struct support into the string
output visitor by just making it return "<omitted>". Unfortunately, it
forgot that the caller will still make more visitor calls for the
content of the struct.

If the struct is contained in a list, such as IOThreadVirtQueueMapping,
in the better case its fields show up as separate list entries. In the
worse case, it contains another list, and the string output visitor
doesn't support nested lists and asserts that this doesn't happen. So as
soon as the optional "vqs" field in IOThreadVirtQueueMapping is
specified, we get a crash.

This can be reproduced with the following command line:

  echo "info qtree" | ./qemu-system-x86_64 \
    -object iothread,id=t0 \
    -blockdev null-co,node-name=disk \
    -device '{"driver": "virtio-blk-pci", "drive": "disk",
              "iothread-vq-mapping": [{"iothread": "t0", "vqs": [0]}]}' \
    -monitor stdio

Fix the problem by counting the nesting level of structs and ignoring
any visitor calls for values (apart from start/end_struct) while we're
not on the top level.

Lists nested directly within lists remain unimplemented, as we don't
currently have a use case for them.

Fixes: ff32bb5
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2069
Reported-by: Aihua Liang <aliang@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20240109181717.42493-1-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
kevmw committed Jan 26, 2024
1 parent a9c8ea9 commit 014b99a
Showing 1 changed file with 46 additions and 0 deletions.
46 changes: 46 additions & 0 deletions qapi/string-output-visitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ struct StringOutputVisitor
} range_start, range_end;
GList *ranges;
void *list; /* Only needed for sanity checking the caller */
unsigned int struct_nesting;
};

static StringOutputVisitor *to_sov(Visitor *v)
Expand Down Expand Up @@ -144,6 +145,10 @@ static bool print_type_int64(Visitor *v, const char *name, int64_t *obj,
StringOutputVisitor *sov = to_sov(v);
GList *l;

if (sov->struct_nesting) {
return true;
}

switch (sov->list_mode) {
case LM_NONE:
string_output_append(sov, *obj);
Expand Down Expand Up @@ -231,6 +236,10 @@ static bool print_type_size(Visitor *v, const char *name, uint64_t *obj,
uint64_t val;
char *out, *psize;

if (sov->struct_nesting) {
return true;
}

if (!sov->human) {
out = g_strdup_printf("%"PRIu64, *obj);
string_output_set(sov, out);
Expand All @@ -250,6 +259,11 @@ static bool print_type_bool(Visitor *v, const char *name, bool *obj,
Error **errp)
{
StringOutputVisitor *sov = to_sov(v);

if (sov->struct_nesting) {
return true;
}

string_output_set(sov, g_strdup(*obj ? "true" : "false"));
return true;
}
Expand All @@ -260,6 +274,10 @@ static bool print_type_str(Visitor *v, const char *name, char **obj,
StringOutputVisitor *sov = to_sov(v);
char *out;

if (sov->struct_nesting) {
return true;
}

if (sov->human) {
out = *obj ? g_strdup_printf("\"%s\"", *obj) : g_strdup("<null>");
} else {
Expand All @@ -273,6 +291,11 @@ static bool print_type_number(Visitor *v, const char *name, double *obj,
Error **errp)
{
StringOutputVisitor *sov = to_sov(v);

if (sov->struct_nesting) {
return true;
}

string_output_set(sov, g_strdup_printf("%.17g", *obj));
return true;
}
Expand All @@ -283,6 +306,10 @@ static bool print_type_null(Visitor *v, const char *name, QNull **obj,
StringOutputVisitor *sov = to_sov(v);
char *out;

if (sov->struct_nesting) {
return true;
}

if (sov->human) {
out = g_strdup("<null>");
} else {
Expand All @@ -295,13 +322,20 @@ static bool print_type_null(Visitor *v, const char *name, QNull **obj,
static bool start_struct(Visitor *v, const char *name, void **obj,
size_t size, Error **errp)
{
StringOutputVisitor *sov = to_sov(v);

sov->struct_nesting++;
return true;
}

static void end_struct(Visitor *v, void **obj)
{
StringOutputVisitor *sov = to_sov(v);

if (--sov->struct_nesting) {
return;
}

/* TODO actually print struct fields */
string_output_set(sov, g_strdup("<omitted>"));
}
Expand All @@ -312,6 +346,10 @@ start_list(Visitor *v, const char *name, GenericList **list, size_t size,
{
StringOutputVisitor *sov = to_sov(v);

if (sov->struct_nesting) {
return true;
}

/* we can't traverse a list in a list */
assert(sov->list_mode == LM_NONE);
/* We don't support visits without a list */
Expand All @@ -329,6 +367,10 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
StringOutputVisitor *sov = to_sov(v);
GenericList *ret = tail->next;

if (sov->struct_nesting) {
return ret;
}

if (ret && !ret->next) {
sov->list_mode = LM_END;
}
Expand All @@ -339,6 +381,10 @@ static void end_list(Visitor *v, void **obj)
{
StringOutputVisitor *sov = to_sov(v);

if (sov->struct_nesting) {
return;
}

assert(sov->list == obj);
assert(sov->list_mode == LM_STARTED ||
sov->list_mode == LM_END ||
Expand Down

0 comments on commit 014b99a

Please sign in to comment.