Re: [Qemu-devel] [PATCH v4] qga: Add support network interfacestatistics in

* Re: [Qemu-devel] [PATCH v4] qga: Add support network interfacestatistics in
@ 2017-04-26  1:20 lu.zhipeng
  2017-04-26 13:24 ` Eric Blake
  0 siblings, 1 reply; 2+ messages in thread
From: lu.zhipeng @ 2017-04-26  1:20 UTC (permalink / raw)
  To: mdroth; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 8098 bytes --]

＞ +   */＞ +        colon = strchr(line, ':')＞ +        if (!colon) {＞ +            continue＞ +        }＞ +        *colon = '\0'＞ +        if (colon - path_len ＞= line && strcmp(colon - path_len, path) == 0) {colon - path_len ＞ line would imply the current line isn't a matcheither, so i think that can be tightened to colon - path_len == line.

------------

The text reading from /proc/net/dev/ is  aligned to the right. so i think it should be colon - path_len ＞= line

为了让您的VPlat虚拟化故障得到高效的处理，请上报故障到: $VPlat技术支持。

芦志朋 luzhipeng

IT开发工程师 IT Development
Engineer
操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R＆D Institute/System Product

深圳市南山区科技南路55号中兴通讯研发大楼33楼 
33/F, R&D Building, ZTE
Corporation Hi-tech Road South, 
Hi-tech
Industrial Park Nanshan District, Shenzhen, P.R.China, 518057 
T: +86 755 xxxxxxxx F:+86 755 xxxxxxxx 
M: +86 xxxxxxxxxxx 
E: lu.zhipeng@zte.com.cn 
www.zte.com.cn

原始邮件

发件人： ＜mdroth@linux.vnet.ibm.com＞
收件人：芦志朋10108272
抄送人：芦志朋10108272 ＜qemu-devel@nongnu.org＞
日 期 ：2017年04月26日 05:17
主 题 ：Re: [Qemu-devel] [PATCH v4] qga: Add support network interfacestatistics in

Quoting ZhiPeng Lu (2017-04-25 03:12:20)
＞ we can get the network interface statistics inside a virtual machine by
＞ guest-network-get-interfaces command. it is very useful for us to monitor
＞ and analyze network traffic.
＞ 
＞ Signed-off-by: ZhiPeng Lu ＜lu.zhipeng@zte.com.cn＞
＞ Signed-off-by: Daniel P. Berrange ＜berrange@redhat.com＞
＞ ---
＞  qga/commands-posix.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++-
＞  qga/qapi-schema.json | 38 +++++++++++++++++++++++++++-
＞  2 files changed, 107 insertions(+), 2 deletions(-)
＞ 
＞ diff --git a/qga/commands-posix.c b/qga/commands-posix.c
＞ index 915df9e..1e35340 100644
＞ --- a/qga/commands-posix.c
＞ +++ b/qga/commands-posix.c
＞ @@ -1638,6 +1638,65 @@ guest_find_interface(GuestNetworkInterfaceList *head,
＞      return head
＞  }
＞ 
＞ + static int guest_get_network_stats(const char *path,

Path is a little misleading. Isn't this the device name?

＞ +                       GuestNetworkInterfaceStat *stats)
＞ +{
＞ +    int path_len
＞ +    char const *devinfo = "/proc/net/dev"
＞ +    FILE *fp
＞ +    char *line = NULL, *colon
＞ +    size_t n
＞ +    fp = fopen(devinfo, "r")
＞ +    if (!fp) {
＞ +        return -1
＞ +    }
＞ +    path_len = strlen(path)

name_len?

＞ +    while (getline(&line, &n, fp) != -1) {
＞ +        long long dummy
＞ +        long long rx_bytes
＞ +        long long rx_packets
＞ +        long long rx_errs
＞ +        long long rx_dropped
＞ +        long long tx_bytes
＞ +        long long tx_packets
＞ +        long long tx_errs
＞ +        long long tx_dropped
＞ +
＞ +  /* The line looks like:
＞ +   *"   eth0:..."
＞ +   *Split it at the colon.

Space after the "*". Indentation seems to wrong too.

＞ +   */
＞ +        colon = strchr(line, ':')
＞ +        if (!colon) {
＞ +            continue
＞ +        }
＞ +        *colon = '\0'
＞ +        if (colon - path_len ＞= line && strcmp(colon - path_len, path) == 0) {

colon - path_len ＞ line would imply the current line isn't a match
either, so i think that can be tightened to colon - path_len == line.

＞ +            if (sscanf(colon + 1,
＞ +                "%lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld",
＞ +                  &rx_bytes, &rx_packets, &rx_errs, &rx_dropped,
＞ +                  &dummy, &dummy, &dummy, &dummy,
＞ +                  &tx_bytes, &tx_packets, &tx_errs, &tx_dropped,
＞ +                  &dummy, &dummy, &dummy, &dummy) != 16) {
＞ +                continue
＞ +            }
＞ +            stats-＞rx_bytes = rx_bytes
＞ +            stats-＞rx_packets = rx_packets
＞ +            stats-＞rx_errs = rx_errs
＞ +            stats-＞rx_dropped = rx_dropped
＞ +            stats-＞tx_bytes = tx_bytes
＞ +            stats-＞tx_packets = tx_packets
＞ +            stats-＞tx_errs = tx_errs
＞ +            stats-＞tx_dropped = tx_dropped
＞ +            fclose(fp)
＞ +            return 0
＞ +        }
＞ +    }
＞ +    fclose(fp)
＞ +    g_debug("/proc/net/dev: Interface not found")
＞ +    return -1
＞ +}
＞ +
＞  /*
＞   * Build information about guest interfaces
＞   */
＞ @@ -1654,6 +1713,7 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
＞      for (ifa = ifap ifa ifa = ifa-＞ifa_next) {
＞          GuestNetworkInterfaceList *info
＞          GuestIpAddressList **address_list = NULL, *address_item = NULL
＞ +        GuestNetworkInterfaceStat  *interface_stat = NULL
＞          char addr4[INET_ADDRSTRLEN]
＞          char addr6[INET6_ADDRSTRLEN]
＞          int sock
＞ @@ -1773,7 +1833,16 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
＞ 
＞          info-＞value-＞has_ip_addresses = true
＞ 
＞ -
＞ +        if (!info-＞value-＞has_statistics) {
＞ +            interface_stat = g_malloc0(sizeof(*interface_stat))
＞ +            if (guest_get_network_stats(info-＞value-＞name,
＞ +                interface_stat) == -1) {
＞ +                error_setg_errno(errp, errno, "guest_get_network_stats failed")

Need to free interface_stat here else you'll leak memory.

Since it's an optional field I'm not sure we should treat cases where
/proc/net/dev doesn't exist as a failure either. We should probably just
set has_statistics = false to maintain the existing support.

＞ +                goto error
＞ +            }
＞ +            info-＞value-＞statistics = interface_stat
＞ +            info-＞value-＞has_statistics = true
＞ +        }
＞      }
＞ 
＞      freeifaddrs(ifap)
＞ diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
＞ index a02dbf2..948219b 100644
＞ --- a/qga/qapi-schema.json
＞ +++ b/qga/qapi-schema.json
＞ @@ -635,6 +635,38 @@
＞             'prefix': 'int'} }
＞ 
＞  ##
＞ +# @GuestNetworkInterfaceStat:
＞ +#
＞ +# @rx-bytes: total bytes received
＞ +#
＞ +# @rx-packets: total packets received
＞ +#
＞ +# @rx-errs: bad packets received
＞ +#
＞ +# @rx-dropped: receiver dropped packets
＞ +#
＞ +# @tx-bytes: total bytes transmitted
＞ +#
＞ +# @tx-packets: total packets transmitted
＞ +#
＞ +# @tx-errs: packet transmit problems
＞ +#
＞ +# @tx-dropped: dropped packets transmitted
＞ +#
＞ +# Since: 2.10
＞ +##
＞ +{ 'struct': 'GuestNetworkInterfaceStat',
＞ +  'data': {'rx-bytes': 'uint64',
＞ +            'rx-packets': 'uint64',
＞ +            'rx-errs': 'uint64',
＞ +            'rx-dropped': 'uint64',
＞ +            'tx-bytes': 'uint64',
＞ +            'tx-packets': 'uint64',
＞ +            'tx-errs': 'uint64',
＞ +            'tx-dropped': 'uint64'
＞ +           } }
＞ +
＞ +##
＞  # @GuestNetworkInterface:
＞  #
＞  # @name: The name of interface for which info are being delivered
＞ @@ -643,12 +675,16 @@
＞  #
＞  # @ip-addresses: List of addresses assigned to @name
＞  #
＞ +# @statistics: various statistic counters related to @name
＞ +# (since 2.10)
＞ +#
＞  # Since: 1.1
＞  ##
＞  { 'struct': 'GuestNetworkInterface',
＞    'data': {'name': 'str',
＞             '*hardware-address': 'str',
＞ -           '*ip-addresses': ['GuestIpAddress'] } }
＞ +           '*ip-addresses': ['GuestIpAddress'],
＞ +           '*statistics': 'GuestNetworkInterfaceStat' } }
＞ 
＞  ##
＞  # @guest-network-get-interfaces:
＞ -- 
＞ 1.8.3.1
＞ 
＞

[-- Attachment #2: 24242e5637af428891c4db731e7765ad.jpg --]
[-- Type: image/jpeg, Size: 2064 bytes --]

[-- Attachment #3: 9ae3e214c17d49ed935d87c674ba3ee2.jpg --]
[-- Type: image/jpeg, Size: 6015 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread