On Tue, Sep 14, 2021 at 15:56:44 +0800, zhanglei wrote:
Please describe your changes in more detail.
> Signed-off-by: zhanglei <zhanglei@xxxxxxxxxx>
> ---
> src/qemu/qemu_agent.c | 9 +++--
> src/qemu/qemu_agent.h | 3 +-
> src/qemu/qemu_driver.c | 89 +++++++++++++++++++++++++++++++++++++++++-
> tests/qemuagenttest.c | 2 +-
> 4 files changed, 96 insertions(+), 7 deletions(-)
[...]
> diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
> index 81b45b8e5d..94eab9de9f 100644
> --- a/src/qemu/qemu_agent.h
> +++ b/src/qemu/qemu_agent.h
> @@ -151,7 +151,8 @@ int qemuAgentSetTime(qemuAgent *mon,
> bool sync);
>
> int qemuAgentGetInterfaces(qemuAgent *mon,
> - virDomainInterfacePtr **ifaces);
> + virDomainInterfacePtr **ifaces,
> + bool report_unsupported);
The addition of 'report_unsupported' and the refactor to existing code
to pass it can be split into a separate commit to reduce noise.
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index dfc27572c4..ec5bf0a451 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
[...]
> +static void
> +virDomainInterfaceFormatParams(virDomainInterfacePtr *ifaces,
> +int nifaces,
> +virTypedParameterPtr *params,
> +int *nparams, int * maxparams)
The function header is totally malformated. There are also multiple
other instances of broken alignment below. I'll mark them with '***'
> +{
> + size_t i, j;
One variable declaration per line please;
> + const char *type = NULL;
> +
> + if (virTypedParamsAddUInt(params, nparams, maxparams,
> + "if.count", nifaces) < 0)
> + return;
Checking the return code and not reporting error feels a bit weird, but
I guess you've got the inspiration from the existing code which does the
same, so it's okay for now.
> +
> + for (i = 0; i < nifaces; i++) {
> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.name", i);
> + if (virTypedParamsAddString(params, nparams, maxparams,
> + param_name, ifaces[i]->name) < 0)
> + return;
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.hwaddr", i);
> + if (virTypedParamsAddString(params, nparams, maxparams,
> + param_name, ifaces[i]->hwaddr) < 0)
> + return;
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.count", i);
> + if (virTypedParamsAddUInt(params, nparams, maxparams,
> + param_name, ifaces[i]->naddrs) < 0)
***
> + return;
> +
> + for (j = 0; j < ifaces[i]->naddrs; j++) {
> + switch (ifaces[i]->addrs[j].type) {
> + case VIR_IP_ADDR_TYPE_IPV4:
***
> + type = "ipv4";
> + break;
> + case VIR_IP_ADDR_TYPE_IPV6:
> + type = "ipv6";
> + break;
> + }
***
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.%zu.type", i, j);
***
> + if (virTypedParamsAddString(params, nparams, maxparams,
> + param_name, type) < 0)
***
> + return;
***
And many more below. I give up, this function has totally broken
formatting.
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.%zu.addr", i, j);
> + if (virTypedParamsAddString(params, nparams, maxparams,
> + param_name, ifaces[i]->addrs[j].addr) < 0)
> + return;
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.%zu.prefix", i, j);
> + if (virTypedParamsAddUInt(params, nparams, maxparams,
> + param_name, ifaces[i]->addrs[j].prefix) < 0)
> + return;
> + }
> + }
> +}
