On 10/28/21 17:54, Daniel P. Berrangé wrote:
> This is a counterpart to the HMP "info cmma" command. It is being
> added with an "x-" prefix because this QMP command is intended as an
> adhoc debugging tool and will thus not be modelled in QAPI as fully
> structured data, nor will it have long term guaranteed stability.
> The existing HMP command is rewritten to call the QMP command.
>
> This command is unable to use the pre-existing HumanReadableText,
> because if 'common.json' is included into 'machine-target.json'
> the static marshalling method for HumanReadableText will be reported
> as unused by the compiler on all architectures except s390x.
>
> Possible options were
>
> 1 Support 'if' conditionals on 'include' statements in QAPI
> 2 Add further commands to 'machine-target.json' that use
> HumanReadableText, such that it has at least one usage
> on all architecture targets.
> 3 Duplicate HumanReadableText as TargetHumanReadableText
> adding conditions
>
> This patch takes option (3) in the belief that we will eventually
> get to a point where option (2) happens, and TargetHumanReadableText
> can be removed again.
Aren't you using (1) here? TargetHumanReadableText was added in v2
but v3 diverged and doesn't add it anymore.
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> ---
> hw/s390x/s390-stattrib.c | 56 +++++++++++++++++++++++++++-------------
> qapi/machine-target.json | 14 ++++++++++
> 2 files changed, 52 insertions(+), 18 deletions(-)
>
> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
> index 9eda1c3b2a..d8f5867a5f 100644
> --- a/hw/s390x/s390-stattrib.c
> +++ b/hw/s390x/s390-stattrib.c
> @@ -17,6 +17,8 @@
> #include "qemu/error-report.h"
> #include "exec/ram_addr.h"
> #include "qapi/error.h"
> +#include "qapi/qapi-commands-machine-target.h"
> +#include "qapi/type-helpers.h"
> #include "qapi/qmp/qdict.h"
>
> /* 512KiB cover 2GB of guest memory */
> @@ -67,41 +69,59 @@ void hmp_migrationmode(Monitor *mon, const QDict *qdict)
> }
> }
>
> -void hmp_info_cmma(Monitor *mon, const QDict *qdict)
> +HumanReadableText *qmp_x_query_cmma(int64_t addr,
> + bool has_count,
> + int64_t count,
> + Error **errp)
> {
> + g_autoptr(GString) buf = g_string_new("");
> S390StAttribState *sas = s390_get_stattrib_device();
> S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas);
> - uint64_t addr = qdict_get_int(qdict, "addr");
> - uint64_t buflen = qdict_get_try_int(qdict, "count", 8);
> - uint8_t *vals;
> + g_autofree uint8_t *vals = NULL;
> int cx, len;
>
> - vals = g_try_malloc(buflen);
> + vals = g_try_malloc(count);
> if (!vals) {
> - monitor_printf(mon, "Error: %s\n", strerror(errno));
> - return;
> + error_setg_errno(errp, errno, "cannot allocate CMMA attribute
> values");
> + return NULL;
> }
>
> - len = sac->peek_stattr(sas, addr / TARGET_PAGE_SIZE, buflen, vals);
> + len = sac->peek_stattr(sas, addr / TARGET_PAGE_SIZE, count, vals);
> if (len < 0) {
> - monitor_printf(mon, "Error: %s", strerror(-len));
> - goto out;
> + error_setg_errno(errp, -len, "cannot peek at CMMA attribute values");
> + return NULL;
> }
>
> - monitor_printf(mon, " CMMA attributes, "
> - "pages %" PRIu64 "+%d (0x%" PRIx64 "):\n",
> - addr / TARGET_PAGE_SIZE, len, addr & ~TARGET_PAGE_MASK);
> + g_string_append_printf(buf, " CMMA attributes, "
> + "pages %" PRIu64 "+%d (0x%" PRIx64 "):\n",
> + addr / TARGET_PAGE_SIZE, len,
> + addr & ~TARGET_PAGE_MASK);
> for (cx = 0; cx < len; cx++) {
> if (cx % 8 == 7) {
> - monitor_printf(mon, "%02x\n", vals[cx]);
> + g_string_append_printf(buf, "%02x\n", vals[cx]);
> } else {
> - monitor_printf(mon, "%02x", vals[cx]);
> + g_string_append_printf(buf, "%02x", vals[cx]);
> }
> }
> - monitor_printf(mon, "\n");
> + g_string_append_printf(buf, "\n");
> +
> + return human_readable_text_from_str(buf);
> +}
> +
> +void hmp_info_cmma(Monitor *mon, const QDict *qdict)
> +{
> + Error *err = NULL;
> + g_autoptr(HumanReadableText) info = NULL;
> + uint64_t addr = qdict_get_int(qdict, "addr");
> + uint64_t count = qdict_get_try_int(qdict, "count", 8);
> +
> + info = qmp_x_query_cmma(addr, true, count, &err);
> + if (err) {
> + error_report_err(err);
> + return;
> + }
>
> -out:
> - g_free(vals);
> + monitor_printf(mon, "%s", info->human_readable_text);
> }
>
> /* Migration support: */
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 7d4e73462f..19e44987af 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -345,6 +345,20 @@
> 'TARGET_MIPS' ] } }
>
>
> +##
> +# @x-query-cmma:
> +#
> +# Query the values of the CMMA storage attributes for a range of pages
> +#
> +# Returns: CMMA storage attribute values
> +#
> +# Since: 6.2
> +##
> +{ 'command': 'x-query-cmma',
> + 'data': { 'addr': 'int', '*count': 'int' },
> + 'returns': 'HumanReadableText',
> + 'if': 'TARGET_S390X' }
> +
> ##
> # @x-query-skeys:
> #
>
