----- Original Message -----
> From: "Peter Krempa" <pkrempa@xxxxxxxxxx>
> To: "Francesco Romani" <fromani@xxxxxxxxxx>, libvir-list@xxxxxxxxxx
> Sent: Tuesday, September 9, 2014 1:42:15 PM
> Subject: Re: [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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list