6:28 p.m.
This series implements support for querying network interface
stats on (Free)BSD.
It's more of an RFC, because I'm uncertain about few things:
- It feels a little strange to have a source file that implements
only a single function like this. I am wondering if it would be better
to just move it to something like util/virnetdev.c?
- FreeBSD stores interface data in the if_data struct and a number of outgoing
packet drops is stored in a field 'ifi_oqdrops'. This field was added
in -CURRENT and later merged back to 10-STABLE. In order not to break
the ABI, it's available only if _IFI_OQDROPS is defined. I've added
a configure.ac check which adds -D_IFI_OQDROPS before checking this field
and resetting it back if it is not present. This way, this flag will
present when the field is available even if the flag is not needed
(e.g. on -CURRENT). Is there a better way of doing it? I was thinking
about trying to check this field without the flag and if it fails check
one more time with the flag, but it looks a little messy.
- Did I get it right that the stats reported are from the guest POV, e.g.
when downloading a large file from guest, it should look like:
vnet0 rx_bytes 731603341
vnet0 rx_packets 518354
vnet0 rx_errs 0
vnet0 rx_drop 0
vnet0 tx_bytes 17577834
vnet0 tx_packets 264226
vnet0 tx_errs 0
vnet0 tx_drop 0
Roman Bogorodskiy (2):
util: virstatslinux: make more generic
Implement interface stats for BSD
configure.ac | 13 ++++-
po/POTFILES.in | 2 +-
src/Makefile.am | 2 +-
src/libvirt_linux.syms | 3 --
src/libvirt_private.syms | 2 +
src/lxc/lxc_driver.c | 2 +-
src/openvz/openvz_driver.c | 2 +-
src/qemu/qemu_driver.c | 16 +-----
src/uml/uml_driver.c | 2 +-
src/util/{virstatslinux.c => virstats.c} | 93 +++++++++++++++++++++++++-------
src/util/{virstatslinux.h => virstats.h} | 12 ++---
src/xen/xen_hypervisor.c | 2 +-
tests/statstest.c | 2 +-
13 files changed, 102 insertions(+), 51 deletions(-)
rename src/util/{virstatslinux.c => virstats.c} (61%)
rename src/util/{virstatslinux.h => virstats.h} (77%)
--
1.9.0
6:28 p.m.
New subject: [libvirt] [PATCH 1/2] util: virstatslinux: make more generic
Rename linuxDomainInterfaceStats to virNetInterfaceStats in order
to allow adding platform specific implementations without
making consumer worrying about specific implementation to be used.
Also, rename util/virstatslinux.c to util/virstats.c so placing
other platform specific implementations into this file don't
look unexpected from the file name.
---
po/POTFILES.in | 2 +-
src/Makefile.am | 2 +-
src/libvirt_linux.syms | 3 ---
src/libvirt_private.syms | 2 ++
src/lxc/lxc_driver.c | 4 +--
src/openvz/openvz_driver.c | 4 +--
src/qemu/qemu_driver.c | 16 ++----------
src/uml/uml_driver.c | 2 +-
src/util/{virstatslinux.c => virstats.c} | 44 ++++++++++++++++++--------------
src/util/{virstatslinux.h => virstats.h} | 12 +++------
src/xen/xen_hypervisor.c | 4 +--
tests/statstest.c | 2 +-
12 files changed, 43 insertions(+), 54 deletions(-)
rename src/util/{virstatslinux.c => virstats.c} (82%)
rename src/util/{virstatslinux.h => virstats.h} (77%)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 64a987e..fd4b56e 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -198,7 +198,7 @@ src/util/virrandom.c
src/util/virsexpr.c
src/util/virscsi.c
src/util/virsocketaddr.c
-src/util/virstatslinux.c
+src/util/virstats.c
src/util/virstorageencryption.c
src/util/virstoragefile.c
src/util/virstring.c
diff --git a/src/Makefile.am b/src/Makefile.am
index e2f76a7..a40e63f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -149,7 +149,7 @@ UTIL_SOURCES = \
util/virseclabel.c util/virseclabel.h \
util/virsexpr.c util/virsexpr.h \
util/virsocketaddr.h util/virsocketaddr.c \
- util/virstatslinux.c util/virstatslinux.h \
+ util/virstats.c util/virstats.h \
util/virstorageencryption.c util/virstorageencryption.h \
util/virstoragefile.c util/virstoragefile.h \
util/virstring.h util/virstring.c \
diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms
index b3b2384..1a7f263 100644
--- a/src/libvirt_linux.syms
+++ b/src/libvirt_linux.syms
@@ -6,9 +6,6 @@
linuxNodeGetCPUStats;
linuxNodeInfoCPUPopulate;
-# util/virstatslinux.h
-linuxDomainInterfaceStats;
-
# Let emacs know we want case-insensitive sorting
# Local Variables:
# sort-fold-case: t
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 29e9db9..1aca260 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1880,6 +1880,8 @@ virSocketAddrPrefixToNetmask;
virSocketAddrSetIPv4Addr;
virSocketAddrSetPort;
+# util/virstats.h
+virNetInterfaceStats;
# util/virstorageencryption.h
virStorageEncryptionFormat;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index fce16f2..fe1e555 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -51,7 +51,7 @@
#include "virnetdevveth.h"
#include "nodeinfo.h"
#include "viruuid.h"
-#include "virstatslinux.h"
+#include "virstats.h"
#include "virhook.h"
#include "virfile.h"
#include "virpidfile.h"
@@ -3092,7 +3092,7 @@ lxcDomainInterfaceStats(virDomainPtr dom,
}
if (ret == 0)
- ret = linuxDomainInterfaceStats(path, stats);
+ ret = virNetInterfaceStats(path, stats);
else
virReportError(VIR_ERR_INVALID_ARG,
_("Invalid path, '%s' is not a known
interface"), path);
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index baa7256..851ed30 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -56,7 +56,7 @@
#include "virlog.h"
#include "vircommand.h"
#include "viruri.h"
-#include "virstatslinux.h"
+#include "virstats.h"
#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_OPENVZ
@@ -2010,7 +2010,7 @@ openvzDomainInterfaceStats(virDomainPtr dom,
}
if (ret == 0)
- ret = linuxDomainInterfaceStats(path, stats);
+ ret = virNetInterfaceStats(path, stats);
else
virReportError(VIR_ERR_INVALID_ARG,
_("invalid path, '%s' is not a known
interface"), path);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2d1aa9e..027a7ef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -61,7 +61,7 @@
#include "datatypes.h"
#include "virbuffer.h"
#include "nodeinfo.h"
-#include "virstatslinux.h"
+#include "virstats.h"
#include "capabilities.h"
#include "viralloc.h"
#include "viruuid.h"
@@ -9760,7 +9760,6 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
return ret;
}
-#ifdef __linux__
static int
qemuDomainInterfaceStats(virDomainPtr dom,
const char *path,
@@ -9792,7 +9791,7 @@ qemuDomainInterfaceStats(virDomainPtr dom,
}
if (ret == 0)
- ret = linuxDomainInterfaceStats(path, stats);
+ ret = virNetInterfaceStats(path, stats);
else
virReportError(VIR_ERR_INVALID_ARG,
_("invalid path, '%s' is not a known
interface"), path);
@@ -9802,17 +9801,6 @@ qemuDomainInterfaceStats(virDomainPtr dom,
virObjectUnlock(vm);
return ret;
}
-#else
-static int
-qemuDomainInterfaceStats(virDomainPtr dom ATTRIBUTE_UNUSED,
- const char *path ATTRIBUTE_UNUSED,
- struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
-{
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("interface stats not implemented on this platform"));
- return -1;
-}
-#endif
static int
qemuDomainSetInterfaceParameters(virDomainPtr dom,
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 62d1afe..7039afc 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -47,7 +47,7 @@
#include "uml_conf.h"
#include "virbuffer.h"
#include "nodeinfo.h"
-#include "virstatslinux.h"
+#include "virstats.h"
#include "capabilities.h"
#include "viralloc.h"
#include "viruuid.h"
diff --git a/src/util/virstatslinux.c b/src/util/virstats.c
similarity index 82%
rename from src/util/virstatslinux.c
rename to src/util/virstats.c
index 60b72dc..17ef5b6 100644
--- a/src/util/virstatslinux.c
+++ b/src/util/virstats.c
@@ -1,5 +1,5 @@
/*
- * virstatslinux.c: Linux block and network stats.
+ * virstats.c: Block and network stats.
*
* Copyright (C) 2007-2010 Red Hat, Inc.
*
@@ -22,23 +22,20 @@
#include <config.h>
-/* This file only applies on Linux. */
-#ifdef __linux__
-
-# include <stdio.h>
-# include <stdlib.h>
-# include <fcntl.h>
-# include <string.h>
-# include <unistd.h>
-# include <regex.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <string.h>
+#include <unistd.h>
+#include <regex.h>
-# include "virerror.h"
-# include "datatypes.h"
-# include "virstatslinux.h"
-# include "viralloc.h"
-# include "virfile.h"
+#include "virerror.h"
+#include "datatypes.h"
+#include "virstats.h"
+#include "viralloc.h"
+#include "virfile.h"
-# define VIR_FROM_THIS VIR_FROM_STATS_LINUX
+#define VIR_FROM_THIS VIR_FROM_STATS_LINUX
/*-------------------- interface stats --------------------*/
@@ -46,10 +43,10 @@
* NB. Caller must check that libvirt user is trying to query
* the interface of a domain they own. We do no such checking.
*/
-
+#ifdef __linux__
int
-linuxDomainInterfaceStats(const char *path,
- struct _virDomainInterfaceStats *stats)
+virNetInterfaceStats(const char *path,
+ struct _virDomainInterfaceStats *stats)
{
int path_len;
FILE *fp;
@@ -117,5 +114,14 @@ linuxDomainInterfaceStats(const char *path,
_("/proc/net/dev: Interface not found"));
return -1;
}
+#else
+int
+virNetInterfaceStats(const char *path ATTRIBUTE_UNUSED,
+ struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
+{
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
+ return -1;
+}
#endif /* __linux__ */
diff --git a/src/util/virstatslinux.h b/src/util/virstats.h
similarity index 77%
rename from src/util/virstatslinux.h
rename to src/util/virstats.h
index d5900ed..9724d8e 100644
--- a/src/util/virstatslinux.h
+++ b/src/util/virstats.h
@@ -1,5 +1,5 @@
/*
- * virstatslinux.h: Linux block and network stats.
+ * virstats.h: Block and network stats.
*
* Copyright (C) 2007 Red Hat, Inc.
*
@@ -23,13 +23,9 @@
#ifndef __STATS_LINUX_H__
# define __STATS_LINUX_H__
-# ifdef __linux__
+# include "internal.h"
-# include "internal.h"
-
-extern int linuxDomainInterfaceStats(const char *path,
- struct _virDomainInterfaceStats *stats);
-
-# endif /* __linux__ */
+extern int virNetInterfaceStats(const char *path,
+ struct _virDomainInterfaceStats *stats);
#endif /* __STATS_LINUX_H__ */
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 524b21f..af24e2e 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -68,7 +68,7 @@
#include "xen_driver.h"
#include "xen_hypervisor.h"
#include "xs_internal.h"
-#include "virstatslinux.h"
+#include "virstats.h"
#include "block_stats.h"
#include "xend_internal.h"
#include "virbuffer.h"
@@ -1470,7 +1470,7 @@ xenHypervisorDomainInterfaceStats(virDomainDefPtr def,
return -1;
}
- return linuxDomainInterfaceStats(path, stats);
+ return virNetInterfaceStats(path, stats);
#else
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("/proc/net/dev: Interface not found"));
diff --git a/tests/statstest.c b/tests/statstest.c
index ce1567c..f9b4573 100644
--- a/tests/statstest.c
+++ b/tests/statstest.c
@@ -5,7 +5,7 @@
#include <string.h>
#include <sys/utsname.h>
-#include "virstatslinux.h"
+#include "virstats.h"
#include "internal.h"
#include "xen/block_stats.h"
#include "testutils.h"
--
1.9.0
6:28 p.m.
New subject: [libvirt] [PATCH 2/2] Implement interface stats for BSD
---
configure.ac | 13 ++++++++++++-
src/util/virstats.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index a12186b..8001e24 100644
--- a/configure.ac
+++ b/configure.ac
@@ -275,7 +275,7 @@ dnl and various less common threadsafe functions
AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \
posix_memalign prlimit regexec sched_getaffinity setgroups setns \
- setrlimit symlink sysctlbyname])
+ setrlimit symlink sysctlbyname getifaddrs])
dnl Availability of pthread functions. Because of $LIB_PTHREAD, we
dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD
@@ -2691,6 +2691,17 @@ if test $with_freebsd = yes; then
)
fi
+# FreeBSD 10-STABLE requires _IFI_OQDROPS to be defined for if_data.ifi_oqdrops
+# field be available
+old_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS -D_IFI_OQDROPS"
+
+AC_CHECK_MEMBERS([struct if_data.ifi_oqdrops],
+ [],
+ [CFLAGS="$old_CFLAGS"],
+ [#include <net/if.h>
+ ])
+
# Check if we need to look for ifconfig
if test "$want_ifconfig" = "yes"; then
AC_PATH_PROG([IFCONFIG_PATH], [ifconfig])
diff --git a/src/util/virstats.c b/src/util/virstats.c
index 17ef5b6..910803f 100644
--- a/src/util/virstats.c
+++ b/src/util/virstats.c
@@ -29,6 +29,11 @@
#include <unistd.h>
#include <regex.h>
+#ifdef HAVE_GETIFADDRS
+# include <net/if.h>
+# include <ifaddrs.h>
+#endif
+
#include "virerror.h"
#include "datatypes.h"
#include "virstats.h"
@@ -114,6 +119,52 @@ virNetInterfaceStats(const char *path,
_("/proc/net/dev: Interface not found"));
return -1;
}
+#elif defined(HAVE_GETIFADDRS)
+int
+virNetInterfaceStats(const char *path,
+ struct _virDomainInterfaceStats *stats)
+{
+ struct ifaddrs *ifap, *ifa;
+ struct if_data *ifd;
+ int ret = -1;
+
+ if (getifaddrs(&ifap) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Could not get interface list"));
+ return -1;
+ }
+
+ for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+ if (ifa->ifa_addr->sa_family != AF_LINK)
+ continue;
+
+ if (STREQ(ifa->ifa_name, path)) {
+ ifd = (struct if_data *)ifa->ifa_data;
+ stats->tx_bytes = ifd->ifi_ibytes;
+ stats->tx_packets = ifd->ifi_ipackets;
+ stats->tx_errs = ifd->ifi_ierrors;
+ stats->tx_drop = ifd->ifi_iqdrops;
+ stats->rx_bytes = ifd->ifi_obytes;
+ stats->rx_packets = ifd->ifi_opackets;
+ stats->rx_errs = ifd->ifi_oerrors;
+# ifdef HAVE_STRUCT_IF_DATA_IFI_OQDROPS
+ stats->rx_drop = ifd->ifi_oqdrops;
+# else
+ stats->rx_drop = 0;
+# endif
+
+ ret = 0;
+ break;
+ }
+ }
+
+ if (ret < 0)
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Interface not found"));
+
+ freeifaddrs(ifap);
+ return ret;
+}
#else
int
virNetInterfaceStats(const char *path ATTRIBUTE_UNUSED,
--
1.9.0
1:49 p.m.
On 06.07.2014 18:28, Roman Bogorodskiy wrote:
This series implements support for querying network interface
stats on (Free)BSD.
It's more of an RFC, because I'm uncertain about few things:
- It feels a little strange to have a source file that implements
only a single function like this. I am wondering if it would be better
to just move it to something like util/virnetdev.c?
- FreeBSD stores interface data in the if_data struct and a number of outgoing
packet drops is stored in a field 'ifi_oqdrops'. This field was added
in -CURRENT and later merged back to 10-STABLE. In order not to break
the ABI, it's available only if _IFI_OQDROPS is defined. I've added
a configure.ac check which adds -D_IFI_OQDROPS before checking this field
and resetting it back if it is not present. This way, this flag will
present when the field is available even if the flag is not needed
(e.g. on -CURRENT). Is there a better way of doing it? I was thinking
about trying to check this field without the flag and if it fails check
one more time with the flag, but it looks a little messy.
- Did I get it right that the stats reported are from the guest POV, e.g.
when downloading a large file from guest, it should look like:
vnet0 rx_bytes 731603341
vnet0 rx_packets 518354
vnet0 rx_errs 0
vnet0 rx_drop 0
vnet0 tx_bytes 17577834
vnet0 tx_packets 264226
vnet0 tx_errs 0
vnet0 tx_drop 0
Roman Bogorodskiy (2):
util: virstatslinux: make more generic
Implement interface stats for BSD
configure.ac | 13 ++++-
po/POTFILES.in | 2 +-
src/Makefile.am | 2 +-
src/libvirt_linux.syms | 3 --
src/libvirt_private.syms | 2 +
src/lxc/lxc_driver.c | 2 +-
src/openvz/openvz_driver.c | 2 +-
src/qemu/qemu_driver.c | 16 +-----
src/uml/uml_driver.c | 2 +-
src/util/{virstatslinux.c => virstats.c} | 93 +++++++++++++++++++++++++-------
src/util/{virstatslinux.h => virstats.h} | 12 ++---
src/xen/xen_hypervisor.c | 2 +-
tests/statstest.c | 2 +-
13 files changed, 102 insertions(+), 51 deletions(-)
rename src/util/{virstatslinux.c => virstats.c} (61%)
rename src/util/{virstatslinux.h => virstats.h} (77%)
ACK to both patches.
Michal
3720
days inactive
3729
days old
4 comments
2 participants
participants (2)
-
Michal Privoznik -
Roman Bogorodskiy