Re: [vhostmd 3/5] Activate virtio support in vhostmd

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

 



On 8/30/18 4:11 AM, Michael Trapp wrote:
Activate the virtio code and extend DTD/XML to support a virtio section
for virtio related configuration and a new transport type virtio.
---
  README              | 56 +++++++++++++++++++++++++++++++++++++++++++++------
  vhostmd.changes     |  5 +++++
  vhostmd.dtd         |  6 +++++-
  vhostmd.xml         |  6 ++++++
  vhostmd/Makefile.am |  2 +-
  vhostmd/vhostmd.c   | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
  6 files changed, 124 insertions(+), 9 deletions(-)

diff --git a/README b/README
index cb23226..44dbbd1 100644
--- a/README
+++ b/README
@@ -50,8 +50,15 @@ includes a few examples of user-defined metrics, which provide a
          <path>/dev/shm/vhostmd0</path>
          <size unit="k">256</size>
        </disk>
+      <virtio>
+        <max_channels>1024</max_channels>
+        <expiration_time>15</expiration_time>
+        <path>/var/lib/libvirt/qemu/channels</path>
+      </virtio>
        <update_period>5</update_period>
        <path>/usr/bin:/usr/sbin:/usr/share/vhostmd/scripts</path>
+      <transport>vbd</transport>
+      <transport>virtio</transport>

Hmm, in hindsight I suppose we could have structured the XML in such a way that information about the transport is embedded within <transport>. E.g. something like

<transport type='vbd>
  <disk>
    <name>host-metrics-disk</name>
    <path>/dev/shm/vhostmd0</path>
    <size unit="k">256</size>
  </disk>
</transport>
<transport type='virtio-serial'>
  ...
</transport>

But we weren't really expecting other transports and didn't consider the config syntax carefully when the second one was added. We can keep things simple and go with the virtio config you have here as long as it is documented.

      </globals>
      <metrics>
        <metric type="string" context="host">
@@ -116,10 +123,13 @@ includes a few examples of user-defined metrics, which provide a
A valid configuration file must contain the root element <vhostmd>.
  The <globals> element contains configuration global to vhostmd, such as
-the metrics disk path and the metrics refresh interval.  The <metrics>
-element is a container for all of the <metric> elements.  A metric element
-is used to define a metric, giving it a name and an action that produces
-the metric value.
+the metrics disk path and the metrics refresh interval. In addition
+metrics can also be read over QEMU virtio channels. Active transports
+can be specified by <transport> element.

And here is a good place to improve the documentation by doing a better job at tying the <transport> to the configuration for that transport. How about rewording this to something like

The <globals> element contains configuration global to vhostmd, such as the metrics refresh interval and the metrics transport mechanism. The <transport> element defines how the metrics are transported between the host and VMs. The vbd transport uses a virtual disk, described in the <disk> element, to share metrics data between host and VM. The virtio transport, described by the <virtio> element, uses a virtio-serial connection to share the metrics data.

?

+
+The <metrics> element is a container for all of the <metric> elements.
+A metric element is used to define a metric, giving it a name and an action
+that produces the metric value.
The supplied vhostmd configuration file provides a useful set of default
  metrics to be collected.  This can be extended or modified by editing
@@ -277,14 +287,48 @@ section:
  (Note: Change target dev and bus as appropriate for the domain)
+Virtio access to Metrics

Since in the end result is the same whether using vbd, xenstore, or virtio as the transport, this section should be titled "Notes on Virtio Transport" or similar.

+------------------------
+
+Basically for a virtio serial device, QEMU creates

And maybe a better introductory sentence. E.g.

The virtio transport uses a virtio serial device to transport metrics data between the host and VMs. Basically for a virtio serial device, QEMU creates...

Also I think this section is a good place to explain the various <virtio> transport settings.

+- a unix domain socket on the Host
+- a serial port on the VM
+- 'connects' both to a 'communication channel'
+
+
+Sample VM config with virtio serial:
+
+  <channel type='unix'>
+    <source mode='bind' path='/var/lib/libvirt/qemu/channels/\
+org.github.vhostmd.1.a70605c8-7d69-8c44-7e1a-5ecd092cb1e1'/>
+    <target type='virtio' name='vhostmd'/>
+    <address type='virtio-serial' controller='0' bus='0' port='1'/>
+  </channel>
+
+
+Vhostmd accepts metric requests 'GET /metrics/XML\n\n' and responds with
+
+  <metrics>
+    <metric type='real64' context='host'>
+      <name>TotalCPUTime</name>
+      <value>179645.910000</value>
+    </metric>
+  ...
+    <metric type='uint64' context='vm' id='9' uuid='a70605c8-7d69-8c44-7e1a-5ecd092cb1e1'>
+      <name>UsedMemory</name>
+      <value>524288</value>
+    </metric>
+  </metrics>
+
+
  Guest Tool/Library for Accessing Metrics Data
  ---------------------------------------------
