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

Re: [Libvir] [PATCH] Enhanced stats for fullvirt domains



Thanks Jim. Here is an updated patch which fixes this issue, and is tested against Xen 3.1.

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903
Index: src/Makefile.am
===================================================================
RCS file: /data/cvs/libvirt/src/Makefile.am,v
retrieving revision 1.50
diff -u -p -r1.50 Makefile.am
--- src/Makefile.am	19 Sep 2007 17:42:40 -0000	1.50
+++ src/Makefile.am	28 Sep 2007 09:26:41 -0000
@@ -38,6 +38,7 @@ CLIENT_SOURCES =						\
 		xen_internal.c xen_internal.h			\
 		xs_internal.c xs_internal.h			\
 		xend_internal.c xend_internal.h			\
+		stats_linux.c stats_linux.h			\
 		sexpr.c sexpr.h					\
 		virterror.c					\
 		driver.h					\
@@ -49,8 +50,8 @@ CLIENT_SOURCES =						\
 		iptables.c iptables.h 				\
 		uuid.c uuid.h 					\
 		qemu_driver.c qemu_driver.h 			\
-		qemu_conf.c qemu_conf.h					\
-		openvz_conf.c openvz_conf.h				\
+		qemu_conf.c qemu_conf.h				\
+		openvz_conf.c openvz_conf.h			\
 		openvz_driver.c openvz_driver.h 		\
                 nodeinfo.h nodeinfo.c                           \
 		util.c util.h
