12:11 p.m.
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Tuesday, September 9, 2014 1:42:15 PM
Subject: Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
> + * 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?
I used long long because of this:
struct _virDomainInterfaceStats {
long long rx_bytes;
long long rx_packets;
long long rx_errs;
long long rx_drop;
long long tx_bytes;
long long tx_packets;
long long tx_errs;
long long tx_drop;
};
But I don't have any problem to cast them as unsigned, with something like:
#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.%u.%s", num, name); \
if (virTypedParamsAddULLong(&(record)->params, \
&(record)->nparams, \
maxparams, \
param_name, \
(unsigned long long)value) < 0) \
return -1; \
} while (0)
> + *
> * 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
Yep but the cycle counter is size_t and then...
$ git diff
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9d53883..e90a8c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17487,7 +17487,7 @@ do { \
do { \
char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
- "net.%lu.%s", num, name); \
+ "net.%u.%s", num, name); \
if (virTypedParamsAddLLong(&(record)->params, \
&(record)->nparams, \
maxparams, \
$ make
[...]
make[1]: Entering directory `/home/fromani/Projects/libvirt/src'
CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo
qemu/qemu_driver.c: In function 'qemuDomainGetStatsInterface':
qemu/qemu_driver.c:17521:9: error: format '%u' expects argument of type
'unsigned int', but argument 4 has type 'size_t' [-Werror=format=]
QEMU_ADD_NET_PARAM(record, maxparams, i,
^
qemu/qemu_driver.c:17521:9: error: format '%u' expects argument of type
'unsigned int', but argument 4 has type 'size_t' [-Werror=format=]
$ gcc --version
gcc (GCC) 4.8.3 20140624 (Red Hat 4.8.3-1)
same story with '%d'. Keeping '%lu' for this reason.
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani