Re: [libvirt] [PATCH v2 1/3] vz: add net dev statistiscs

On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote: > From: Nikolay Shirokovskiy <nshirokovskiy(a)parallels.com&gt; > > Populate counters SDK currenly supports: > rx_bytes > rx_packets > tx_bytes > tx_packets > > Comments. > > 1. Use vzDomObjFromDomainRef/virDomainObjEndAPI pair to get domain > object as we use prlsdkGetStatsParam that can release domain > object lock and thus we need a reference in case domain > is deleated meanwhile. > > 2. Introduce prlsdkGetAdapterIndex to convert from > device name to SDK device index. We need this index > as it is used in statistics data from SDK identify > network device. Unfortunately we need to enumerate > network devices to discover this mapping. > > Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; > --- > src/vz/vz_driver.c | 44 +++++++++++++++++++++++++++++++++ > src/vz/vz_sdk.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++- > src/vz/vz_sdk.h | 4 +++ > 3 files changed, 116 insertions(+), 1 deletions(-) > > diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c > index cef3c77..deac572 100644 > --- a/src/vz/vz_driver.c > +++ b/src/vz/vz_driver.c > @@ -1321,6 +1321,49 @@ vzDomainBlockStatsFlags(virDomainPtr domain, > return ret; > } > +static int > +vzDomainInterfaceStats(virDomainPtr domain, > + const char *path, > + virDomainInterfaceStatsPtr stats) > +{ > + virDomainObjPtr dom = NULL; > + int ret = -1; > + int net_index = -1; > + char *name = NULL; > + > + if (!(dom = vzDomObjFromDomainRef(domain))) > + return -1; > + > + net_index = prlsdkGetAdapterIndex(dom, path); We have function prlsdkGetNetIndex, which looks up an adapter by given libvirt structure virDomainNetDef. Net and Adapter are sysnonyms, libvirt uses Net, and prlsdk uses 'Adapter' name, so I think we should rename these function, so the name will show, by which property adapter is looked up. > + if (net_index < 0) In libvirt's code people usually combine the function call and a check in one line if ((net_index = prlsdkGetAdapterIndex(dom, path)) < 0) ... > + return -1; > + > +#define PARALLELS_GET_NET_COUNTER(VAL, NAME) \ > + if (virAsprintf(&name, "net.nic%d.%s", net_index, NAME) < 0) \ > + goto cleanup; \ > + if (prlsdkGetStatsParam(dom, name, &stats->VAL) < 0) \ > + goto cleanup; \ > + VIR_FREE(name); > + > + PARALLELS_GET_NET_COUNTER(rx_bytes, "bytes_in") > + PARALLELS_GET_NET_COUNTER(rx_packets, "pkts_in") > + PARALLELS_GET_NET_COUNTER(tx_bytes, "bytes_out") > + PARALLELS_GET_NET_COUNTER(tx_packets, "pkts_out") > + stats->rx_errs = -1; > + stats->rx_drop = -1; > + stats->tx_errs = -1; > + stats->tx_drop = -1; > + > +#undef PARALLELS_GET_NET_COUNTER > + ret = 0; > + > + cleanup: > + VIR_FREE(name); > + if (dom) > + virDomainObjEndAPI(&dom); > + > + return ret; > +} > static virHypervisorDriver vzDriver = { > .name = "vz", > @@ -1373,6 +1416,7 @@ static virHypervisorDriver vzDriver = { > .domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */ > .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */ > .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */ > + .domainInterfaceStats = vzDomainInterfaceStats, /* 1.3.0 */ > }; > static virConnectDriver vzConnectDriver = { > diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c > index f9cde44..0956b58 100644 > --- a/src/vz/vz_sdk.c > +++ b/src/vz/vz_sdk.c > @@ -3562,7 +3562,7 @@ prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char *name, long long *val) > #define PARALLELS_STATISTICS_TIMEOUT (60 * 1000) > -static int > +int > prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val) > { > vzDomObjPtr privdom = dom->privateData; > @@ -3658,3 +3658,70 @@ prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBloc > VIR_FREE(name); > return ret; > } > + > +static PRL_HANDLE > +prlsdkFindNetAdapter(virDomainObjPtr dom, const char *path) > +{ > + PRL_UINT32 count = 0; > + vzDomObjPtr privdom = dom->privateData; > + PRL_UINT32 buflen = 0; > + PRL_RESULT pret; > + size_t i; > + char *name = NULL; > + PRL_HANDLE net = PRL_INVALID_HANDLE; > + > + pret = PrlVmCfg_GetNetAdaptersCount(privdom->sdkdom, &count); > + prlsdkCheckRetGoto(pret, error); > + > + for (i = 0; i < count; ++i) { > + pret = PrlVmCfg_GetNetAdapter(privdom->sdkdom, i, &net); > + prlsdkCheckRetGoto(pret, error); > + > + pret = PrlVmDevNet_GetHostInterfaceName(net, NULL, &buflen); > + prlsdkCheckRetGoto(pret, error); > + > + if (VIR_ALLOC_N(name, buflen) < 0) > + goto error; > + > + pret = PrlVmDevNet_GetHostInterfaceName(net, name, &buflen); > + prlsdkCheckRetGoto(pret, error); > + > + if (STREQ(name, path)) > + break; > + > + VIR_FREE(name); > + PrlHandle_Free(net); > + net = PRL_INVALID_HANDLE; > + } > + > + if (net == PRL_INVALID_HANDLE) > + virReportError(VIR_ERR_INVALID_ARG, > + _("invalid path, '%s' is not a known interface"), path); > + return net; > + > + error: > + VIR_FREE(name); > + PrlHandle_Free(net); > + return PRL_INVALID_HANDLE; > +} > + I think you can just return 'i' as adapter index, you use prlsdkFindNetAdapter function only in prlsdkGetAdapterIndex, do you?

> +int > +prlsdkGetAdapterIndex(virDomainObjPtr dom, const char *path) > +{ > + PRL_HANDLE net = PRL_INVALID_HANDLE; > + PRL_UINT32 net_index = -1; > + PRL_RESULT pret; > + int ret = -1; > + > + net = prlsdkFindNetAdapter(dom, path); > + if (net == PRL_INVALID_HANDLE) > + return -1; > + > + pret = PrlVmDev_GetIndex(net, &net_index); > + prlsdkCheckRetGoto(pret, cleanup); > + > + ret = net_index; > + cleanup: > + PrlHandle_Free(net); > + return ret; > +} > diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h > index dd4fecf..8f72e24 100644 > --- a/src/vz/vz_sdk.h > +++ b/src/vz/vz_sdk.h > @@ -66,3 +66,7 @@ int > prlsdkDetachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk); > int > prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr stats); > +int > +prlsdkGetAdapterIndex(virDomainObjPtr dom, const char *path); > +int > +prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val);