Index: src/stats_linux.c
===================================================================
RCS file: src/stats_linux.c
diff -N src/stats_linux.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/stats_linux.c	28 Sep 2007 09:26:41 -0000
@@ -0,0 +1,334 @@
+/*
+ * Linux block and network stats.
+ *
+ * Copyright (C) 2007 Red Hat, Inc.
+ *
+ * See COPYING.LIB for the License of this software
+ *
+ * Richard W.M. Jones <rjones redhat com>
+ */
+
+#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 <xs.h>
+
+#include "internal.h"
+#include "xen_unified.h"
+#include "stats_linux.h"
+
+/**
+ * statsErrorFunc:
+ * @error: the error number
+ * @func: the function failing
+ * @info: extra information string
+ * @value: extra information number
+ *
+ * Handle a stats error.
+ */
+static void
+statsErrorFunc(virErrorNumber error, const char *func, const char *info,
+               int value)
+{
+    char fullinfo[1000];
+    const char *errmsg;
+
+    errmsg = __virErrorMsg(error, info);
+    if (func != NULL) {
+        snprintf(fullinfo, 999, "%s: %s", func, info);
+        fullinfo[999] = 0;
+        __virRaiseError(NULL, NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR,
+                        errmsg, fullinfo, NULL, value, 0, errmsg, fullinfo,
+                        value);
+    } else {
+        __virRaiseError(NULL, NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR,
+                        errmsg, info, NULL, value, 0, errmsg, info,
+                        value);
+    }
+}
+
+#ifdef WITH_XEN
+/*-------------------- Xen: block stats --------------------*/
+
+#include <linux/major.h>
+
+/* This is normally defined in <linux/major.h> but previously we
+ * hard-coded it.  So if it's not defined, hard-code again.
+ */
+#ifndef XENVBD_MAJOR
+#define XENVBD_MAJOR 202
+#endif
+
+static int
+xstrtoint64 (char const *s, int base, int64_t *result)
+{
+    long long int lli;
+    char *p;
+
+    errno = 0;
+    lli = strtoll (s, &p, base);
+    if (errno || !(*p == 0 || *p == '\n') || p == s || (int64_t) lli != lli)
+        return -1;
+    *result = lli;
+    return 0;
+}
+
+static int64_t
+read_stat (const char *path)
+{
+    char str[64];
+    int64_t r;
+    int i;
+    FILE *fp;
+
+    fp = fopen (path, "r");
+    if (!fp)
+      return -1;
+
+    /* read, but don't bail out before closing */
+    i = fread (str, 1, sizeof str - 1, fp);
+
+    if (fclose (fp) != 0        /* disk error */
+        || i < 1)               /* ensure we read at least one byte */
+        return -1;
+
+    str[i] = '\0';              /* make sure the string is nul-terminated */
+    if (xstrtoint64 (str, 10, &r) == -1)
+        return -1;
+
+    return r;
+}
+
+static int64_t
+read_bd_stat (int device, int domid, const char *str)
+{
+    char path[PATH_MAX];
+    int64_t r;
+
+    snprintf (path, sizeof path,
+              "/sys/devices/xen-backend/vbd-%d-%d/statistics/%s",
+              domid, device, str);
+    r = read_stat (path);
+    if (r >= 0) return r;
+
+    snprintf (path, sizeof path,
+              "/sys/devices/xen-backend/tap-%d-%d/statistics/%s",
+              domid, device, str);
+    r = read_stat (path);
+    return r;
+}
+
+/* In Xenstore, /local/domain/0/backend/vbd/<domid>/<device>/state,
+ * if available, must be XenbusStateConnected (= 4), otherwise there
+ * is no connected device.
+ */
+static int
+check_bd_connected (xenUnifiedPrivatePtr priv, int device, int domid)
+{
+    char s[256], *rs;
+    int r;
+    unsigned len = 0;
+
+    /* This code assumes we're connected if we can't get to
+     * xenstore, etc.
+     */
+    if (!priv->xshandle) return 1;
+    snprintf (s, sizeof s, "/local/domain/0/backend/vbd/%d/%d/state",
+              domid, device);
+    s[sizeof s - 1] = '\0';
+
+    rs = xs_read (priv->xshandle, 0, s, &len);
+    if (!rs) return 1;
+
+    r = STREQ (rs, "4");
+    free (rs);
+    return r;
+}
+
+static int
+read_bd_stats (xenUnifiedPrivatePtr priv,
+               int device, int domid, struct _virDomainBlockStats *stats)
+{
+    stats->rd_req   = read_bd_stat (device, domid, "rd_req");
+    stats->rd_bytes = read_bd_stat (device, domid, "rd_sect");
+    stats->wr_req   = read_bd_stat (device, domid, "wr_req");
+    stats->wr_bytes = read_bd_stat (device, domid, "wr_sect");
+    stats->errs     = read_bd_stat (device, domid, "oo_req");
+
+    /* None of the files were found - it's likely that this version
+     * of Xen is an old one which just doesn't support stats collection.
+     */
+    if (stats->rd_req == -1 && stats->rd_bytes == -1 &&
+        stats->wr_req == -1 && stats->wr_bytes == -1 &&
+        stats->errs == -1) {
+        statsErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
+                        "Failed to read any block statistics", domid);
+        return -1;
+    }
+
+    /* If stats are all zero then either there really isn't any block
+     * device activity, or there is no connected front end device
+     * in which case there are no stats.
+     */
+    if (stats->rd_req == 0 && stats->rd_bytes == 0 &&
+        stats->wr_req == 0 && stats->wr_bytes == 0 &&
+        stats->errs == 0 &&
+        !check_bd_connected (priv, device, domid)) {
+        statsErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
+                        "Frontend block device not connected", domid);
+        return -1;
+    }
+
+    /* 'Bytes' was really sectors when we read it.  Scale up by
+     * an assumed sector size.
+     */
+    if (stats->rd_bytes > 0) stats->rd_bytes *= 512;
+    if (stats->wr_bytes > 0) stats->wr_bytes *= 512;
+
+    return 0;
+}
+
+int
+xenLinuxDomainBlockStats (xenUnifiedPrivatePtr priv,
+                          virDomainPtr dom,
+                          const char *path,
+                          struct _virDomainBlockStats *stats)
+{
+    int minor, device;
+
+    /* Paravirt domains:
+     * Paths have the form "xvd[a-]" and map to paths
+     * /sys/devices/xen-backend/(vbd|tap)-domid-major:minor/
+     * statistics/(rd|wr|oo)_req.
+     * The major:minor is in this case fixed as 202*256 + minor*16
+     * where minor is 0 for xvda, 1 for xvdb and so on.
+     */
+    if (strlen (path) == 4 &&
+        STREQLEN (path, "xvd", 3)) {
+        if ((minor = path[3] - 'a') < 0 || minor > 26) {
+            statsErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
+                            "invalid path, should be xvda, xvdb, etc.",
+                            dom->id);
+            return -1;
+        }
+        device = XENVBD_MAJOR * 256 + minor * 16;
+
+        return read_bd_stats (priv, device, dom->id, stats);
+    }
+    /* Fullvirt domains:
+     * hda, hdb etc map to major = HD_MAJOR*256 + minor*16.
+     */
+    else if (strlen (path) == 3 &&
+             STREQLEN (path, "hd", 2)) {
+        if ((minor = path[2] - 'a') < 0 || minor > 26) {
+            statsErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
+                            "invalid path, should be hda, hdb, etc.",
+                            dom->id);
+            return -1;
+        }
+        device = HD_MAJOR * 256 + minor * 16;
+
+        return read_bd_stats (priv, device, dom->id, stats);
+    }
+
+    /* Otherwise, unsupported device name. */
+    statsErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
+                    "unsupported path (use xvda, hda, etc.)", dom->id);
+    return -1;
+}
+
+#endif /* WITH_XEN */
+
+/*-------------------- interface stats --------------------*/
+/* Just reads the named interface, so not Xen or QEMU-specific.
+ * NB. Caller must check that libvirt user is trying to query
+ * the interface of a domain they own.  We do no such checking.
+ */
+
+int
+linuxDomainInterfaceStats (const char *path,
+                           struct _virDomainInterfaceStats *stats)
+{
+    int path_len;
+    FILE *fp;
+    char line[256];
+
+    fp = fopen ("/proc/net/dev", "r");
+    if (!fp) {
+        statsErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
+                        "/proc/net/dev", errno);
+        return -1;
+    }
+
+    path_len = strlen (path);
+
+    while (fgets (line, sizeof line, fp)) {
+        long long dummy;
+        long long rx_bytes;
+        long long rx_packets;
+        long long rx_errs;
+        long long rx_drop;
+        long long tx_bytes;
+        long long tx_packets;
+        long long tx_errs;
+        long long tx_drop;
+
+        if (STREQLEN (line, path, path_len) &&
+            line[path_len] == ':' &&
+            line[path_len+1] == ' ') {
+            /* IMPORTANT NOTE!
+             * /proc/net/dev vif<domid>.nn sees the network from the point
+             * of view of dom0 / hypervisor.  So bytes TRANSMITTED by dom0
+             * are bytes RECEIVED by the domain.  That's why the TX/RX fields
+             * appear to be swapped here.
+             */
+            if (sscanf (&line[path_len+2],
+                        "%lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld",
+                        &tx_bytes, &tx_packets, &tx_errs, &tx_drop,
+                        &dummy, &dummy, &dummy, &dummy,
+                        &rx_bytes, &rx_packets, &rx_errs, &rx_drop,
+                        &dummy, &dummy, &dummy, &dummy) != 16)
+                continue;
+
+            stats->rx_bytes = rx_bytes;
+            stats->rx_packets = rx_packets;
+            stats->rx_errs = rx_errs;
+            stats->rx_drop = rx_drop;
+            stats->tx_bytes = tx_bytes;
+            stats->tx_packets = tx_packets;
+            stats->tx_errs = tx_errs;
+            stats->tx_drop = tx_drop;
+            fclose (fp);
+
+            return 0;
+        }
+    }
+    fclose (fp);
+
+    statsErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
+                    "/proc/net/dev: Interface not found", 0);
+    return -1;
+}
+
+#endif /* __linux__ */
+/*
+ * vim: set tabstop=4:
+ * vim: set shiftwidth=4:
+ * vim: set expandtab:
+ */
+/*
+ * Local variables:
+ *  indent-tabs-mode: nil
+ *  c-indent-level: 4
+ *  c-basic-offset: 4
+ *  tab-width: 4
+ * End:
+ */
Index: src/stats_linux.h
===================================================================
RCS file: src/stats_linux.h
diff -N src/stats_linux.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/stats_linux.h	28 Sep 2007 09:26:41 -0000
@@ -0,0 +1,39 @@
+/*
+ * Linux block and network stats.
+ *
+ * Copyright (C) 2007 Red Hat, Inc.
+ *
+ * See COPYING.LIB for the License of this software
+ *
+ * Richard W.M. Jones <rjones redhat com>
+ */
+
+#ifndef __STATS_LINUX_H__
+#define __STATS_LINUX_H__
+
+#ifdef __linux__
+
+#include "xen_unified.h"
+
+extern int xenLinuxDomainBlockStats (xenUnifiedPrivatePtr priv,
+				     virDomainPtr dom, const char *path,
+				     struct _virDomainBlockStats *stats);
+extern int linuxDomainInterfaceStats (const char *path,
+				      struct _virDomainInterfaceStats *stats);
+
+#endif /* __linux__ */
+
+#endif /* __STATS_LINUX_H__ */
+/*
+ * vim: set tabstop=4:
+ * vim: set shiftwidth=4:
+ * vim: set expandtab:
+ */
+/*
+ * Local variables:
+ *  indent-tabs-mode: nil
+ *  c-indent-level: 4
+ *  c-basic-offset: 4
+ *  tab-width: 4
+ * End:
+ */
Index: src/xen_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xen_internal.c,v
retrieving revision 1.94
diff -u -p -r1.94 xen_internal.c
--- src/xen_internal.c	29 Aug 2007 13:35:15 -0000	1.94
+++ src/xen_internal.c	28 Sep 2007 09:26:43 -0000
@@ -27,7 +27,9 @@
 #include <regex.h>
 #include <errno.h>
 #include <sys/utsname.h>
