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

[Dmitry Guryanov]

On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote:
> From: Nikolay Shirokovskiy <nshirokovskiy at parallels.com>
>
> 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 at virtuozzo.com>
> ---
>   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);


-- 
Dmitry Guryanov