[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group



On 09/08/14 15:05, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_INTERFACE
> group of statistics.
> 
> Signed-off-by: Francesco Romani <fromani redhat com>
> ---
>  include/libvirt/libvirt.h.in |  1 +
>  src/libvirt.c                | 14 +++++++
>  src/qemu/qemu_driver.c       | 87 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 68573a0..93aa1fb 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2514,6 +2514,7 @@ typedef enum {
>      VIR_DOMAIN_STATS_CPU_TOTAL = (1 << 1), /* return domain CPU info */
>      VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */
>      VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
> +    VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */
>  } virDomainStatsTypes;
>  
>  typedef enum {
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 0326847..8aa6cb1 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21582,6 +21582,20 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>   *                     as unsigned long long.
>   * "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU <num> as int.
>   *
> + * VIR_DOMAIN_STATS_INTERFACE: Return network interface statistics.
> + * The typed parameter keys are in this format:
> + * "net.count" - number of network interfaces on this domain
> + *               as unsigned int.
> + * "net.<num>.name" - name of the interface <num> as string.
> + * "net.<num>.rx.bytes" - bytes received as long long.
> + * "net.<num>.rx.pkts" - packets received as long long.
> + * "net.<num>.rx.errs" - receive errors as long long.
> + * "net.<num>.rx.drop" - receive packets dropped as long long.
> + * "net.<num>.tx.bytes" - bytes transmitted as long long.
> + * "net.<num>.tx.pkts" - packets transmitted as long long.
> + * "net.<num>.tx.errs" - transmission errors as long long.
> + * "net.<num>.tx.drop" - transmit packets dropped as long long.

Why are all of those represented as long long instead of unsigned long
long? I don't see how these could be negative. If we need to express
that the value is unsupported we can just drop it from here and not
waste half of the range here.

Any other opinions on this?

> + *
>   * Using 0 for @stats returns all stats groups supported by the given
>   * hypervisor.
>   *
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6bcbfb5..989eb3e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17537,6 +17537,92 @@ qemuDomainGetStatsVcpu(virConnectPtr conn ATTRIBUTE_UNUSED,
>      return ret;
>  }
>  
> +#define QEMU_ADD_COUNT_PARAM(record, maxparams, type, count) \
> +do { \
> +    char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
> +    snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "%s.count", type); \
> +    if (virTypedParamsAddUInt(&(record)->params, \
> +                              &(record)->nparams, \
> +                              maxparams, \
> +                              param_name, \
> +                              count) < 0) \
> +        return -1; \
> +} while (0)
> +
> +#define QEMU_ADD_NAME_PARAM(record, maxparams, type, num, name) \
> +do { \
> +    char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
> +    snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
> +             "%s.%lu.name", type, num); \
> +    if (virTypedParamsAddString(&(record)->params, \
> +                                &(record)->nparams, \
> +                                maxparams, \
> +                                param_name, \
> +                                name) < 0) \
> +        return -1; \
> +} while (0)
> +
> +#define QEMU_ADD_NET_PARAM(record, maxparams, num, name, value) \
> +do { \
> +    char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
> +    snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
> +             "net.%lu.%s", num, name); \

%lu? the count is unsigned int so you should be fine with %d

> +    if (virTypedParamsAddLLong(&(record)->params, \
> +                               &(record)->nparams, \
> +                               maxparams, \
> +                               param_name, \
> +                               value) < 0) \
> +        return -1; \
> +} while (0)
> +
> +static int
> +qemuDomainGetStatsInterface(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                            virDomainObjPtr dom,
> +                            virDomainStatsRecordPtr record,
> +                            int *maxparams,
> +                            unsigned int privflags ATTRIBUTE_UNUSED)
> +{
> +    size_t i;
> +    struct _virDomainInterfaceStats tmp;
> +
> +    QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets);
> +
> +    /* Check the path is one of the domain's network interfaces. */
> +    for (i = 0; i < dom->def->nnets; i++) {
> +        memset(&tmp, 0, sizeof(tmp));
> +
> +        if (virNetInterfaceStats(dom->def->nets[i]->ifname, &tmp) < 0)
> +            continue;
> +
> +        QEMU_ADD_NAME_PARAM(record, maxparams,
> +                            "net", i, dom->def->nets[i]->ifname);
> +
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "rx.bytes", tmp.rx_bytes);
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "rx.pkts", tmp.rx_packets);
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "rx.errs", tmp.rx_errs);
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "rx.drop", tmp.rx_drop);
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "tx.bytes", tmp.tx_bytes);
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "tx.pkts", tmp.tx_packets);
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "tx.errs", tmp.tx_errs);
> +        QEMU_ADD_NET_PARAM(record, maxparams, i,
> +                           "tx.drop", tmp.tx_drop);
> +    }
> +
> +    return 0;
> +}
> +
> +#undef QEMU_ADD_NET_PARAM
> +
> +#undef QEMU_ADD_NAME_PARAM
> +
> +#undef QEMU_ADD_COUNT_PARAM
>  
>  typedef int
>  (*qemuDomainGetStatsFunc)(virConnectPtr conn,
> @@ -17556,6 +17642,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = {
>      { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false },
>      { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true },
>      { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false },
> +    { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false },
>      { NULL, 0, false }
>  };
>  
> 

Peter

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]