[libvirt PATCH v2 0/4] report interface infomation in guestinfo command

zhanglei (4): domain: add interface information to 'virDomainGetGuestInfo' virsh: add interface information to guestinfo command qemu: report guest interface information in 'qemuDomainGetGuestInfo' NEWS: qemu: report guest interfaces information in 'virDomainGetGuestInfo' NEWS.rst | 5 ++ docs/manpages/virsh.rst | 12 ++++- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 13 +++++ src/qemu/qemu_agent.c | 9 ++-- src/qemu/qemu_agent.h | 3 +- src/qemu/qemu_driver.c | 89 +++++++++++++++++++++++++++++++- tests/qemuagenttest.c | 2 +- tools/virsh-domain.c | 6 +++ 9 files changed, 131 insertions(+), 9 deletions(-) -- 2.31.1

Signed-off-by: zhanglei <zhanglei@smartx.com> --- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7ef8ac51e5..b088847725 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5074,6 +5074,7 @@ typedef enum { VIR_DOMAIN_GUEST_INFO_HOSTNAME = (1 << 3), /* return hostname information */ VIR_DOMAIN_GUEST_INFO_FILESYSTEM = (1 << 4), /* return filesystem information */ VIR_DOMAIN_GUEST_INFO_DISKS = (1 << 5), /* return disks information */ + VIR_DOMAIN_GUEST_INFO_INTERFACES = (1 << 6), /* return interfaces information */ } virDomainGuestInfoTypes; int virDomainGetGuestInfo(virDomainPtr domain, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 57baef6889..b4d88f55d4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12579,6 +12579,19 @@ virDomainSetVcpu(virDomainPtr domain, * * "hostname" - the hostname of the domain * + * VIR_DOMAIN_GUEST_INFO_INTERFACES: + * Returns information about the interfaces within the domain. the typed + * parameter keys are in this format: + * + * "if.count" - the number of interfaces defined on this domain + * as an unsigned int + * "if.<num>.name" - interface name in the guest + * "if.<num>.hwaddr" - hardware address of interface + * "if.<num>.addr.count - the number of ip addresses of interface + * "if.<num>.addr.<num>.type" - the type of ip address(e.g. ipv4) + * "if.<num>.addr.<num>.addr" - the ip address + * "if.<num>.addr.<num>.prefix" - the number of bits of ip address prefix + * * Using 0 for @types returns all information groups supported by the given * hypervisor. * -- 2.31.1

On Tue, Sep 14, 2021 at 15:56:42 +0800, zhanglei wrote: We usually ask for commit message, e.g.: "The new parameter group returns information about network interfaces".
Signed-off-by: zhanglei <zhanglei@smartx.com> --- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)
[...]
int virDomainGetGuestInfo(virDomainPtr domain, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 57baef6889..b4d88f55d4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12579,6 +12579,19 @@ virDomainSetVcpu(virDomainPtr domain, * * "hostname" - the hostname of the domain * + * VIR_DOMAIN_GUEST_INFO_INTERFACES: + * Returns information about the interfaces within the domain. the typed
The second sentence should start with a capital letter.
+ * parameter keys are in this format: + * + * "if.count" - the number of interfaces defined on this domain + * as an unsigned int
Here it mentions the type of the field ...
+ * "if.<num>.name" - interface name in the guest + * "if.<num>.hwaddr" - hardware address of interface + * "if.<num>.addr.count - the number of ip addresses of interface + * "if.<num>.addr.<num>.type" - the type of ip address(e.g. ipv4) + * "if.<num>.addr.<num>.addr" - the ip address + * "if.<num>.addr.<num>.prefix" - the number of bits of ip address prefix
... but neither of these do.
+ * * Using 0 for @types returns all information groups supported by the given * hypervisor. * -- 2.31.1

Signed-off-by: zhanglei <zhanglei@smartx.com> --- docs/manpages/virsh.rst | 12 ++++++++++-- tools/virsh-domain.c | 6 ++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 9561b3f59d..dd186ab14d 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2700,7 +2700,7 @@ guestinfo :: guestinfo domain [--user] [--os] [--timezone] [--hostname] [--filesystem] - [--disk] + [--disk] [--interface] Print information about the guest from the point of view of the guest agent. Note that this command requires a guest agent to be configured and running in @@ -2711,7 +2711,7 @@ are supported by the guest agent. You can limit the types of information that are returned by specifying one or more flags. If a requested information type is not supported, the processes will provide an exit code of 1. Available information types flags are *--user*, *--os*, -*--timezone*, *--hostname*, *--filesystem* and *--disk*. +*--timezone*, *--hostname*, *--filesystem*, *--disk* and *--interface*. Note that depending on the hypervisor type and the version of the guest agent running within the domain, not all of the following information may be @@ -2779,6 +2779,14 @@ returned: * ``disk.<num>.alias`` - the device alias of the disk (e.g. sda) * ``disk.<num>.guest_alias`` - optional alias assigned to the disk +*--interface* returns: +* ``if.count`` - the number of interfaces defined on this domain +* ``if.<num>.name`` - name in the guest (e.g. ``eth0``) for interface <num> +* ``if.<num>.hwaddr`` - hardware address in the guest for interface <num> +* ``if.<num>.addr.count`` - the number of IP addresses of interface <num> +* ``if.<num>.addr.<num>.type`` - the type of IP address <num> (e.g. ipv4) +* ``if.<num>.addr.<num>.addr`` - the address of IP address <num> +* ``if.<num>.addr.<num>.prefix`` - the prefix of IP address <num> guestvcpus ---------- diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e5bd1fdd75..5ba5bd6b8c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13829,6 +13829,10 @@ static const vshCmdOptDef opts_guestinfo[] = { .type = VSH_OT_BOOL, .help = N_("report disk information"), }, + {.name = "interface", + .type = VSH_OT_BOOL, + .help = N_("report interface information"), + }, {.name = NULL} }; @@ -13854,6 +13858,8 @@ cmdGuestInfo(vshControl *ctl, const vshCmd *cmd) types |= VIR_DOMAIN_GUEST_INFO_FILESYSTEM; if (vshCommandOptBool(cmd, "disk")) types |= VIR_DOMAIN_GUEST_INFO_DISKS; + if (vshCommandOptBool(cmd, "interface")) + types |= VIR_DOMAIN_GUEST_INFO_INTERFACES; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; -- 2.31.1

On Tue, Sep 14, 2021 at 15:56:43 +0800, zhanglei wrote:
Signed-off-by: zhanglei <zhanglei@smartx.com> --- docs/manpages/virsh.rst | 12 ++++++++++-- tools/virsh-domain.c | 6 ++++++ 2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 9561b3f59d..dd186ab14d 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst
[...]
@@ -2779,6 +2779,14 @@ returned: * ``disk.<num>.alias`` - the device alias of the disk (e.g. sda) * ``disk.<num>.guest_alias`` - optional alias assigned to the disk
+*--interface* returns: +* ``if.count`` - the number of interfaces defined on this domain +* ``if.<num>.name`` - name in the guest (e.g. ``eth0``) for interface <num> +* ``if.<num>.hwaddr`` - hardware address in the guest for interface <num> +* ``if.<num>.addr.count`` - the number of IP addresses of interface <num> +* ``if.<num>.addr.<num>.type`` - the type of IP address <num> (e.g. ipv4)
This is confusing. You have two '<num>' substitutions in the filed name format example and refer to one of them in the descriptition.

Signed-off-by: zhanglei <zhanglei@smartx.com> --- 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.c b/src/qemu/qemu_agent.c index 5f421be6f6..a7f943e0dc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2245,17 +2245,20 @@ qemuAgentGetAllInterfaceAddresses(virDomainInterfacePtr **ifaces_ret, */ int qemuAgentGetInterfaces(qemuAgent *agent, - virDomainInterfacePtr **ifaces) + virDomainInterfacePtr **ifaces, + bool report_unsupported) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; virJSONValue *ret_array = NULL; + int rc; if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL))) return -1; - if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) - return -1; + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, + report_unsupported)) < 0) + return rc; if (!(ret_array = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", 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); int qemuAgentSetUserPassword(qemuAgent *mon, const char *user, 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 @@ -18950,7 +18950,7 @@ qemuDomainInterfaceAddresses(virDomainPtr dom, goto endjob; agent = qemuDomainObjEnterAgent(vm); - ret = qemuAgentGetInterfaces(agent, ifaces); + ret = qemuAgentGetInterfaces(agent, ifaces, true); qemuDomainObjExitAgent(vm, agent); endjob: @@ -19896,7 +19896,8 @@ static const unsigned int qemuDomainGetGuestInfoSupportedTypes = VIR_DOMAIN_GUEST_INFO_TIMEZONE | VIR_DOMAIN_GUEST_INFO_HOSTNAME | VIR_DOMAIN_GUEST_INFO_FILESYSTEM | - VIR_DOMAIN_GUEST_INFO_DISKS; + VIR_DOMAIN_GUEST_INFO_DISKS | + VIR_DOMAIN_GUEST_INFO_INTERFACES; static int qemuDomainGetGuestInfoCheckSupport(unsigned int types, @@ -20095,6 +20096,70 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, } } +static void +virDomainInterfaceFormatParams(virDomainInterfacePtr *ifaces, +int nifaces, +virTypedParameterPtr *params, +int *nparams, int * maxparams) +{ + size_t i, j; + const char *type = NULL; + + if (virTypedParamsAddUInt(params, nparams, maxparams, + "if.count", nifaces) < 0) + return; + + 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; + + 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; + } + } +} static int qemuDomainGetGuestInfo(virDomainPtr dom, @@ -20116,6 +20181,8 @@ qemuDomainGetGuestInfo(virDomainPtr dom, qemuAgentFSInfo **agentfsinfo = NULL; size_t ndisks = 0; qemuAgentDiskInfo **agentdiskinfo = NULL; + virDomainInterfacePtr *ifaces = NULL; + size_t nifaces = 0; size_t i; virCheckFlags(0, -1); @@ -20181,6 +20248,15 @@ qemuDomainGetGuestInfo(virDomainPtr dom, } } + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_INTERFACES) { + rc = qemuAgentGetInterfaces(agent, &ifaces, report_unsupported); + if (rc == -1) { + goto exitagent; + } else if (rc >= 0) { + nifaces = rc; + } + } + qemuDomainObjExitAgent(vm, agent); qemuDomainObjEndAgentJob(vm); @@ -20203,6 +20279,10 @@ qemuDomainGetGuestInfo(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); } + if (nifaces > 0) { + virDomainInterfaceFormatParams(ifaces, nifaces, params, nparams, &maxparams); + } + ret = 0; cleanup: @@ -20212,6 +20292,11 @@ qemuDomainGetGuestInfo(virDomainPtr dom, for (i = 0; i < ndisks; i++) qemuAgentDiskInfoFree(agentdiskinfo[i]); g_free(agentdiskinfo); + if (ifaces && nifaces > 0) { + for (i = 0; i < nifaces; i++) + virDomainInterfaceFree(ifaces[i]); + } + g_free(ifaces); virDomainObjEndAPI(&vm); return ret; diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index bef6dfd152..8fb3da7fef 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -907,7 +907,7 @@ testQemuAgentGetInterfaces(const void *data) goto cleanup; if ((ifaces_count = qemuAgentGetInterfaces(qemuMonitorTestGetAgent(test), - &ifaces)) < 0) + &ifaces, true)) < 0) goto cleanup; if (ifaces_count != 4) { -- 2.31.1

On Tue, Sep 14, 2021 at 15:56:44 +0800, zhanglei wrote: Please describe your changes in more detail.
Signed-off-by: zhanglei <zhanglei@smartx.com> --- 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; + } + } +}

Signed-off-by: zhanglei <zhanglei@smartx.com> --- NEWS.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 4521499db7..9288597f18 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -19,6 +19,11 @@ v7.8.0 (unreleased) * **Improvements** + * qemu: Report guest interfaces information in ``virDomainGetGuestInfo`` + + Libvirt is now able to report interfaces from the guest's + perspective (using guest agent). + * **Bug fixes** -- 2.31.1
participants (2)
-
Peter Krempa
-
zhanglei