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

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

 




On 25.06.2015 17:51, Dmitry Guryanov wrote:
> On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote:
>> From: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxx>
>> ---
>>   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?
No I can't. Enumeration index and device index are not the same. Say you can add
3 net devices, net0, net1, net2, then delete net1 and you get net0, net2 when
you enumerate them, for net2 enumeration index will be 1, not 2.

I use prlsdkFindNetAdapter only in prlsdkFindNetAdapter, but I want them to be splitted as 
they have different functions, namely 'find by property' and 'get property'.
>> +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);
> 
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]