Tool: vm_dump_metrics
   Stand alone static utility will read all the metrics and write them
   to stdout or optionally an argumented file.
- Usaga:
-   vhostmd [-f dest_file]
+ Usage:
+   vhostmd -d|-i|-x [-f dest_file]

Heh, you already fixed one typo (s/Usaga/Usage/), but isn't 'vhostmd' a typo as well? Should it be 'vm_dump_metrics'?

Library: libmetrics.so.0
   Dynamic library that supports individual metrics gathering
diff --git a/vhostmd.changes b/vhostmd.changes
index c7b453d..d32ba5a 100644
--- a/vhostmd.changes
+++ b/vhostmd.changes
@@ -1,3 +1,8 @@
+-------------------------------------------------------------------
+Wed Jul 25 10:00:20 CEST 2018 - michael.trapp@xxxxxxx
+
+- add virtio support

add virtio as a transport mechanism

+
  -------------------------------------------------------------------
  Mon Jun 29 16:42:39 MDT 2009 - jfehlig@xxxxxxxxxx
diff --git a/vhostmd.dtd b/vhostmd.dtd
index 471c72f..6480532 100644
--- a/vhostmd.dtd
+++ b/vhostmd.dtd
@@ -9,7 +9,7 @@ Virtual Host Metrics Daemon (vhostmd). Configuration file DTD
  -->
