[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from bloc
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/ |
Date: |
Fri, 9 Jun 2017 16:34:08 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 2017-06-08 19:43, Markus Armbruster wrote:
> Marc-André Lureau <address@hidden> writes:
>
>> The dump functions could be generally useful for any qobject user or for
>> debugging etc.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>> include/qapi/qmp/qdict.h | 2 ++
>> include/qapi/qmp/qlist.h | 2 ++
>> include/qapi/qmp/qobject.h | 7 ++++
>> block/qapi.c | 90
>> +++-------------------------------------------
>> qobject/qdict.c | 30 ++++++++++++++++
>> qobject/qlist.c | 23 ++++++++++++
>> qobject/qobject.c | 19 ++++++++++
>> tests/check-qjson.c | 19 ++++++++++
>> 8 files changed, 106 insertions(+), 86 deletions(-)
>>
>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
>> index 8c7c2b762b..1ef3bc8cda 100644
>> --- a/include/qapi/qmp/qdict.h
>> +++ b/include/qapi/qmp/qdict.h
>> @@ -91,4 +91,6 @@ QObject *qdict_crumple(const QDict *src, Error **errp);
>>
>> void qdict_join(QDict *dest, QDict *src, bool overwrite);
>>
>> +char *qdict_to_string(QDict *dict, int indent);
>> +
>> #endif /* QDICT_H */
>> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
>> index c4b5fdad9b..c93ac3e15b 100644
>> --- a/include/qapi/qmp/qlist.h
>> +++ b/include/qapi/qmp/qlist.h
>> @@ -60,6 +60,8 @@ size_t qlist_size(const QList *qlist);
>> QList *qobject_to_qlist(const QObject *obj);
>> void qlist_destroy_obj(QObject *obj);
>>
>> +char *qlist_to_string(QList *list, int indent);
>> +
>> static inline const QListEntry *qlist_first(const QList *qlist)
>> {
>> return QTAILQ_FIRST(&qlist->head);
>> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
>> index b8ddbca405..0d6ae5048a 100644
>> --- a/include/qapi/qmp/qobject.h
>> +++ b/include/qapi/qmp/qobject.h
>> @@ -101,4 +101,11 @@ static inline QObject *qnull(void)
>> return &qnull_;
>> }
>>
>> +char *qobject_to_string_indent(QObject *obj, int indent);
>> +
>> +static inline char *qobject_to_string(QObject *obj)
>> +{
>> + return qobject_to_string_indent(obj, 0);
>> +}
>> +
>> #endif /* QOBJECT_H */
>> diff --git a/block/qapi.c b/block/qapi.c
>> index 2050df29e4..9b7d42e50a 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -586,101 +586,19 @@ void bdrv_snapshot_dump(fprintf_function
>> func_fprintf, void *f,
>> }
>> }
>>
>> -static void dump_qdict(fprintf_function func_fprintf, void *f, int
>> indentation,
>> - QDict *dict);
>> -static void dump_qlist(fprintf_function func_fprintf, void *f, int
>> indentation,
>> - QList *list);
>> -
>> -static void dump_qobject(fprintf_function func_fprintf, void *f,
>> - int comp_indent, QObject *obj)
>> -{
>> - switch (qobject_type(obj)) {
>> - case QTYPE_QNUM: {
>> - QNum *value = qobject_to_qnum(obj);
>> - char *tmp = qnum_to_string(value);
>> - func_fprintf(f, "%s", tmp);
>> - g_free(tmp);
>> - break;
>> - }
>> - case QTYPE_QSTRING: {
>> - QString *value = qobject_to_qstring(obj);
>> - func_fprintf(f, "%s", qstring_get_str(value));
>> - break;
>> - }
>> - case QTYPE_QDICT: {
>> - QDict *value = qobject_to_qdict(obj);
>> - dump_qdict(func_fprintf, f, comp_indent, value);
>> - break;
>> - }
>> - case QTYPE_QLIST: {
>> - QList *value = qobject_to_qlist(obj);
>> - dump_qlist(func_fprintf, f, comp_indent, value);
>> - break;
>> - }
>> - case QTYPE_QBOOL: {
>> - QBool *value = qobject_to_qbool(obj);
>> - func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
>> - break;
>> - }
>> - default:
>> - abort();
>> - }
>> -}
>> -
>> -static void dump_qlist(fprintf_function func_fprintf, void *f, int
>> indentation,
>> - QList *list)
>> -{
>> - const QListEntry *entry;
>> - int i = 0;
>> -
>> - for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
>> - QType type = qobject_type(entry->value);
>> - bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
>> - func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i,
>> - composite ? '\n' : ' ');
>> - dump_qobject(func_fprintf, f, indentation + 1, entry->value);
>> - if (!composite) {
>> - func_fprintf(f, "\n");
>> - }
>> - }
>> -}
>> -
>> -static void dump_qdict(fprintf_function func_fprintf, void *f, int
>> indentation,
>> - QDict *dict)
>> -{
>> - const QDictEntry *entry;
>> -
>> - for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry))
>> {
>> - QType type = qobject_type(entry->value);
>> - bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
>> - char *key = g_malloc(strlen(entry->key) + 1);
>> - int i;
>> -
>> - /* replace dashes with spaces in key (variable) names */
>> - for (i = 0; entry->key[i]; i++) {
>> - key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
>> - }
>> - key[i] = 0;
>> - func_fprintf(f, "%*s%s:%c", indentation * 4, "", key,
>> - composite ? '\n' : ' ');
>> - dump_qobject(func_fprintf, f, indentation + 1, entry->value);
>> - if (!composite) {
>> - func_fprintf(f, "\n");
>> - }
>> - g_free(key);
>> - }
>> -}
>> -
>> void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
>> ImageInfoSpecific *info_spec)
>> {
>> QObject *obj, *data;
>> Visitor *v = qobject_output_visitor_new(&obj);
>> + char *tmp;
>>
>> visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort);
>> visit_complete(v, &obj);
>> data = qdict_get(qobject_to_qdict(obj), "data");
>> - dump_qobject(func_fprintf, f, 1, data);
>> + tmp = qobject_to_string_indent(data, 1);
>> + func_fprintf(f, "%s", tmp);
>> + g_free(tmp);
>> qobject_decref(obj);
>> visit_free(v);
>> }
>
> The title claims "move dump_qobject() from block/ to qobject/", but
> that's not what the patch does. It *replaces* dump_qobject() by
> qobject_to_string(). The former dumps to a callback, the latter to a
> dynamic string buffer.
>
> Providing dump functionality in one way doesn't preclude the other way:
> given callback, one could define a callback that builds up a string
> buffer, and given buffer, one could (and you actually do) pass the
> buffer to a callback. That's less efficient, though.
>
> Trading efficiency for ease-of-use should be okay here, but I'm cc'ing
> Max and Kevin to double-check.
Given that this function is called only by HMP's "info block -v" and
qemu-img, I don't think efficiency is too important. In the former case,
it's your own fault for using HMP, in the latter you'll have only a
single image anyway.
So I'm OK with this change.
Max
>
> Two ways forward:
>
> 1. Get Max / Kevin's blessing, change the commit message to match the
> code.
>
> 2. Change the code to match the commit message.
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v3 41/43] tests/qdict: check more get_try_int() cases, (continued)
- [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/, Marc-André Lureau, 2017/06/07
- Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/, Markus Armbruster, 2017/06/08
- Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/, Marc-André Lureau, 2017/06/09
- Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/, Marc-André Lureau, 2017/06/09
- Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/, Markus Armbruster, 2017/06/09
- Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/, Eric Blake, 2017/06/09
- Re: [Qemu-devel] [PATCH v3 43/43] qobject: move dump_qobject() from block/ to qobject/,
Max Reitz <=
Re: [Qemu-devel] [PATCH v3 00/43] qobject/qapi: add uint type, no-reply, 2017/06/07
Re: [Qemu-devel] [PATCH v3 00/43] qobject/qapi: add uint type, no-reply, 2017/06/07
Re: [Qemu-devel] [PATCH v3 00/43] qobject/qapi: add uint type, Markus Armbruster, 2017/06/09