+
 #include "xs_internal.h"
+#include "stats_linux.h"
 
 /* required for dom0_getdomaininfo_t */
 #include <xen/dom0_ops.h>
@@ -1306,74 +1308,23 @@ xenHypervisorSetSchedulerParameters(virD
     return 0;
 }
 
-static int64_t
-read_stat (const char *path)
-{
-    char str[64];
-    int64_t r;
-    int i;
-    FILE *fp;
 
-    fp = fopen (path, "r");
-    if (!fp) return -1;
-    /* stupid GCC warning */ i = fread (str, sizeof str, 1, fp);
-    r = strtoll (str, NULL, 10);
-    fclose (fp);
-    return r;
-}
-
-static int64_t
-read_bd_stat (int device, int domid, const char *str)
-{
-    char path[PATH_MAX];
-    int64_t r;
-
-    snprintf (path, sizeof path,
-              "/sys/devices/xen-backend/vbd-%d-%d/statistics/%s_req",
-              domid, device, str);
-    r = read_stat (path);
-    if (r >= 0) return r;
-
-    snprintf (path, sizeof path,
-              "/sys/devices/xen-backend/tap-%d-%d/statistics/%s_req",
-              domid, device, str);
-    r = read_stat (path);
-    return r;
-}
-
-/* Paths have the form "xvd[a-]" and map to paths /sys/devices/xen-backend/
- * (vbd|tap)-domid-major:minor/statistics/(rd|wr|oo)_req.  The major:minor
- * is in this case fixed as 202*256 + 16*minor where minor is 0 for xvda,
- * 1 for xvdb and so on.
- */
 int
 xenHypervisorDomainBlockStats (virDomainPtr dom,
                                const char *path,
                                struct _virDomainBlockStats *stats)
 {
-    int minor, device;
-
-    if (strlen (path) != 4 ||
-        STRNEQLEN (path, "xvd", 3) ||
-        (minor = path[3] - 'a') < 0 ||
-        minor > 26) {
-        virXenErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
-                         "invalid path, should be xvda, xvdb, etc.", 0);
-        return -1;
-    }
-    device = 202 * 256 + minor;
-
-    stats->rd_req = read_bd_stat (device, dom->id, "rd");
-    stats->wr_req = read_bd_stat (device, dom->id, "wr");
-    stats->errs =   read_bd_stat (device, dom->id, "oo");
-
-    if (stats->rd_req == -1 && stats->wr_req == -1 && stats->errs == -1) {
-        virXenErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
-                         "Failed to read any block statistics", dom->id);
-        return -1;
-    }
+#ifdef __linux__
+    xenUnifiedPrivatePtr priv;
 