<!ELEMENT vhostmd (globals,metrics)>
-<!ELEMENT globals (disk,update_period,path,transport+)>
+<!ELEMENT globals (disk,virtio,update_period,path,transport+)>
<!ELEMENT disk (name,path,size)>
  <!ELEMENT name (#PCDATA)>
@@ -20,6 +20,10 @@ Virtual Host Metrics Daemon (vhostmd). Configuration file DTD
  <!ELEMENT update_period (#PCDATA)>
  <!ELEMENT transport (#PCDATA)>
+<!ELEMENT virtio (max_channels,expiration_time,path)>
+<!ELEMENT max_channels (#PCDATA)>
+<!ELEMENT expiration_time (#PCDATA)>
+
  <!ELEMENT metrics (metric*)>
  <!ELEMENT metric (name,action,variable*)>
  <!ELEMENT action (#PCDATA)>
diff --git a/vhostmd.xml b/vhostmd.xml
index 9b048df..5400981 100644
--- a/vhostmd.xml
+++ b/vhostmd.xml
@@ -33,9 +33,15 @@ the logical && operator must be replaced with "&amp;&amp;".
          <path>/dev/shm/vhostmd0</path>
          <size unit="k">256</size>
        </disk>
+      <virtio>
+        <max_channels>1024</max_channels>
+        <expiration_time>15</expiration_time>
+        <path>/var/lib/libvirt/qemu/channels</path>
+      </virtio>
        <path>/usr/sbin:/sbin:/usr/bin:/bin:/usr/share/vhostmd/scripts</path>
        <transport>vbd</transport>
+      <transport>virtio</transport>
        <!-- <transport>xenstore</transport> -->
      </globals>
      <metrics>
diff --git a/vhostmd/Makefile.am b/vhostmd/Makefile.am
index 3585970..860f019 100644
--- a/vhostmd/Makefile.am
+++ b/vhostmd/Makefile.am
@@ -3,7 +3,7 @@ INCLUDES = \
      -I../include
sbin_PROGRAMS = vhostmd
-vhostmd_SOURCES = vhostmd.c util.c metric.c virt-util.c
+vhostmd_SOURCES = vhostmd.c util.c metric.c virt-util.c virtio.c
  vhostmd_CFLAGS = $(LIBXML_CFLAGS) $(LIBVIRT_CFLAGS)
  vhostmd_LDADD = -lm $(LIBXML_LIBS) $(LIBVIRT_LIBS)
diff --git a/vhostmd/vhostmd.c b/vhostmd/vhostmd.c
index dc80345..e6a1283 100644
--- a/vhostmd/vhostmd.c
+++ b/vhostmd/vhostmd.c
@@ -41,10 +41,11 @@
  #include <sys/stat.h>
  #include <libxml/parser.h>
  #include <libxml/xpath.h>
+#include <pthread.h>
#include "util.h"
  #include "metric.h"
-
+#include "virtio.h"
/*
   * vhostmd will periodically write metrics to a disk.  The metrics
@@ -84,6 +85,7 @@ typedef struct _mdisk_header
   */
  #define VBD      (1 << 0)
  #define XENSTORE (1 << 1)
+#define VIRTIO   (1 << 2)
/* Global variables */
  static int down = 0;
@@ -102,6 +104,9 @@ static mdisk_header md_header =
           };
  static char *search_path = NULL;
  static int transports = 0;
+static char *virtio_path = "/var/lib/libvirt/qemu/channels";
+static unsigned virtio_max_channels = 1024;
+static unsigned virtio_expiration_time = 15;
/**********************************************************************
@@ -469,6 +474,8 @@ static int parse_transports(xmlDocPtr xml,
  	     return -1;
  #endif
  	 }
+         if (strncasecmp((char *)str, "virtio", strlen("virtio")) == 0)
+             transports |= VIRTIO;
           free(str);
        }
     }
@@ -604,6 +611,19 @@ static int parse_config_file(const char *filename)
        goto out;
     }
+ if (transports & VIRTIO) {
+      char *p = NULL;
+
+      if (vu_xpath_long("string(./globals/virtio/max_channels[1])", ctxt, &l) == 0)
+         virtio_max_channels = (unsigned)l;
+
+      if (vu_xpath_long("string(./globals/virtio/expiration_time[1])", ctxt, &l) == 0)
+         virtio_expiration_time = (unsigned)l;
+
+      if ((p = vu_xpath_string("string(./globals/virtio/path[1])", ctxt)) != NULL)
+         virtio_path = p;
+   }
+
     /* Parse requested metrics definitions */
     if (parse_metrics(xml, ctxt)) {
        vu_log(VHOSTMD_ERR, "Unable to parse metrics definition "
@@ -838,6 +858,8 @@ static int metrics_host_get(vu_buffer *buf)
  {
     metric *m = metrics;
+ unsigned start = buf->use;
+
     while (m) {
        if (m->ctx != METRIC_CONTEXT_HOST) {
           m = m->next;
@@ -849,6 +871,10 @@ static int metrics_host_get(vu_buffer *buf)
m = m->next;
     }
+
+   if (transports & VIRTIO)
+      virtio_metrics_update(&buf->content[start], buf->use - start, NULL, METRIC_CONTEXT_HOST);
+
     return 0;
  }
@@ -856,6 +882,8 @@ static int metrics_vm_get(vu_vm *vm, vu_buffer *buf)
  {
     metric *m = metrics;
+ unsigned start = buf->use;
+
     while (m) {
        if (m->ctx != METRIC_CONTEXT_VM) {
           m = m->next;
@@ -867,6 +895,10 @@ static int metrics_vm_get(vu_vm *vm, vu_buffer *buf)
m = m->next;
     }
+
+   if (transports & VIRTIO)
+      virtio_metrics_update(&buf->content[start], buf->use - start, vm->uuid, METRIC_CONTEXT_VM);
+
     return 0;
  }
@@ -911,11 +943,29 @@ static int vhostmd_run(int diskfd)
     int *ids = NULL;
     int num_vms = 0;
     vu_buffer *buf = NULL;
+   pthread_t  virtio_tid;
if (vu_buffer_create(&buf, MDISK_SIZE_MIN - MDISK_HEADER_SIZE)) {
        vu_log(VHOSTMD_ERR, "Unable to allocate memory");
        return -1;
     }
+
+   if (transports & VIRTIO) {
+      int rc;
+
+      if (virtio_expiration_time < (update_period * 3))

vhostmd.c:956:34: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
       if (virtio_expiration_time < (update_period * 3))

Regards,
Jim

+         virtio_expiration_time = (update_period * 3);
+
+      if (virtio_init(virtio_path, virtio_max_channels, virtio_expiration_time))
+         return -1;
+
+      rc = pthread_create(&virtio_tid, NULL, virtio_run, NULL);
+
+      if (rc != 0) {
+         vu_log(VHOSTMD_ERR, "Failed to start virtio thread '%s'\n", strerror(rc));
+         return -1;
+      }
+   }
while (!down) {
        vu_buffer_add(buf, "<metrics>\n", -1);
@@ -940,6 +990,12 @@ static int vhostmd_run(int diskfd)
        vu_buffer_erase(buf);
     }
     vu_buffer_delete(buf);
+
+   if (transports & VIRTIO) {
+      virtio_stop();
+      pthread_join(virtio_tid, NULL);
+   }
+
     return 0;
  }

_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list




[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux