src/virt.c: State notifications for all persistent domains
authorAntoine Naud <antoinex.naud@intel.com>
Thu, 14 Sep 2017 10:14:28 +0000 (11:14 +0100)
committerKrzysztof Kepka <krzysztof.kepka@intel.com>
Mon, 19 Feb 2018 11:48:32 +0000 (12:48 +0100)
When virt option PersistentNotification is enabled, the notifications of VMs
states are sent only for running VMs (whereas they should be sent for all
persistent VMs, including running, suspended, shut off VMs). This behaviour is
due to the fact that VM states are notified while fetching all other metrics,
which is executed only for running VMs.

This commit fixes this issue by decoupling the notifications of VMs states from
fetching and dispatching metrics values. In this purpose, a new function named
persistent_domains_state_notification is added to fetch the states (and reasons)
of persistent VMs and notify them. This function is called at each read interval
before fetching and dispatching the metrics from running VMs.

Unit Tests for the new functions are implemented.

Change-Id: Ic2befb3e7826184696344b037916bc603ec01341
Signed-off-by: Antoine Naud <antoinex.naud@intel.com>
Makefile.am
src/daemon/plugin_mock.c
src/virt.c
src/virt_test.c

index e37716f..dfd4b5e 100644 (file)
@@ -1762,14 +1762,15 @@ virt_la_LIBADD = libignorelist.la $(BUILD_WITH_LIBVIRT_LIBS) $(BUILD_WITH_LIBXML
 # the libvirt on wheezy is linked in libnl v1, and there is a small leak here,
 # triggered by the library initialization. There are no means to avoid it,
 # and libvirt switched to libnl3 anyway
-#test_plugin_virt_SOURCES = src/virt_test.c
-#test_plugin_virt_CPPFLAGS = $(AM_CPPFLAGS) \
-#      $(BUILD_WITH_LIBVIRT_CFLAGS) $(BUILD_WITH_LIBXML2_CFLAGS)
-#test_plugin_virt_LDFLAGS = $(PLUGIN_LDFLAGS)
-#test_plugin_virt_LDADD = libplugin_mock.la \
-#      $(BUILD_WITH_LIBVIRT_LIBS) $(BUILD_WITH_LIBXML2_LIBS)
-#check_PROGRAMS += test_plugin_virt
-#TESTS += test_plugin_virt
+test_plugin_virt_SOURCES = src/virt_test.c 
+test_plugin_virt_CPPFLAGS = $(AM_CPPFLAGS) \
+       $(BUILD_WITH_LIBVIRT_CPPFLAGS) $(BUILD_WITH_LIBXML2_CFLAGS)
+test_plugin_virt_LDFLAGS = $(PLUGIN_LDFLAGS) \
+       $(BUILD_WITH_LIBVIRT_LDFLAGS) $(BUILD_WITH_LIBXML2_LDFLAGS)
+test_plugin_virt_LDADD = libplugin_mock.la \
+       $(BUILD_WITH_LIBVIRT_LIBS) $(BUILD_WITH_LIBXML2_LIBS)
+check_PROGRAMS += test_plugin_virt
+TESTS += test_plugin_virt
 endif
 
 if BUILD_PLUGIN_VMEM
index 6df4c15..d16a641 100644 (file)
@@ -71,6 +71,24 @@ int plugin_register_data_set(const data_set_t *ds) { return ENOTSUP; }
 
 int plugin_dispatch_values(value_list_t const *vl) { return ENOTSUP; }
 
+int plugin_dispatch_notification(const notification_t *notif) { return ENOTSUP; }
+
+int plugin_notification_meta_add_string(notification_t *n, const char *name,
+                                        const char *value) { return ENOTSUP; }
+int plugin_notification_meta_add_signed_int(notification_t *n, const char *name,
+                                            int64_t value) { return ENOTSUP; }
+int plugin_notification_meta_add_unsigned_int(notification_t *n,
+                                              const char *name, uint64_t value) { return ENOTSUP; }
+int plugin_notification_meta_add_double(notification_t *n, const char *name,
+                                        double value) { return ENOTSUP; }
+int plugin_notification_meta_add_boolean(notification_t *n, const char *name,
+                                         _Bool value) { return ENOTSUP; }
+
+int plugin_notification_meta_copy(notification_t *dst,
+                                  const notification_t *src) { return ENOTSUP; }
+
+int plugin_notification_meta_free(notification_meta_t *n) { return ENOTSUP; }
+
 int plugin_flush(const char *plugin, cdtime_t timeout, const char *identifier) {
   return ENOTSUP;
 }
index f3d7a51..20336b4 100644 (file)
 #define HAVE_DOM_REASON_RUNNING_WAKEUP 1
 #endif
 
+/*
+  virConnectListAllDomains() appeared in 0.10.2
+  Note that LIBVIR_CHECK_VERSION appeared a year later, so
+  in some systems which actually have virConnectListAllDomains()
+  we can't detect this.
+ */
+#ifdef LIBVIR_CHECK_VERSION
+#if LIBVIR_CHECK_VERSION(0, 10, 2)
+#define HAVE_LIST_ALL_DOMAINS 1
+#endif
+#endif
+
 #if LIBVIR_CHECK_VERSION(1, 0, 1)
 #define HAVE_DOM_REASON_PAUSED_SNAPSHOT 1
 #endif
@@ -976,7 +988,7 @@ static unsigned int parse_ex_stats_flags(char **exstats, int numexstats) {
   return ex_stats_flags;
 }
 
-static void domain_state_submit(virDomainPtr dom, int state, int reason) {
+static void domain_state_submit_notif(virDomainPtr dom, int state, int reason) {
   if ((state < 0) || (state >= STATIC_ARRAY_SIZE(domain_states))) {
     ERROR(PLUGIN_NAME ": Array index out of bounds: state=%d", state);
     return;
@@ -1415,11 +1427,27 @@ static int get_domain_state(virDomainPtr domain) {
     return status;
   }
 
+  return status;
+}
+
+#ifdef HAVE_LIST_ALL_DOMAINS
+static int get_domain_state_notify(virDomainPtr domain) {
+  int domain_state = 0;
+  int domain_reason = 0;
+
+  int status = virDomainGetState(domain, &domain_state, &domain_reason, 0);
+  if (status != 0) {
+    ERROR(PLUGIN_NAME " plugin: virDomainGetState failed with status %i.",
+          status);
+    return status;
+  }
+
   if (persistent_notification)
-    domain_state_submit(domain, domain_state, domain_reason);
+    domain_state_submit_notif(domain, domain_state, domain_reason);
 
   return status;
 }
+#endif /* HAVE_LIST_ALL_DOMAINS */
 #endif /* HAVE_DOM_REASON */
 
 static int get_memory_stats(virDomainPtr domain) {
@@ -1671,11 +1699,6 @@ static int get_domain_metrics(domain_t *domain) {
      * We need to get it from virDomainGetState.
      */
     GET_STATS(get_domain_state, "domain reason", domain->ptr);
-#else
-    /* virDomainGetState is not available. Submit 0, which corresponds to
-     * unknown reason. */
-    if (persistent_notification)
-      domain_state_submit(domain->ptr, info.di.state, 0);
 #endif
   }
 
@@ -1768,7 +1791,7 @@ static int domain_lifecycle_event_cb(__attribute__((unused)) virConnectPtr conn,
                                      __attribute__((unused)) void *opaque) {
   int domain_state = map_domain_event_to_state(event);
   int domain_reason = map_domain_event_detail_to_reason(event, detail);
-  domain_state_submit(dom, domain_state, domain_reason);
+  domain_state_submit_notif(dom, domain_state, domain_reason);
 
   return 0;
 }
@@ -1831,6 +1854,77 @@ static void stop_event_loop(void) {
     virConnectDomainEventDeregisterAny(conn, domain_event_cb_id);
 }
 
+static int persistent_domains_state_notification(void) {
+  int status = 0;
+  int n;
+#ifdef HAVE_LIST_ALL_DOMAINS
+  virDomainPtr *domains;
+  n = virConnectListAllDomains(conn, &domains,
+                               VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT);
+  if (n < 0) {
+    VIRT_ERROR(conn, "reading list of persistent domains");
+    status = -1;
+  }
+  else {
+    DEBUG(PLUGIN_NAME " plugin: getting state of %i persistent domains", n);
+    /* Fetch each persistent domain's state and notify it */
+    int n_notified = n;
+    for (int i = 0; i < n; ++i) {
+      status = get_domain_state_notify(domains[i]);
+      if (status != 0) {
+        n_notified--;
+        ERROR(PLUGIN_NAME " plugin: could not notify state of domain %s",
+              virDomainGetName(domains[i]));
+      }
+    }
+
+    sfree(domains);
+    DEBUG(PLUGIN_NAME " plugin: notified state of %i persistent domains",
+          n_notified);
+  }
+#else
+  n = virConnectNumOfDomains(conn);
+  if (n > 0) {
+    int *domids;
+    /* Get list of domains. */
+    domids = malloc(sizeof(*domids) * n);
+    if (domids == NULL) {
+      ERROR(PLUGIN_NAME " plugin: malloc failed.");
+      return -1;
+    }
+    n = virConnectListDomains(conn, domids, n);
+    if (n < 0) {
+      VIRT_ERROR(conn, "reading list of domains");
+      sfree(domids);
+      return -1;
+    }
+    /* Fetch info of each active domain and notify it */
+    for (int i = 0; i < n; ++i) {
+      virDomainInfo info;
+      virDomainPtr dom = NULL;
+      dom = virDomainLookupByID(conn, domids[i]);
+      if (dom == NULL) {
+        VIRT_ERROR(conn, "virDomainLookupByID");
+        /* Could be that the domain went away -- ignore it anyway. */
+        continue;
+      }
+      status = virDomainGetInfo(dom, &info);
+      if (status != 0) {
+        ERROR(PLUGIN_NAME " plugin: virDomainGetInfo failed with status %i.",
+              status);
+        continue;
+      }
+      /* virDomainGetState is not available. Submit 0, which corresponds to
+       * unknown reason. */
+      domain_state_submit_notif(domain->ptr, info.di.state, 0);
+    }
+    sfree(domids);
+  }
+#endif
+
+  return status;
+}
+
 static int lv_read(user_data_t *ud) {
   time_t t;
   struct lv_read_instance *inst = NULL;
@@ -1864,6 +1958,12 @@ static int lv_read(user_data_t *ud) {
   /* Need to refresh domain or device lists? */
   if ((last_refresh == (time_t)0) ||
       ((interval > 0) && ((last_refresh + interval) <= t))) {
+    if (inst->id == 0 && persistent_notification) {
+      int status = persistent_domains_state_notification();
+      if (status != 0)
+        DEBUG(PLUGIN_NAME " plugin: persistent_domains_state_notifications " \
+              "returned with status %i", status);
+    }
     if (refresh_lists(inst) != 0) {
       if (inst->id == 0) {
         if (!persistent_notification)
@@ -2074,18 +2174,6 @@ static int lv_instance_include_domain(struct lv_read_instance *inst,
   return 0;
 }
 
-/*
-  virConnectListAllDomains() appeared in 0.10.2
-  Note that LIBVIR_CHECK_VERSION appeared a year later, so
-  in some systems which actually have virConnectListAllDomains()
-  we can't detect this.
- */
-#ifdef LIBVIR_CHECK_VERSION
-#if LIBVIR_CHECK_VERSION(0, 10, 2)
-#define HAVE_LIST_ALL_DOMAINS 1
-#endif
-#endif
-
 static int refresh_lists(struct lv_read_instance *inst) {
   struct lv_read_state *state = &inst->read_state;
   int n;
index 489a367..19ca88e 100644 (file)
 #include "testing.h"
 #include "virt.c" /* sic */
 
-#include <unistd.h>
+#ifdef HAVE_LIST_ALL_DOMAINS
 
-static const char minimal_xml[] =
-    ""
-    "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
-    "<domain type=\"kvm\" xmlns:ovirt=\"http://ovirt.org/vm/tune/1.0\">"
-    "  <metadata/>"
-    "</domain>";
+virDomainPtr *domains;
 
-static const char minimal_metadata_xml[] =
-    ""
-    "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
-    "<domain type=\"kvm\" xmlns:ovirt=\"http://ovirt.org/vm/tune/1.0\">"
-    "  <metadata>"
-    "    <ovirtmap:tag "
-    "xmlns:ovirtmap=\"http://ovirt.org/ovirtmap/tag/1.0\">virt-0</ovirtmap:tag>"
-    "  </metadata>"
-    "</domain>";
-
-struct xml_state {
-  xmlDocPtr xml_doc;
-  xmlXPathContextPtr xpath_ctx;
-  xmlXPathObjectPtr xpath_obj;
-  char tag[PARTITION_TAG_MAX_LEN];
-};
-
-static int init_state(struct xml_state *st, const char *xml) {
-  memset(st, 0, sizeof(*st));
-
-  st->xml_doc = xmlReadDoc((const xmlChar *)xml, NULL, NULL, XML_PARSE_NONET);
-  if (st->xml_doc == NULL) {
+static int setup(void)
+{
+  if (virInitialize() != 0) {
+    printf("ERROR: virInitialize() != 0\n");
     return -1;
   }
-  st->xpath_ctx = xmlXPathNewContext(st->xml_doc);
-  if (st->xpath_ctx == NULL) {
-    return -1;
-  }
-  return 0;
-}
 
-static void fini_state(struct xml_state *st) {
-  if (st->xpath_ctx) {
-    xmlXPathFreeContext(st->xpath_ctx);
-    st->xpath_ctx = NULL;
-  }
-  if (st->xml_doc) {
-    xmlFreeDoc(st->xml_doc);
-    st->xml_doc = NULL;
+  conn = virConnectOpen("test:///default");
+  if (conn == NULL) {
+    printf("ERROR: virConnectOpen == NULL\n");
+    return -1;
   }
-}
-
-#define TAG "virt-0"
-
-DEF_TEST(lv_domain_get_tag_no_metadata_xml) {
-  int err;
-  struct xml_state st;
-  err = init_state(&st, minimal_xml);
-  EXPECT_EQ_INT(0, err);
 
-  err = lv_domain_get_tag(st.xpath_ctx, "test", st.tag);
-
-  EXPECT_EQ_INT(0, err);
-  EXPECT_EQ_STR("", st.tag);
-
-  fini_state(&st);
   return 0;
 }
 
-DEF_TEST(lv_domain_get_tag_valid_xml) {
-  int err;
-  struct xml_state st;
-  err = init_state(&st, minimal_metadata_xml);
-  EXPECT_EQ_INT(0, err);
-
-  err = lv_domain_get_tag(st.xpath_ctx, "test", st.tag);
-
-  EXPECT_EQ_INT(0, err);
-  EXPECT_EQ_STR(TAG, st.tag);
+static int teardown(void)
+{
+  sfree(domains);
+  if (conn != NULL)
+    virConnectClose(conn);
 
   return 0;
 }
 
-DEF_TEST(lv_default_instance_include_domain_without_tag) {
-  struct lv_read_instance *inst = NULL;
-  int ret;
-
-  ret = lv_init_instance(0, lv_read);
-  inst = &(lv_read_user_data[0].inst);
-  EXPECT_EQ_STR("virt-0", inst->tag);
-
-  ret = lv_instance_include_domain(inst, "testing", "");
-  EXPECT_EQ_INT(1, ret);
-
-  lv_fini_instance(0);
-  return 0;
-}
-
-DEF_TEST(lv_regular_instance_skip_domain_without_tag) {
-  struct lv_read_instance *inst = NULL;
-  int ret;
-
-  ret = lv_init_instance(1, lv_read);
-  inst = &(lv_read_user_data[1].inst);
-  EXPECT_EQ_STR("virt-1", inst->tag);
+DEF_TEST(get_domain_state_notify) {
+  if (setup() == 0) {
+    int n_domains = virConnectListAllDomains(conn, &domains, VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT);
+    if (n_domains <= 0) {
+      printf("ERROR: virConnectListAllDomains: n_domains <= 0\n");
+      return -1;
+    }
 
-  ret = lv_instance_include_domain(inst, "testing", "");
-  EXPECT_EQ_INT(0, ret);
-
-  lv_fini_instance(0);
-  return 0;
-}
-
-DEF_TEST(lv_include_domain_matching_tags) {
-  struct lv_read_instance *inst = NULL;
-  int ret;
-
-  ret = lv_init_instance(0, lv_read);
-  inst = &(lv_read_user_data[0].inst);
-  EXPECT_EQ_STR("virt-0", inst->tag);
-
-  ret = lv_instance_include_domain(inst, "testing", "virt-0");
-  EXPECT_EQ_INT(1, ret);
-
-  ret = lv_init_instance(1, lv_read);
-  inst = &(lv_read_user_data[1].inst);
-  EXPECT_EQ_STR("virt-1", inst->tag);
-
-  ret = lv_instance_include_domain(inst, "testing", "virt-1");
-  EXPECT_EQ_INT(1, ret);
-
-  lv_fini_instance(0);
-  lv_fini_instance(1);
-  return 0;
-}
-
-DEF_TEST(lv_default_instance_include_domain_with_unknown_tag) {
-  struct lv_read_instance *inst = NULL;
-  int ret;
-
-  ret = lv_init_instance(0, lv_read);
-  inst = &(lv_read_user_data[0].inst);
-  EXPECT_EQ_STR("virt-0", inst->tag);
-
-  ret = lv_instance_include_domain(inst, "testing", "unknownFormat-tag");
-  EXPECT_EQ_INT(1, ret);
-
-  lv_fini_instance(0);
+    int ret = get_domain_state_notify(domains[0]);
+    EXPECT_EQ_INT(0, ret);
+  }
+  teardown();
+  
   return 0;
 }
 
-DEF_TEST(lv_regular_instance_skip_domain_with_unknown_tag) {
-  struct lv_read_instance *inst = NULL;
-  int ret;
-
-  ret = lv_init_instance(1, lv_read);
-  inst = &(lv_read_user_data[1].inst);
-  EXPECT_EQ_STR("virt-1", inst->tag);
-
-  ret = lv_instance_include_domain(inst, "testing", "unknownFormat-tag");
-  EXPECT_EQ_INT(0, ret);
-
-  lv_fini_instance(0);
+DEF_TEST(persistent_domains_state_notification) {
+  if (setup() == 0) {
+    int ret = persistent_domains_state_notification();
+    EXPECT_EQ_INT(0, ret);
+  }
+  teardown();
+  
   return 0;
 }
-#undef TAG
+#endif
 
 int main(void) {
-  RUN_TEST(lv_domain_get_tag_no_metadata_xml);
-  RUN_TEST(lv_domain_get_tag_valid_xml);
-
-  RUN_TEST(lv_default_instance_include_domain_without_tag);
-  RUN_TEST(lv_regular_instance_skip_domain_without_tag);
-  RUN_TEST(lv_include_domain_matching_tags);
-  RUN_TEST(lv_default_instance_include_domain_with_unknown_tag);
-  RUN_TEST(lv_regular_instance_skip_domain_with_unknown_tag);
+#ifdef HAVE_LIST_ALL_DOMAINS
+  RUN_TEST(get_domain_state_notify);
+  RUN_TEST(persistent_domains_state_notification);
+#endif
 
   END_TEST;
 }
-
-/* vim: set sw=2 sts=2 et : */