-    return 0;
+    priv = (xenUnifiedPrivatePtr) dom->conn->privateData;
+    return xenLinuxDomainBlockStats (priv, dom, path, stats);
+#else
+    virXenErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
+                     "block statistics not supported on this platform",
+                     dom->id);
+    return -1;
+#endif
 }
 
 /* Paths have the form vif<domid>.<n> (this interface checks that
@@ -1382,19 +1333,18 @@ xenHypervisorDomainBlockStats (virDomain
  * In future we may allow you to query bridge stats (virbrX or
  * xenbrX), but that will probably be through a separate
  * virNetwork interface, as yet not decided.
- *
- * On Linux we open /proc/net/dev and look for the device
- * called vif<domid>.<n>.
  */
 int
 xenHypervisorDomainInterfaceStats (virDomainPtr dom,
                                    const char *path,
                                    struct _virDomainInterfaceStats *stats)
 {
+#ifdef __linux__
     int rqdomid, device;
-    FILE *fp;
-    char line[256];
 
+    /* Verify that the vif requested is one belonging to the current
+     * domain.
+     */
     if (sscanf (path, "vif%d.%d", &rqdomid, &device) != 2) {
         virXenErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
                          "invalid path, should be vif<domid>.<n>.", 0);
@@ -1406,56 +1356,12 @@ xenHypervisorDomainInterfaceStats (virDo
         return -1;
     }
 
-    fp = fopen ("/proc/net/dev", "r");
-    if (!fp) {
-        virXenErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
-                         "/proc/net/dev", errno);
-        return -1;
-    }
-    while (fgets (line, sizeof line, fp)) {
-        int domid, port;
-        long long dummy;
-        long long rx_bytes;
-        long long rx_packets;
-        long long rx_errs;
-        long long rx_drop;
-        long long tx_bytes;
-        long long tx_packets;
-        long long tx_errs;
-        long long tx_drop;
-
-        /* IMPORTANT NOTE!
-         * /proc/net/dev vif<domid>.nn sees the network from the point
-         * of view of dom0 / hypervisor.  So bytes TRANSMITTED by dom0
-         * are bytes RECEIVED by the domain.  That's why the TX/RX fields
-         * appear to be swapped here.
-         */
-        if (sscanf (line, "vif%d.%d: %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld",
-                    &domid, &port,
-                    &tx_bytes, &tx_packets, &tx_errs, &tx_drop,
-                    &dummy, &dummy, &dummy, &dummy,
-                    &rx_bytes, &rx_packets, &rx_errs, &rx_drop,
-                    &dummy, &dummy, &dummy, &dummy) != 18)
-            continue;
-
-        if (domid == dom->id && port == device) {
-            stats->rx_bytes = rx_bytes;
-            stats->rx_packets = rx_packets;
-            stats->rx_errs = rx_errs;
-            stats->rx_drop = rx_drop;
-            stats->tx_bytes = tx_bytes;
-            stats->tx_packets = tx_packets;
-            stats->tx_errs = tx_errs;
-            stats->tx_drop = tx_drop;
-            fclose (fp);
-            return 0;
-        }
-    }
-    fclose (fp);
-
+    return linuxDomainInterfaceStats (path, stats);
+#else
     virXenErrorFunc (VIR_ERR_NO_SUPPORT, __FUNCTION__,
                      "/proc/net/dev: Interface not found", 0);
     return -1;
+#endif
 }
 
 /**

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


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