| Message ID | 20220620162704.80987-2-hreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu-img info: Show protocol-level information | expand |
Am 20.06.2022 um 18:26 hat Hanna Reitz geschrieben: > When a block driver supports obtaining format-specific information, but > that object only contains optional fields, it is possible that none of > them are present, so that dump_qobject() (called by > bdrv_image_info_specific_dump()) will not print anything. > > The callers of bdrv_image_info_specific_dump() put a header above this > information ("Format specific information:\n"), which will look strange > when there is nothing below. Modify bdrv_image_info_specific_dump() to > print this header instead of its callers, and only if there is indeed > something to be printed. > > Signed-off-by: Hanna Reitz <hreitz@redhat.com> > diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c > index 2f0d8ac25a..084ec44d3b 100644 > --- a/qemu-io-cmds.c > +++ b/qemu-io-cmds.c > @@ -1819,8 +1819,8 @@ static int info_f(BlockBackend *blk, int argc, char **argv) > return -EIO; > } > if (spec_info) { > - printf("Format specific information:\n"); > - bdrv_image_info_specific_dump(spec_info); > + bdrv_image_info_specific_dump(spec_info, > + "Format specific information:\n"); > qapi_free_ImageInfoSpecific(spec_info); > } Interesting observation here: That qemu-io uses printf() instead of qemu_printf() for the top level, but then dump_qobject() (which results in qemu_printf()) for the format specific information, means that if you use the 'qemu-io' HMP command, you'll get half of the output on stdout and the other half in the monitor. This series doesn't fix this, but the split makes a little more sense after this patch at least... Kevin
On 19.01.23 15:00, Kevin Wolf wrote: > Am 20.06.2022 um 18:26 hat Hanna Reitz geschrieben: >> When a block driver supports obtaining format-specific information, but >> that object only contains optional fields, it is possible that none of >> them are present, so that dump_qobject() (called by >> bdrv_image_info_specific_dump()) will not print anything. >> >> The callers of bdrv_image_info_specific_dump() put a header above this >> information ("Format specific information:\n"), which will look strange >> when there is nothing below. Modify bdrv_image_info_specific_dump() to >> print this header instead of its callers, and only if there is indeed >> something to be printed. >> >> Signed-off-by: Hanna Reitz <hreitz@redhat.com> >> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c >> index 2f0d8ac25a..084ec44d3b 100644 >> --- a/qemu-io-cmds.c >> +++ b/qemu-io-cmds.c >> @@ -1819,8 +1819,8 @@ static int info_f(BlockBackend *blk, int argc, char **argv) >> return -EIO; >> } >> if (spec_info) { >> - printf("Format specific information:\n"); >> - bdrv_image_info_specific_dump(spec_info); >> + bdrv_image_info_specific_dump(spec_info, >> + "Format specific information:\n"); >> qapi_free_ImageInfoSpecific(spec_info); >> } > Interesting observation here: That qemu-io uses printf() instead of > qemu_printf() for the top level, but then dump_qobject() (which results > in qemu_printf()) for the format specific information, means that if you > use the 'qemu-io' HMP command, you'll get half of the output on stdout > and the other half in the monitor. Hu. I can’t find a single instance of qemu_printf() in qemu-io-cmds.c, but then I assume all printf()s there should really be qemu_printf()? Hanna > This series doesn't fix this, but the split makes a little more sense > after this patch at least... > > Kevin
Am 20.01.2023 um 14:35 hat Hanna Czenczek geschrieben: > On 19.01.23 15:00, Kevin Wolf wrote: > > Am 20.06.2022 um 18:26 hat Hanna Reitz geschrieben: > > > When a block driver supports obtaining format-specific information, but > > > that object only contains optional fields, it is possible that none of > > > them are present, so that dump_qobject() (called by > > > bdrv_image_info_specific_dump()) will not print anything. > > > > > > The callers of bdrv_image_info_specific_dump() put a header above this > > > information ("Format specific information:\n"), which will look strange > > > when there is nothing below. Modify bdrv_image_info_specific_dump() to > > > print this header instead of its callers, and only if there is indeed > > > something to be printed. > > > > > > Signed-off-by: Hanna Reitz <hreitz@redhat.com> > > > diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c > > > index 2f0d8ac25a..084ec44d3b 100644 > > > --- a/qemu-io-cmds.c > > > +++ b/qemu-io-cmds.c > > > @@ -1819,8 +1819,8 @@ static int info_f(BlockBackend *blk, int argc, char **argv) > > > return -EIO; > > > } > > > if (spec_info) { > > > - printf("Format specific information:\n"); > > > - bdrv_image_info_specific_dump(spec_info); > > > + bdrv_image_info_specific_dump(spec_info, > > > + "Format specific information:\n"); > > > qapi_free_ImageInfoSpecific(spec_info); > > > } > > Interesting observation here: That qemu-io uses printf() instead of > > qemu_printf() for the top level, but then dump_qobject() (which results > > in qemu_printf()) for the format specific information, means that if you > > use the 'qemu-io' HMP command, you'll get half of the output on stdout > > and the other half in the monitor. > > Hu. I can’t find a single instance of qemu_printf() in qemu-io-cmds.c, but > then I assume all printf()s there should really be qemu_printf()? That would probably be the most consistent way. I expect it would change the output of some qemu-iotests cases, but we can explicitly print whatever was returned in QMP to keep the same information in the test output. Kevin
diff --git a/include/block/qapi.h b/include/block/qapi.h index 22c7807c89..c09859ea78 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -40,6 +40,7 @@ void bdrv_query_image_info(BlockDriverState *bs, Error **errp); void bdrv_snapshot_dump(QEMUSnapshotInfo *sn); -void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec); +void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec, + const char *prefix); void bdrv_image_info_dump(ImageInfo *info); #endif diff --git a/block/qapi.c b/block/qapi.c index cf557e3aea..51202b470a 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -777,7 +777,35 @@ static void dump_qdict(int indentation, QDict *dict) } } -void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec) +/* + * Return whether dumping the given QObject with dump_qobject() would + * yield an empty dump, i.e. not print anything. + */ +static bool qobject_is_empty_dump(const QObject *obj) +{ + switch (qobject_type(obj)) { + case QTYPE_QNUM: + case QTYPE_QSTRING: + case QTYPE_QBOOL: + return false; + + case QTYPE_QDICT: + return qdict_size(qobject_to(QDict, obj)) == 0; + + case QTYPE_QLIST: + return qlist_empty(qobject_to(QList, obj)); + + default: + abort(); + } +} + +/** + * Dumps the given ImageInfoSpecific object in a human-readable form, + * prepending an optional prefix if the dump is not empty. + */ +void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec, + const char *prefix) { QObject *obj, *data; Visitor *v = qobject_output_visitor_new(&obj); @@ -785,7 +813,12 @@ void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec) visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort); visit_complete(v, &obj); data = qdict_get(qobject_to(QDict, obj), "data"); - dump_qobject(1, data); + if (!qobject_is_empty_dump(data)) { + if (prefix) { + qemu_printf("%s", prefix); + } + dump_qobject(1, data); + } qobject_unref(obj); visit_free(v); } @@ -866,7 +899,7 @@ void bdrv_image_info_dump(ImageInfo *info) } if (info->has_format_specific) { - qemu_printf("Format specific information:\n"); - bdrv_image_info_specific_dump(info->format_specific); + bdrv_image_info_specific_dump(info->format_specific, + "Format specific information:\n"); } } diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 2f0d8ac25a..084ec44d3b 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -1819,8 +1819,8 @@ static int info_f(BlockBackend *blk, int argc, char **argv) return -EIO; } if (spec_info) { - printf("Format specific information:\n"); - bdrv_image_info_specific_dump(spec_info); + bdrv_image_info_specific_dump(spec_info, + "Format specific information:\n"); qapi_free_ImageInfoSpecific(spec_info); }
When a block driver supports obtaining format-specific information, but that object only contains optional fields, it is possible that none of them are present, so that dump_qobject() (called by bdrv_image_info_specific_dump()) will not print anything. The callers of bdrv_image_info_specific_dump() put a header above this information ("Format specific information:\n"), which will look strange when there is nothing below. Modify bdrv_image_info_specific_dump() to print this header instead of its callers, and only if there is indeed something to be printed. Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/qapi.h | 3 ++- block/qapi.c | 41 +++++++++++++++++++++++++++++++++++++---- qemu-io-cmds.c | 4 ++-- 3 files changed, 41 insertions(+), 7 deletions(-)