[PATCH v2 0/4] Testing libvirt XML -> libxl_domain_config conversion

[PATCH v2 0/4] Testing libvirt XML -> libxl_domain_config conversion

[Daniel P. Berrange]

Version 2 of:

  https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html

This tests the conversion of libvirt XML to libxl_domain_config
objects by the libvirt libxl driver.

Changed in v2:

 - Compare the parsed JSON object model instead of strcmp() on
   the string-ified JSON document

 - To cope with new additions between Xen 4.3 and 4.4

    - Ignore case where actual JSON object has gained new keys.
      This copes with fact that '/c_info' gained a new key
      called 'pvh' in 4.4

    - Ignore case where actual JSON value has changed from 'null'
      to a non-'null' value type. This copes with fact that
      the element at /b_info/device_model_version changed from 
      'null' to a specific value by default in 4.4.

 - Use libxl_domain_config_to_json instead of libxl_json.h
   functions, and disable test if running on Xen version
   which lacks this function (eg 4.2)

 - Isolate VNC port allocator from host TCP ports so a predictable
   VNC port is always in the config.

Daniel P. Berrange (4):
  util: Introduce virJSONStringCompare for JSON doc comparisons
  util: Allow port allocator to skip bind() check
  tests: Add more test suite mock helpers
  libxl: Add a test suite for libxl option generator

 configure.ac                         |   2 +
 src/libvirt_private.syms             |   1 +
 src/libxl/libxl_driver.c             |   3 +-
 src/qemu/qemu_driver.c               |   9 +-
 src/util/virjson.c                   | 185 ++++++++++++++++++++++++++++++++++
 src/util/virjson.h                   |  22 +++++
 src/util/virportallocator.c          |  14 ++-
 src/util/virportallocator.h          |   7 +-
 tests/Makefile.am                    |  25 ++++-
 tests/libxlxml2jsondata/minimal.json | 172 ++++++++++++++++++++++++++++++++
 tests/libxlxml2jsondata/minimal.xml  |  36 +++++++
 tests/libxlxml2jsontest.c            | 186 +++++++++++++++++++++++++++++++++++
 tests/virfirewalltest.c              |   4 +-
 tests/virmock.h                      |  54 +++++++---
 tests/virmocklibxl.c                 |  87 ++++++++++++++++
 tests/virportallocatortest.c         |   4 +-
 tests/virsystemdtest.c               |   4 +-
 17 files changed, 786 insertions(+), 29 deletions(-)
 create mode 100644 tests/libxlxml2jsondata/minimal.json
 create mode 100644 tests/libxlxml2jsondata/minimal.xml
 create mode 100644 tests/libxlxml2jsontest.c
 create mode 100644 tests/virmocklibxl.c

-- 
1.9.3

[libvirt] [PATCH v2 1/4] util: Introduce virJSONStringCompare for JSON doc comparisons

[Daniel P. Berrange]

Comparing JSON docs using strcmp is simple, but is not flexible
as it is sensitive to whitespace used in the doc generation.
When comparing objects it may also be desirable to treat the
existance of keys in the actual object but not expected object
as non-fatal. Introduce a virJSONStringCompare function which
takes two strings representing expected and actual JSON docs
and then does a DOM comparison.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 src/libvirt_private.syms |   1 +
 src/util/virjson.c       | 185 +++++++++++++++++++++++++++++++++++++++++++++++
 src/util/virjson.h       |  22 ++++++
 3 files changed, 208 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4c1f61c..5f2b9d9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1406,6 +1406,7 @@ virISCSIScanTargets;
 
 
 # util/virjson.h
+virJSONStringCompare;
 virJSONValueArrayAppend;
 virJSONValueArrayGet;
 virJSONValueArraySize;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 35a8252..0a6d1be 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -47,6 +47,11 @@
 
 VIR_LOG_INIT("util.json");
 
+VIR_ENUM_DECL(virJSONType)
+VIR_ENUM_IMPL(virJSONType, VIR_JSON_TYPE_LAST,
+              "object", "array", "string",
+              "number", "boolean", "null")
+
 typedef struct _virJSONParserState virJSONParserState;
 typedef virJSONParserState *virJSONParserStatePtr;
 struct _virJSONParserState {
@@ -90,6 +95,7 @@ void virJSONValueFree(virJSONValuePtr value)
         break;
     case VIR_JSON_TYPE_BOOLEAN:
     case VIR_JSON_TYPE_NULL:
+    case VIR_JSON_TYPE_LAST:
         break;
     }
 
@@ -1152,3 +1158,182 @@ char *virJSONValueToString(virJSONValuePtr object ATTRIBUTE_UNUSED,
     return NULL;
 }
 #endif
+
+
+static bool
+virJSONValueCompare(virJSONValuePtr expect,
+                    virJSONValuePtr actual,
+                    const char *context,
+                    unsigned int flags)
+{
+    size_t i, j;
+    if (expect->type != actual->type) {
+        if (expect->type == VIR_JSON_TYPE_NULL &&
+            (flags & VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL))
+            return true;
+
+        const char *expectType = virJSONTypeTypeToString(expect->type);
+        const char *actualType = virJSONTypeTypeToString(actual->type);
+
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Expected value type '%s' but got value type '%s' at '%s'"),
+                       expectType, actualType, context);
+        return false;
+    }
+
+    switch (expect->type) {
+    case VIR_JSON_TYPE_OBJECT:
+        for (i = 0; i < expect->data.object.npairs; i++) {
+            bool found = false;
+            char *childcontext;
+            for (j = 0; j < actual->data.object.npairs; j++) {
+                if (STREQ(expect->data.object.pairs[i].key,
+                          actual->data.object.pairs[j].key)) {
+                    found = true;
+                    break;
+                }
+            }
+            if (!found) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Expected object key '%s' not found in actual object at '%s'"),
+                               expect->data.object.pairs[i].key, context);
+                return false;
+            }
+
+            if (virAsprintf(&childcontext, "%s%s%s",
+                            context,
+                            STREQ(context, "/") ? "" : "/",
+                            expect->data.object.pairs[i].key) < 0)
+                return false;
+
+            if (!virJSONValueCompare(expect->data.object.pairs[i].value,
+                                     actual->data.object.pairs[j].value,
+                                     childcontext,
+                                     flags)) {
+                VIR_FREE(childcontext);
+                return false;
+            }
+            VIR_FREE(childcontext);
+        }
+        if (!(flags & VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS)) {
+            for (i = 0; i < actual->data.object.npairs; i++) {
+                bool found = false;
+                char *childcontext;
+                for (j = 0; j < expect->data.object.npairs; j++) {
+                    if (STREQ(actual->data.object.pairs[i].key,
+                              expect->data.object.pairs[j].key)) {
+                        found = true;
+                        break;
+                    }
+                }
+                if (!found) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR,
+                                   _("Actual object key '%s' not found in expected object at '%s'"),
+                                   actual->data.object.pairs[i].key, context);
+                    return false;
+                }
+
+                if (virAsprintf(&childcontext, "%s%s%s",
+                                STREQ(context, "/") ? "" : "/",
+                                context, actual->data.object.pairs[i].key) < 0)
+                    return false;
+
+                if (!virJSONValueCompare(actual->data.object.pairs[i].value,
+                                         expect->data.object.pairs[j].value,
+                                         childcontext,
+                                         flags)) {
+                    VIR_FREE(childcontext);
+                    return false;
+                }
+                VIR_FREE(childcontext);
+            }
+        }
+        break;
+    case VIR_JSON_TYPE_ARRAY:
+        if (expect->data.array.nvalues !=
+            actual->data.array.nvalues) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Expected array size '%zu' but got size '%zu' at '%s'"),
+                           expect->data.array.nvalues,
+                           actual->data.array.nvalues,
+                           context);
+            return false;
+        }
+
+        for (i = 0; i < expect->data.array.nvalues; i++) {
+            char *childcontext;
+            if (virAsprintf(&childcontext, "%s%s[%zu]",
+                            STREQ(context, "/") ? "" : "/",
+                            context, i) < 0)
+                return false;
+
+            if (!virJSONValueCompare(expect->data.array.values[i],
+                                     actual->data.array.values[i],
+                                     childcontext,
+                                     flags)) {
+                VIR_FREE(childcontext);
+                return false;
+            }
+            VIR_FREE(childcontext);
+        }
+        break;
+    case VIR_JSON_TYPE_STRING:
+        if (STRNEQ(expect->data.string,
+                   actual->data.string)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Expected string value '%s' but got '%s' at '%s'"),
+                           expect->data.string, actual->data.string, context);
+            return false;
+        }
+        break;
+    case VIR_JSON_TYPE_NUMBER:
+        if (STRNEQ(expect->data.number,
+                   actual->data.number)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Expected number value '%s' but got '%s' at '%s'"),
+                           expect->data.number, actual->data.number, context);
+            return false;
+        }
+        break;
+    case VIR_JSON_TYPE_BOOLEAN:
+        if (expect->data.boolean !=
+            actual->data.boolean) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Expected bool value '%d' but got '%d' at '%s'"),
+                           expect->data.boolean, actual->data.boolean, context);
+            return false;
+        }
+        break;
+    case VIR_JSON_TYPE_NULL:
+        /* trivially equal */
+        break;
+
+    case VIR_JSON_TYPE_LAST:
+        /* nothing */
+        break;
+    }
+
+    return true;
+}
+
+
+bool virJSONStringCompare(const char *expect,
+                          const char *actual,
+                          unsigned int flags)
+{
+    virJSONValuePtr expectVal = NULL;
+    virJSONValuePtr actualVal = NULL;
+    int ret = false;
+
+    if (!(expectVal = virJSONValueFromString(expect)))
+        goto cleanup;
+    if (!(actualVal = virJSONValueFromString(actual)))
+        goto cleanup;
+
+    ret = virJSONValueCompare(expectVal, actualVal, "/", flags);
+
+ cleanup:
+    virJSONValueFree(expectVal);
+    virJSONValueFree(actualVal);
+    return ret;
+}
diff --git a/src/util/virjson.h b/src/util/virjson.h
index db11396..b1f96d3 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -34,6 +34,8 @@ typedef enum {
     VIR_JSON_TYPE_NUMBER,
     VIR_JSON_TYPE_BOOLEAN,
     VIR_JSON_TYPE_NULL,
+
+    VIR_JSON_TYPE_LAST,
 } virJSONType;
 
 typedef struct _virJSONValue virJSONValue;
@@ -139,4 +141,24 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring);
 char *virJSONValueToString(virJSONValuePtr object,
                            bool pretty);
 
+typedef enum {
+    /*
+     * When comparing a pair of Objects, if the 'actual' object
+     * has keys that are not present in the 'expected' object,
+     * the existance of these extra keys will be non-fatal.
+     */
+    VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS = (1 << 0),
+
+    /*
+     * when comparing two values, if their types are different,
+     * and the 'expected' value type is 'null', then this will
+     * be considered non-fatal.
+     */
+    VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL = (1 << 1),
+} virJSONCompareFlags;
+
+bool virJSONStringCompare(const char *expect,
+                          const char *actual,
+                          unsigned int flags);
+
 #endif /* __VIR_JSON_H_ */
-- 
1.9.3

Re: [libvirt] [PATCH v2 1/4] util: Introduce virJSONStringCompare for JSON doc comparisons

[Jim Fehlig]

Daniel P. Berrange wrote:
> Comparing JSON docs using strcmp is simple, but is not flexible
> as it is sensitive to whitespace used in the doc generation.
> When comparing objects it may also be desirable to treat the
> existance of keys in the actual object but not expected object
> as non-fatal. Introduce a virJSONStringCompare function which
> takes two strings representing expected and actual JSON docs
> and then does a DOM comparison.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  src/libvirt_private.syms |   1 +
>  src/util/virjson.c       | 185 +++++++++++++++++++++++++++++++++++++++++++++++
>   

I'm not too familiar with this file, but didn't notice any problems with
this patch beyond a little nit below.

>  src/util/virjson.h       |  22 ++++++
>  3 files changed, 208 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 4c1f61c..5f2b9d9 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1406,6 +1406,7 @@ virISCSIScanTargets;
>  
>  
>  # util/virjson.h
> +virJSONStringCompare;
>  virJSONValueArrayAppend;
>  virJSONValueArrayGet;
>  virJSONValueArraySize;
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 35a8252..0a6d1be 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -47,6 +47,11 @@
>  
>  VIR_LOG_INIT("util.json");
>  
> +VIR_ENUM_DECL(virJSONType)
> +VIR_ENUM_IMPL(virJSONType, VIR_JSON_TYPE_LAST,
> +              "object", "array", "string",
> +              "number", "boolean", "null")
> +
>  typedef struct _virJSONParserState virJSONParserState;
>  typedef virJSONParserState *virJSONParserStatePtr;
>  struct _virJSONParserState {
> @@ -90,6 +95,7 @@ void virJSONValueFree(virJSONValuePtr value)
>          break;
>      case VIR_JSON_TYPE_BOOLEAN:
>      case VIR_JSON_TYPE_NULL:
> +    case VIR_JSON_TYPE_LAST:
>          break;
>      }
>  
> @@ -1152,3 +1158,182 @@ char *virJSONValueToString(virJSONValuePtr object ATTRIBUTE_UNUSED,
>      return NULL;
>  }
>  #endif
> +
> +
> +static bool
> +virJSONValueCompare(virJSONValuePtr expect,
> +                    virJSONValuePtr actual,
> +                    const char *context,
> +                    unsigned int flags)
> +{
> +    size_t i, j;
> +    if (expect->type != actual->type) {
>   

Super nit: seems most of libvirt code would have a line between local
variable declarations and start of code, but this file is not consistent
in that regard anyhow.

ACK.

Regards,
Jim

> +        if (expect->type == VIR_JSON_TYPE_NULL &&
> +            (flags & VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL))
> +            return true;
> +
> +        const char *expectType = virJSONTypeTypeToString(expect->type);
> +        const char *actualType = virJSONTypeTypeToString(actual->type);
> +
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Expected value type '%s' but got value type '%s' at '%s'"),
> +                       expectType, actualType, context);
> +        return false;
> +    }
> +
> +    switch (expect->type) {
> +    case VIR_JSON_TYPE_OBJECT:
> +        for (i = 0; i < expect->data.object.npairs; i++) {
> +            bool found = false;
> +            char *childcontext;
> +            for (j = 0; j < actual->data.object.npairs; j++) {
> +                if (STREQ(expect->data.object.pairs[i].key,
> +                          actual->data.object.pairs[j].key)) {
> +                    found = true;
> +                    break;
> +                }
> +            }
> +            if (!found) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Expected object key '%s' not found in actual object at '%s'"),
> +                               expect->data.object.pairs[i].key, context);
> +                return false;
> +            }
> +
> +            if (virAsprintf(&childcontext, "%s%s%s",
> +                            context,
> +                            STREQ(context, "/") ? "" : "/",
> +                            expect->data.object.pairs[i].key) < 0)
> +                return false;
> +
> +            if (!virJSONValueCompare(expect->data.object.pairs[i].value,
> +                                     actual->data.object.pairs[j].value,
> +                                     childcontext,
> +                                     flags)) {
> +                VIR_FREE(childcontext);
> +                return false;
> +            }
> +            VIR_FREE(childcontext);
> +        }
> +        if (!(flags & VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS)) {
> +            for (i = 0; i < actual->data.object.npairs; i++) {
> +                bool found = false;
> +                char *childcontext;
> +                for (j = 0; j < expect->data.object.npairs; j++) {
> +                    if (STREQ(actual->data.object.pairs[i].key,
> +                              expect->data.object.pairs[j].key)) {
> +                        found = true;
> +                        break;
> +                    }
> +                }
> +                if (!found) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("Actual object key '%s' not found in expected object at '%s'"),
> +                                   actual->data.object.pairs[i].key, context);
> +                    return false;
> +                }
> +
> +                if (virAsprintf(&childcontext, "%s%s%s",
> +                                STREQ(context, "/") ? "" : "/",
> +                                context, actual->data.object.pairs[i].key) < 0)
> +                    return false;
> +
> +                if (!virJSONValueCompare(actual->data.object.pairs[i].value,
> +                                         expect->data.object.pairs[j].value,
> +                                         childcontext,
> +                                         flags)) {
> +                    VIR_FREE(childcontext);
> +                    return false;
> +                }
> +                VIR_FREE(childcontext);
> +            }
> +        }
> +        break;
> +    case VIR_JSON_TYPE_ARRAY:
> +        if (expect->data.array.nvalues !=
> +            actual->data.array.nvalues) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Expected array size '%zu' but got size '%zu' at '%s'"),
> +                           expect->data.array.nvalues,
> +                           actual->data.array.nvalues,
> +                           context);
> +            return false;
> +        }
> +
> +        for (i = 0; i < expect->data.array.nvalues; i++) {
> +            char *childcontext;
> +            if (virAsprintf(&childcontext, "%s%s[%zu]",
> +                            STREQ(context, "/") ? "" : "/",
> +                            context, i) < 0)
> +                return false;
> +
> +            if (!virJSONValueCompare(expect->data.array.values[i],
> +                                     actual->data.array.values[i],
> +                                     childcontext,
> +                                     flags)) {
> +                VIR_FREE(childcontext);
> +                return false;
> +            }
> +            VIR_FREE(childcontext);
> +        }
> +        break;
> +    case VIR_JSON_TYPE_STRING:
> +        if (STRNEQ(expect->data.string,
> +                   actual->data.string)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Expected string value '%s' but got '%s' at '%s'"),
> +                           expect->data.string, actual->data.string, context);
> +            return false;
> +        }
> +        break;
> +    case VIR_JSON_TYPE_NUMBER:
> +        if (STRNEQ(expect->data.number,
> +                   actual->data.number)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Expected number value '%s' but got '%s' at '%s'"),
> +                           expect->data.number, actual->data.number, context);
> +            return false;
> +        }
> +        break;
> +    case VIR_JSON_TYPE_BOOLEAN:
> +        if (expect->data.boolean !=
> +            actual->data.boolean) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Expected bool value '%d' but got '%d' at '%s'"),
> +                           expect->data.boolean, actual->data.boolean, context);
> +            return false;
> +        }
> +        break;
> +    case VIR_JSON_TYPE_NULL:
> +        /* trivially equal */
> +        break;
> +
> +    case VIR_JSON_TYPE_LAST:
> +        /* nothing */
> +        break;
> +    }
> +
> +    return true;
> +}
> +
> +
> +bool virJSONStringCompare(const char *expect,
> +                          const char *actual,
> +                          unsigned int flags)
> +{
> +    virJSONValuePtr expectVal = NULL;
> +    virJSONValuePtr actualVal = NULL;
> +    int ret = false;
> +
> +    if (!(expectVal = virJSONValueFromString(expect)))
> +        goto cleanup;
> +    if (!(actualVal = virJSONValueFromString(actual)))
> +        goto cleanup;
> +
> +    ret = virJSONValueCompare(expectVal, actualVal, "/", flags);
> +
> + cleanup:
> +    virJSONValueFree(expectVal);
> +    virJSONValueFree(actualVal);
> +    return ret;
> +}
> diff --git a/src/util/virjson.h b/src/util/virjson.h
> index db11396..b1f96d3 100644
> --- a/src/util/virjson.h
> +++ b/src/util/virjson.h
> @@ -34,6 +34,8 @@ typedef enum {
>      VIR_JSON_TYPE_NUMBER,
>      VIR_JSON_TYPE_BOOLEAN,
>      VIR_JSON_TYPE_NULL,
> +
> +    VIR_JSON_TYPE_LAST,
>  } virJSONType;
>  
>  typedef struct _virJSONValue virJSONValue;
> @@ -139,4 +141,24 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring);
>  char *virJSONValueToString(virJSONValuePtr object,
>                             bool pretty);
>  
> +typedef enum {
> +    /*
> +     * When comparing a pair of Objects, if the 'actual' object
> +     * has keys that are not present in the 'expected' object,
> +     * the existance of these extra keys will be non-fatal.
> +     */
> +    VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS = (1 << 0),
> +
> +    /*
> +     * when comparing two values, if their types are different,
> +     * and the 'expected' value type is 'null', then this will
> +     * be considered non-fatal.
> +     */
> +    VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL = (1 << 1),
> +} virJSONCompareFlags;
> +
> +bool virJSONStringCompare(const char *expect,
> +                          const char *actual,
> +                          unsigned int flags);
> +
>  #endif /* __VIR_JSON_H_ */
>   

[PATCH v2 2/4] util: Allow port allocator to skip bind() check

[Daniel P. Berrange]

Test suites using the port allocator don't want to have different
behaviour depending on whether a port is in use on the host. Add
a VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK which test suites can use
to skip the bind() test. The port allocator will thus only track
ports in use by the test suite process itself. This is fine when
using the port allocator to generate guest configs which won't
actually be launched

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 src/libxl/libxl_driver.c     |  3 ++-
 src/qemu/qemu_driver.c       |  9 ++++++---
 src/util/virportallocator.c  | 14 ++++++++++----
 src/util/virportallocator.h  |  7 ++++++-
 tests/virportallocatortest.c |  4 ++--
 5 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 515d5c9..70f9bb8 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -298,7 +298,8 @@ libxlStateInitialize(bool privileged,
     if (!(libxl_driver->reservedVNCPorts =
           virPortAllocatorNew(_("VNC"),
                               LIBXL_VNC_PORT_MIN,
-                              LIBXL_VNC_PORT_MAX)))
+                              LIBXL_VNC_PORT_MAX,
+                              0)))
         goto error;
 
     if (!(libxl_driver->domains = virDomainObjListNew()))
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a7622a..b3a9036 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -680,19 +680,22 @@ qemuStateInitialize(bool privileged,
     if ((qemu_driver->remotePorts =
          virPortAllocatorNew(_("display"),
                              cfg->remotePortMin,
-                             cfg->remotePortMax)) == NULL)
+                             cfg->remotePortMax,
+                             0)) == NULL)
         goto error;
 
     if ((qemu_driver->webSocketPorts =
          virPortAllocatorNew(_("webSocket"),
                              cfg->webSocketPortMin,
-                             cfg->webSocketPortMax)) == NULL)
+                             cfg->webSocketPortMax,
+                             0)) == NULL)
         goto error;
 
     if ((qemu_driver->migrationPorts =
          virPortAllocatorNew(_("migration"),
                              cfg->migrationPortMin,
-                             cfg->migrationPortMax)) == NULL)
+                             cfg->migrationPortMax,
+                             0)) == NULL)
         goto error;
 
     if (qemuSecurityInit(qemu_driver) < 0)
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index b68133a..fa17475 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -43,6 +43,8 @@ struct _virPortAllocator {
 
     unsigned short start;
     unsigned short end;
+
+    unsigned int flags;
 };
 
 static virClassPtr virPortAllocatorClass;
@@ -71,7 +73,8 @@ VIR_ONCE_GLOBAL_INIT(virPortAllocator)
 
 virPortAllocatorPtr virPortAllocatorNew(const char *name,
                                         unsigned short start,
-                                        unsigned short end)
+                                        unsigned short end,
+                                        unsigned int flags)
 {
     virPortAllocatorPtr pa;
 
@@ -87,6 +90,7 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
     if (!(pa = virObjectLockableNew(virPortAllocatorClass)))
         return NULL;
 
+    pa->flags = flags;
     pa->start = start;
     pa->end = end;
 
@@ -193,9 +197,11 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
         if (used)
             continue;
 
-        if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
-            virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
-            goto cleanup;
+        if (!(pa->flags & VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK)) {
+            if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
+                virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
+                goto cleanup;
+        }
 
         if (!used && !v6used) {
             /* Add port to bitmap of reserved ports */
diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h
index c8aa6de..2fdb8d9 100644
--- a/src/util/virportallocator.h
+++ b/src/util/virportallocator.h
@@ -28,9 +28,14 @@
 typedef struct _virPortAllocator virPortAllocator;
 typedef virPortAllocator *virPortAllocatorPtr;
 
+typedef enum {
+    VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK = (1 << 0),
+} virPortAllocatorFlags;
+
 virPortAllocatorPtr virPortAllocatorNew(const char *name,
                                         unsigned short start,
-                                        unsigned short end);
+                                        unsigned short end,
+                                        unsigned int flags);
 
 int virPortAllocatorAcquire(virPortAllocatorPtr pa,
                             unsigned short *port);
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 48d2c9a..96d2ade 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -122,7 +122,7 @@ VIR_LOG_INIT("tests.portallocatortest");
 
 static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
 {
-    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5909);
+    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5909, 0);
     int ret = -1;
     unsigned short p1, p2, p3, p4, p5, p6, p7;
 
@@ -193,7 +193,7 @@ static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
 
 static int testAllocReuse(const void *args ATTRIBUTE_UNUSED)
 {
-    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5910);
+    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5910, 0);
     int ret = -1;
     unsigned short p1, p2, p3, p4;
 
-- 
1.9.3

Re: [libvirt] [PATCH v2 2/4] util: Allow port allocator to skip bind() check

[Jim Fehlig]

Daniel P. Berrange wrote:
> Test suites using the port allocator don't want to have different
> behaviour depending on whether a port is in use on the host. Add
> a VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK which test suites can use
> to skip the bind() test. The port allocator will thus only track
> ports in use by the test suite process itself. This is fine when
> using the port allocator to generate guest configs which won't
> actually be launched
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  src/libxl/libxl_driver.c     |  3 ++-
>  src/qemu/qemu_driver.c       |  9 ++++++---
>  src/util/virportallocator.c  | 14 ++++++++++----
>  src/util/virportallocator.h  |  7 ++++++-
>  tests/virportallocatortest.c |  4 ++--
>  5 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 515d5c9..70f9bb8 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -298,7 +298,8 @@ libxlStateInitialize(bool privileged,
>      if (!(libxl_driver->reservedVNCPorts =
>            virPortAllocatorNew(_("VNC"),
>                                LIBXL_VNC_PORT_MIN,
> -                              LIBXL_VNC_PORT_MAX)))
> +                              LIBXL_VNC_PORT_MAX,
> +                              0)))
>          goto error;
>  
>      if (!(libxl_driver->domains = virDomainObjListNew()))
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3a7622a..b3a9036 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -680,19 +680,22 @@ qemuStateInitialize(bool privileged,
>      if ((qemu_driver->remotePorts =
>           virPortAllocatorNew(_("display"),
>                               cfg->remotePortMin,
> -                             cfg->remotePortMax)) == NULL)
> +                             cfg->remotePortMax,
> +                             0)) == NULL)
>          goto error;
>  
>      if ((qemu_driver->webSocketPorts =
>           virPortAllocatorNew(_("webSocket"),
>                               cfg->webSocketPortMin,
> -                             cfg->webSocketPortMax)) == NULL)
> +                             cfg->webSocketPortMax,
> +                             0)) == NULL)
>          goto error;
>  
>      if ((qemu_driver->migrationPorts =
>           virPortAllocatorNew(_("migration"),
>                               cfg->migrationPortMin,
> -                             cfg->migrationPortMax)) == NULL)
> +                             cfg->migrationPortMax,
> +                             0)) == NULL)
>          goto error;
>  
>      if (qemuSecurityInit(qemu_driver) < 0)
> diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
> index b68133a..fa17475 100644
> --- a/src/util/virportallocator.c
> +++ b/src/util/virportallocator.c
> @@ -43,6 +43,8 @@ struct _virPortAllocator {
>  
>      unsigned short start;
>      unsigned short end;
> +
> +    unsigned int flags;
>  };
>  
>  static virClassPtr virPortAllocatorClass;
> @@ -71,7 +73,8 @@ VIR_ONCE_GLOBAL_INIT(virPortAllocator)
>  
>  virPortAllocatorPtr virPortAllocatorNew(const char *name,
>                                          unsigned short start,
> -                                        unsigned short end)
> +                                        unsigned short end,
> +                                        unsigned int flags)
>  {
>      virPortAllocatorPtr pa;
>  
> @@ -87,6 +90,7 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
>      if (!(pa = virObjectLockableNew(virPortAllocatorClass)))
>          return NULL;
>  
> +    pa->flags = flags;
>      pa->start = start;
>      pa->end = end;
>  
> @@ -193,9 +197,11 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
>          if (used)
>              continue;
>  
> -        if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
> -            virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
> -            goto cleanup;
> +        if (!(pa->flags & VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK)) {
> +            if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
> +                virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
> +                goto cleanup;
> +        }
>  
>          if (!used && !v6used) {
>              /* Add port to bitmap of reserved ports */
> diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h
> index c8aa6de..2fdb8d9 100644
> --- a/src/util/virportallocator.h
> +++ b/src/util/virportallocator.h
> @@ -28,9 +28,14 @@
>  typedef struct _virPortAllocator virPortAllocator;
>  typedef virPortAllocator *virPortAllocatorPtr;
>  
> +typedef enum {
> +    VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK = (1 << 0),
> +} virPortAllocatorFlags;
> +
>  virPortAllocatorPtr virPortAllocatorNew(const char *name,
>                                          unsigned short start,
> -                                        unsigned short end);
> +                                        unsigned short end,
> +                                        unsigned int flags);
>  
>  int virPortAllocatorAcquire(virPortAllocatorPtr pa,
>                              unsigned short *port);
> diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
> index 48d2c9a..96d2ade 100644
> --- a/tests/virportallocatortest.c
> +++ b/tests/virportallocatortest.c
> @@ -122,7 +122,7 @@ VIR_LOG_INIT("tests.portallocatortest");
>  
>  static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
>  {
> -    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5909);
> +    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5909, 0);
>   

Should these tests be changed to use the new
VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK flag?

Regards,
Jim

>      int ret = -1;
>      unsigned short p1, p2, p3, p4, p5, p6, p7;
>  
> @@ -193,7 +193,7 @@ static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
>  
>  static int testAllocReuse(const void *args ATTRIBUTE_UNUSED)
>  {
> -    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5910);
> +    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5910, 0);
>      int ret = -1;
>      unsigned short p1, p2, p3, p4;
>  
>   

[PATCH v2 3/4] tests: Add more test suite mock helpers

[Daniel P. Berrange]

Rename the VIR_MOCK_IMPL* macros to VIR_MOCK_WRAP*
and add new VIR_MOCK_IMPL macros which let you directly
implement overrides in the preloaded source.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 tests/virfirewalltest.c |  4 ++--
 tests/virmock.h         | 54 +++++++++++++++++++++++++++++++++++++------------
 tests/virsystemdtest.c  |  4 ++--
 3 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index ba2e6ad..81c5557 100644
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -67,7 +67,7 @@ static bool fwError;
     "target     prot opt source               destination\n"
 
 # if WITH_DBUS
-VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
+VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
                        DBusMessage *,
                        DBusConnection *, connection,
                        DBusMessage *, message,
@@ -82,7 +82,7 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
     char **args = NULL;
     char *type = NULL;
 
-    VIR_MOCK_IMPL_INIT_REAL(dbus_connection_send_with_reply_and_block);
+    VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
 
     if (STREQ(service, "org.freedesktop.DBus") &&
         STREQ(member, "ListNames")) {
diff --git a/tests/virmock.h b/tests/virmock.h
index 0dd8bb5..8352e30 100644
--- a/tests/virmock.h
+++ b/tests/virmock.h
@@ -234,33 +234,61 @@
  */
 
 # define VIR_MOCK_IMPL_RET_ARGS(name, rettype, ...)                     \
+    rettype name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));                   \
+    static rettype (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));      \
+    rettype name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
+
+# define VIR_MOCK_IMPL_RET_VOID(name, rettype)                          \
+    rettype name(void);                                                 \
+    static rettype (*real_##name)(void);                                \
+    rettype name(void)
+
+# define VIR_MOCK_IMPL_VOID_ARGS(name, ...)                             \
+    void name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));                      \
+    static void (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));         \
+    void name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
+
+# define VIR_MOCK_IMPL_VOID_VOID(name)                                  \
+    void name(void);                                                    \
+    static void (*real_##name)(void);                                   \
+    void name(void)
+
+/*
+ * The VIR_MOCK_WRAP_NNN_MMM() macros are intended for use in the
+ * individual test suites. The define a stub implementation of
+ * the wrapped method and insert the caller provided code snippet
+ * as the body of the method.
+ */
+
+# define VIR_MOCK_WRAP_RET_ARGS(name, rettype, ...)                     \
     rettype wrap_##name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));            \
     static rettype (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));      \
     rettype wrap_##name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
 
-# define VIR_MOCK_IMPL_INIT_REAL(name)                                  \
-    do {                                                                \
-        if (real_##name == NULL &&                                      \
-            !(real_##name = dlsym(RTLD_NEXT,                            \
-                                  #name))) {                            \
-            fprintf(stderr, "Missing symbol '" #name "'\n");            \
-            abort();                                                    \
-        }                                                               \
-    } while (0)
-
-# define VIR_MOCK_IMPL_RET_VOID(name, rettype)                          \
+# define VIR_MOCK_WRAP_RET_VOID(name, rettype)                          \
     rettype wrap_##name(void);                                          \
     static rettype (*real_##name)(void);                                \
     rettype wrap_##name(void)
 
-# define VIR_MOCK_IMPL_VOID_ARGS(name, ...)                             \
+# define VIR_MOCK_WRAP_VOID_ARGS(name, ...)                             \
     void wrap_##name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));               \
     static void (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));         \
     void wrap_##name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
 
-# define VIR_MOCK_IMPL_VOID_VOID(name)                                  \
+# define VIR_MOCK_WRAP_VOID_VOID(name)                                  \
     void wrap_##name(void);                                             \
     static void (*real_##name)(void);                                   \
     void wrap_##name(void)
 
+
+# define VIR_MOCK_REAL_INIT(name)                                       \
+    do {                                                                \
+        if (real_##name == NULL &&                                      \
+            !(real_##name = dlsym(RTLD_NEXT,                            \
+                                  #name))) {                            \
+            fprintf(stderr, "Missing symbol '" #name "'\n");            \
+            abort();                                                    \
+        }                                                               \
+    } while (0)
+
 #endif /* __VIR_MOCK_H__ */
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 8f7b47e..0d57a6a 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -34,7 +34,7 @@
 
 VIR_LOG_INIT("tests.systemdtest");
 
-VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
+VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
                        DBusMessage *,
                        DBusConnection *, connection,
                        DBusMessage *, message,
@@ -45,7 +45,7 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
     const char *service = dbus_message_get_destination(message);
     const char *member = dbus_message_get_member(message);
 
-    VIR_MOCK_IMPL_INIT_REAL(dbus_connection_send_with_reply_and_block);
+    VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
 
     if (STREQ(service, "org.freedesktop.machine1")) {
         if (getenv("FAIL_BAD_SERVICE")) {
-- 
1.9.3

Re: [libvirt] [PATCH v2 3/4] tests: Add more test suite mock helpers

[Jim Fehlig]

Daniel P. Berrange wrote:
> Rename the VIR_MOCK_IMPL* macros to VIR_MOCK_WRAP*
> and add new VIR_MOCK_IMPL macros which let you directly
> implement overrides in the preloaded source.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  tests/virfirewalltest.c |  4 ++--
>  tests/virmock.h         | 54 +++++++++++++++++++++++++++++++++++++------------
>  tests/virsystemdtest.c  |  4 ++--
>  3 files changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
> index ba2e6ad..81c5557 100644
> --- a/tests/virfirewalltest.c
> +++ b/tests/virfirewalltest.c
> @@ -67,7 +67,7 @@ static bool fwError;
>      "target     prot opt source               destination\n"
>  
>  # if WITH_DBUS
> -VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
> +VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
>                         DBusMessage *,
>                         DBusConnection *, connection,
>                         DBusMessage *, message,
> @@ -82,7 +82,7 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
>      char **args = NULL;
>      char *type = NULL;
>  
> -    VIR_MOCK_IMPL_INIT_REAL(dbus_connection_send_with_reply_and_block);
> +    VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
>  
>      if (STREQ(service, "org.freedesktop.DBus") &&
>          STREQ(member, "ListNames")) {
> diff --git a/tests/virmock.h b/tests/virmock.h
> index 0dd8bb5..8352e30 100644
> --- a/tests/virmock.h
> +++ b/tests/virmock.h
> @@ -234,33 +234,61 @@
>   */
>  
>  # define VIR_MOCK_IMPL_RET_ARGS(name, rettype, ...)                     \
> +    rettype name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));                   \
> +    static rettype (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));      \
> +    rettype name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
> +
> +# define VIR_MOCK_IMPL_RET_VOID(name, rettype)                          \
> +    rettype name(void);                                                 \
> +    static rettype (*real_##name)(void);                                \
> +    rettype name(void)
> +
> +# define VIR_MOCK_IMPL_VOID_ARGS(name, ...)                             \
> +    void name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));                      \
> +    static void (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));         \
> +    void name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
> +
> +# define VIR_MOCK_IMPL_VOID_VOID(name)                                  \
> +    void name(void);                                                    \
> +    static void (*real_##name)(void);                                   \
> +    void name(void)
> +
> +/*
> + * The VIR_MOCK_WRAP_NNN_MMM() macros are intended for use in the
> + * individual test suites. The define a stub implementation of
>   

s/The/They/ or s/The/The macros/

This is the first time I've looked at virmock.h in any detail, but
didn't notice any problems with this patch.  ACK.

Regards,
Jim

> + * the wrapped method and insert the caller provided code snippet
> + * as the body of the method.
> + */
> +
> +# define VIR_MOCK_WRAP_RET_ARGS(name, rettype, ...)                     \
>      rettype wrap_##name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));            \
>      static rettype (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));      \
>      rettype wrap_##name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
>  
> -# define VIR_MOCK_IMPL_INIT_REAL(name)                                  \
> -    do {                                                                \
> -        if (real_##name == NULL &&                                      \
> -            !(real_##name = dlsym(RTLD_NEXT,                            \
> -                                  #name))) {                            \
> -            fprintf(stderr, "Missing symbol '" #name "'\n");            \
> -            abort();                                                    \
> -        }                                                               \
> -    } while (0)
> -
> -# define VIR_MOCK_IMPL_RET_VOID(name, rettype)                          \
> +# define VIR_MOCK_WRAP_RET_VOID(name, rettype)                          \
>      rettype wrap_##name(void);                                          \
>      static rettype (*real_##name)(void);                                \
>      rettype wrap_##name(void)
>  
> -# define VIR_MOCK_IMPL_VOID_ARGS(name, ...)                             \
> +# define VIR_MOCK_WRAP_VOID_ARGS(name, ...)                             \
>      void wrap_##name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));               \
>      static void (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));         \
>      void wrap_##name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
>  
> -# define VIR_MOCK_IMPL_VOID_VOID(name)                                  \
> +# define VIR_MOCK_WRAP_VOID_VOID(name)                                  \
>      void wrap_##name(void);                                             \
>      static void (*real_##name)(void);                                   \
>      void wrap_##name(void)
>  
> +
> +# define VIR_MOCK_REAL_INIT(name)                                       \
> +    do {                                                                \
> +        if (real_##name == NULL &&                                      \
> +            !(real_##name = dlsym(RTLD_NEXT,                            \
> +                                  #name))) {                            \
> +            fprintf(stderr, "Missing symbol '" #name "'\n");            \
> +            abort();                                                    \
> +        }                                                               \
> +    } while (0)
> +
>  #endif /* __VIR_MOCK_H__ */
> diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
> index 8f7b47e..0d57a6a 100644
> --- a/tests/virsystemdtest.c
> +++ b/tests/virsystemdtest.c
> @@ -34,7 +34,7 @@
>  
>  VIR_LOG_INIT("tests.systemdtest");
>  
> -VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
> +VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
>                         DBusMessage *,
>                         DBusConnection *, connection,
>                         DBusMessage *, message,
> @@ -45,7 +45,7 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
>      const char *service = dbus_message_get_destination(message);
>      const char *member = dbus_message_get_member(message);
>  
> -    VIR_MOCK_IMPL_INIT_REAL(dbus_connection_send_with_reply_and_block);
> +    VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
>  
>      if (STREQ(service, "org.freedesktop.machine1")) {
>          if (getenv("FAIL_BAD_SERVICE")) {
>   

[PATCH v2 4/4] libxl: Add a test suite for libxl option generator

[Daniel P. Berrange]

The libxl library allows a libxl_domain_config object to be
serialized to a JSON string. Use this to allow testing of
the XML -> libxl_domain_config conversion process

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 configure.ac                         |   2 +
 tests/Makefile.am                    |  25 ++++-
 tests/libxlxml2jsondata/minimal.json | 172 ++++++++++++++++++++++++++++++++
 tests/libxlxml2jsondata/minimal.xml  |  36 +++++++
 tests/libxlxml2jsontest.c            | 186 +++++++++++++++++++++++++++++++++++
 tests/virmocklibxl.c                 |  87 ++++++++++++++++
 6 files changed, 507 insertions(+), 1 deletion(-)
 create mode 100644 tests/libxlxml2jsondata/minimal.json
 create mode 100644 tests/libxlxml2jsondata/minimal.xml
 create mode 100644 tests/libxlxml2jsontest.c
 create mode 100644 tests/virmocklibxl.c

diff --git a/configure.ac b/configure.ac
index 368d094..e87be85 100644
--- a/configure.ac
+++ b/configure.ac
@@ -875,6 +875,8 @@ if test "$with_libxl" != "no" ; then
     AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [
         with_libxl=yes
         LIBXL_LIBS="$LIBXL_LIBS -lxenlight -lxenctrl"
+        LIBS="$LIBS -lxenlight -lxenctrl"
+        AC_CHECK_FUNCS([libxl_domain_config_to_json])
     ],[
         if test "$with_libxl" = "yes"; then
             fail=1
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f9f2b84..742503d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST =		\
 	domainsnapshotxml2xmlout \
 	fchostdata \
 	interfaceschemadata \
+	libxlxml2jsondata \
 	lxcconf2xmldata \
 	lxcxml2xmldata \
 	lxcxml2xmloutdata \
@@ -216,6 +217,9 @@ if WITH_XEN
 test_programs += xml2sexprtest sexpr2xmltest \
 	xmconfigtest xencapstest statstest reconnect
 endif WITH_XEN
+if WITH_LIBXL
+test_programs += libxlxml2jsontest
+endif WITH_LIBXL
 if WITH_QEMU
 test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
 	qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
@@ -378,7 +382,9 @@ test_libraries += libqemumonitortestutils.la \
 		qemuxml2argvmock.la \
 		$(NULL)
 endif WITH_QEMU
-
+if WITH_LIBXL
+test_libraries += virmocklibxl.la
+endif WITH_LIBXL
 if WITH_BHYVE
 test_libraries += bhyvexml2argvmock.la
 endif WITH_BHYVE
@@ -577,6 +583,23 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
 	$(QEMUMONITORTESTUTILS_SOURCES)
 endif ! WITH_QEMU
 
+if WITH_LIBXL
+libxl_LDADDS = ../src/libvirt_driver_libxl_impl.la
+libxl_LDADDS += $(LDADDS)
+
+libxlxml2jsontest_SOURCES = \
+	libxlxml2jsontest.c testutilsxen.c testutilsxen.h \
+	testutils.c testutils.h
+libxlxml2jsontest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS)
+
+virmocklibxl_la_SOURCES = \
+	virmocklibxl.c
+virmocklibxl_la_CFLAGS = $(AM_CFLAGS)
+virmocklibxl_la_LDFLAGS = -module -avoid-version \
+        -rpath /evil/libtool/hack/to/force/shared/lib/creation
+
+endif WITH_LIBXL
+
 if WITH_LXC
 
 lxc_LDADDS = ../src/libvirt_driver_lxc_impl.la
diff --git a/tests/libxlxml2jsondata/minimal.json b/tests/libxlxml2jsondata/minimal.json
new file mode 100644
index 0000000..930e1d8
--- /dev/null
+++ b/tests/libxlxml2jsondata/minimal.json
@@ -0,0 +1,172 @@
+{
+    "c_info": {
+        "type": "pv",
+        "hap": "<default>",
+        "oos": "<default>",
+        "ssidref": 0,
+        "name": "rhel5pv",
+        "uuid": "8f07fe28-753f-2729-d76d-bdbd892f949a",
+        "xsdata": {
+
+        },
+        "platformdata": {
+
+        },
+        "poolid": 0,
+        "run_hotplug_scripts": "<default>"
+    },
+    "b_info": {
+        "max_vcpus": 4,
+        "avail_vcpus": [
+            0,
+            1,
+            2,
+            3
+        ],
+        "cpumap": [
+
+        ],
+        "nodemap": [
+
+        ],
+        "numa_placement": "<default>",
+        "tsc_mode": "default",
+        "max_memkb": 2560000,
+        "target_memkb": 307200,
+        "video_memkb": -1,
+        "shadow_memkb": -1,
+        "rtc_timeoffset": 0,
+        "exec_ssidref": 0,
+        "localtime": "<default>",
+        "disable_migrate": "<default>",
+        "cpuid": [
+
+        ],
+        "blkdev_start": null,
+        "device_model_version": null,
+        "device_model_stubdomain": "<default>",
+        "device_model": null,
+        "device_model_ssidref": 0,
+        "extra": [
+
+        ],
+        "extra_pv": [
+
+        ],
+        "extra_hvm": [
+
+        ],
+        "sched_params": {
+            "sched": "unknown",
+            "weight": 1000,
+            "cap": -1,
+            "period": -1,
+            "slice": -1,
+            "latency": -1,
+            "extratime": -1
+        },
+        "ioports": [
+
+        ],
+        "irqs": [
+
+        ],
+        "iomem": [
+
+        ],
+        "claim_mode": "<default>",
+        "u": {
+            "kernel": null,
+            "slack_memkb": -1,
+            "bootloader": "/usr/bin/pygrub",
+            "bootloader_args": [
+
+            ],
+            "cmdline": null,
+            "ramdisk": null,
+            "e820_host": "<default>"
+        }
+    },
+    "disks": [
+        {
+            "backend_domid": 0,
+            "backend_domname": null,
+            "pdev_path": "/var/lib/xen/images/rhel5pv.img",
+            "vdev": "xvda",
+            "backend": "tap",
+            "format": "raw",
+            "script": null,
+            "removable": 1,
+            "readwrite": 1,
+            "is_cdrom": 0
+        },
+        {
+            "backend_domid": 0,
+            "backend_domname": null,
+            "pdev_path": "/root/qcow1-xen.img",
+            "vdev": "xvdd",
+            "backend": "qdisk",
+            "format": "qcow",
+            "script": null,
+            "removable": 1,
+            "readwrite": 1,
+            "is_cdrom": 0
+        }
+    ],
+    "nics": [
+        {
+            "backend_domid": 0,
+            "backend_domname": null,
+            "devid": 0,
+            "mtu": 0,
+            "model": null,
+            "mac": "00:16:3e:60:36:ba",
+            "ip": null,
+            "bridge": "xenbr0",
+            "ifname": null,
+            "script": null,
+            "nictype": "vif",
+            "rate_bytes_per_interval": 0,
+            "rate_interval_usecs": 0,
+            "gatewaydev": null
+        }
+    ],
+    "pcidevs": [
+
+    ],
+    "vfbs": [
+        {
+            "backend_domid": 0,
+            "backend_domname": null,
+            "devid": -1,
+            "vnc": {
+                "enable": "True",
+                "listen": "0.0.0.0",
+                "passwd": null,
+                "display": 0,
+                "findunused": "False"
+            },
+            "sdl": {
+                "enable": "<default>",
+                "opengl": "<default>",
+                "display": null,
+                "xauthority": null
+            },
+            "keymap": null
+        }
+    ],
+    "vkbs": [
+        {
+            "backend_domid": 0,
+            "backend_domname": null,
+            "devid": -1
+        }
+    ],
+    "vtpms": [
+
+    ],
+    "on_poweroff": null,
+    "on_reboot": "destroy",
+    "on_watchdog": null,
+    "on_crash": "destroy"
+}
diff --git a/tests/libxlxml2jsondata/minimal.xml b/tests/libxlxml2jsondata/minimal.xml
new file mode 100644
index 0000000..1361028
--- /dev/null
+++ b/tests/libxlxml2jsondata/minimal.xml
@@ -0,0 +1,36 @@
+<domain type='xen'>
+  <name>rhel5pv</name>
+  <uuid>8f07fe28-753f-2729-d76d-bdbd892f949a</uuid>
+  <memory>2560000</memory>
+  <currentMemory>307200</currentMemory>
+  <vcpu>4</vcpu>
+  <bootloader>/usr/bin/pygrub</bootloader>
+  <os>
+    <type arch='i686' machine='xenpv'>linux</type>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <disk type='file' device='disk'>
+      <driver name='tap' type='aio'/>
+      <source file='/var/lib/xen/images/rhel5pv.img'/>
+      <target dev='xvda' bus='xen'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <driver name='tap' type='qcow'/>
+      <source file='/root/qcow1-xen.img'/>
+      <target dev='xvdd' bus='xen'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3e:60:36:ba'/>
+      <source bridge='xenbr0'/>
+    </interface>
+    <console type='pty'>
+      <target port='0'/>
+    </console>
+    <input type='mouse' bus='xen'/>
+    <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'/>
+  </devices>
+</domain>
diff --git a/tests/libxlxml2jsontest.c b/tests/libxlxml2jsontest.c
new file mode 100644
index 0000000..a0f7d88
--- /dev/null
+++ b/tests/libxlxml2jsontest.c
@@ -0,0 +1,186 @@
+/*
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange@redhat.com>
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <sys/types.h>
+#include <fcntl.h>
+
+#include "testutils.h"
+
+#if defined(WITH_LIBXL) && defined(WITH_YAJL) && defined(HAVE_LIBXL_DOMAIN_CONFIG_TO_JSON)
+
+# include "internal.h"
+# include "viralloc.h"
+# include "libxl/libxl_conf.h"
+# include "datatypes.h"
+# include "virstring.h"
+# include "virmock.h"
+# include "virjson.h"
+# include "testutilsxen.h"
+
+# define VIR_FROM_THIS VIR_FROM_XEN
+
+static const char *abs_top_srcdir;
+static virCapsPtr xencaps;
+
+static int testCompareXMLToJSONFiles(const char *xml,
+                                     const char *cmdline)
+{
+    char *expectargv = NULL;
+    int ret = -1;
+    virDomainDefPtr vmdef = NULL;
+    virPortAllocatorPtr gports = NULL;
+    libxl_ctx *ctx = NULL;
+    libxl_domain_config config;
+    xentoollog_logger *log = NULL;
+    virDomainXMLOptionPtr xmlopt = NULL;
+    char *actualargv;
+
+    libxl_domain_config_init(&config);
+
+    if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, XTL_DEBUG, 0)))
+        goto cleanup;
+
+    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, log) < 0)
+        goto cleanup;
+
+    if (!(gports = virPortAllocatorNew("vnc", 5900, 6000,
+                                       VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK)))
+        goto cleanup;
+
+    if (!(xmlopt = libxlCreateXMLConf()))
+        goto cleanup;
+
+    if (!(vmdef = virDomainDefParseFile(xml, xencaps, xmlopt,
+                                        1 << VIR_DOMAIN_VIRT_XEN,
+                                        VIR_DOMAIN_XML_INACTIVE)))
+        goto cleanup;
+
+    if (libxlBuildDomainConfig(gports, vmdef, ctx, &config) < 0)
+        goto cleanup;
+
+    if (!(actualargv = libxl_domain_config_to_json(ctx, &config))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       "Failed to create JSON doc for xl config");
+        goto cleanup;
+    }
+
+    virtTestLoadFile(cmdline, &expectargv);
+
+    if (!virJSONStringCompare(expectargv, actualargv,
+                              VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS |
+                              VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL))
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(expectargv);
+    VIR_FREE(actualargv);
+    virDomainDefFree(vmdef);
+    virObjectUnref(gports);
+    virObjectUnref(xmlopt);
+    libxl_ctx_free(ctx);
+    libxl_domain_config_dispose(&config);
+    xtl_logger_destroy(log);
+    return ret;
+}
+
+
+struct testInfo {
+    const char *name;
+};
+
+static int
+testCompareXMLToJSONHelper(const void *data)
+{
+    int ret = -1;
+    const struct testInfo *info = data;
+    char *xml = NULL;
+    char *args = NULL;
+
+    if (virAsprintf(&xml, "%s/libxlxml2jsondata/%s.xml",
+                    abs_srcdir, info->name) < 0 ||
+        virAsprintf(&args, "%s/libxlxml2jsondata/%s.json",
+                    abs_srcdir, info->name) < 0)
+        goto cleanup;
+
+    ret = testCompareXMLToJSONFiles(xml, args);
+
+ cleanup:
+    VIR_FREE(xml);
+    VIR_FREE(args);
+    return ret;
+}
+
+
+static int
+mymain(void)
+{
+    int ret = 0;
+
+    abs_top_srcdir = getenv("abs_top_srcdir");
+    if (!abs_top_srcdir)
+        abs_top_srcdir = abs_srcdir "/..";
+
+    /* Set the timezone because we are mocking the time() function.
+     * If we don't do that, then localtime() may return unpredictable
+     * results. In order to detect things that just work by a blind
+     * chance, we need to set an virtual timezone that no libvirt
+     * developer resides in. */
+    if (setenv("TZ", "VIR00:30", 1) < 0) {
+        perror("setenv");
+        return EXIT_FAILURE;
+    }
+
+    if ((xencaps = testXenCapsInit()) == NULL)
+        return EXIT_FAILURE;
+
+# define DO_TEST(name)                                                  \
+    do {                                                                \
+        static struct testInfo info = {                                 \
+            name,                                                       \
+        };                                                              \
+        if (virtTestRun("LibXL XML-2-JSON " name,                       \
+                        testCompareXMLToJSONHelper, &info) < 0)         \
+            ret = -1;                                                   \
+    } while (0)
+
+    DO_TEST("minimal");
+
+    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmocklibxl.so")
+
+#else
+
+int main(void)
+{
+    return EXIT_AM_SKIP;
+}
+
+#endif /* WITH_XEN && WITH_YAJL && HAVE_LIBXL_DOMAIN_CONFIG_TO_JSON */
diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
new file mode 100644
index 0000000..bc4b53d
--- /dev/null
+++ b/tests/virmocklibxl.c
@@ -0,0 +1,87 @@
+/*
+ * virmocklibxl.c: mocking of xenstore/libxs for libxl
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange@redhat.com>
+ */
+
+#include <config.h>
+
+#if defined(WITH_LIBXL) && defined(WITH_YAJL)
+# include "virmock.h"
+# include <sys/stat.h>
+# include <unistd.h>
+# include <libxl.h>
+# include <xenstore.h>
+# include <xenctrl.h>
+
+VIR_MOCK_IMPL_RET_VOID(xs_daemon_open,
+                       struct xs_handle *)
+{
+    VIR_MOCK_REAL_INIT(xs_daemon_open);
+    return (void*)0x1;
+}
+
+VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
+                       xc_interface *,
+                       xentoollog_logger *, logger,
+                       xentoollog_logger *, dombuild_logger,
+                       unsigned, open_flags)
+{
+    VIR_MOCK_REAL_INIT(xc_interface_open);
+    return (void*)0x1;
+}
+
+
+VIR_MOCK_STUB_RET_ARGS(xc_interface_close,
+                       int, 0,
+                       xc_interface *, handle)
+
+VIR_MOCK_STUB_VOID_ARGS(xs_daemon_close,
+                        struct xs_handle *, handle)
+
+VIR_MOCK_IMPL_RET_ARGS(__xstat, int,
+                       int, ver,
+                       const char *, path,
+                       struct stat *, sb)
+{
+    VIR_MOCK_REAL_INIT(__xstat);
+
+    if (strstr(path, "xenstored.pid")) {
+        memset(sb, 0, sizeof(*sb));
+        return 0;
+    }
+
+    return real___xstat(ver, path, sb);
+}
+
+VIR_MOCK_IMPL_RET_ARGS(stat, int,
+                       const char *, path,
+                       struct stat *, sb)
+{
+    VIR_MOCK_REAL_INIT(stat);
+
+    if (strstr(path, "xenstored.pid")) {
+        memset(sb, 0, sizeof(*sb));
+        return 0;
+    }
+
+    return real_stat(path, sb);
+}
+
+#endif /* WITH_LIBXL && WITH_YAJL */
-- 
1.9.3

Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

[Jim Fehlig]

Daniel P. Berrange wrote:
> The libxl library allows a libxl_domain_config object to be
> serialized to a JSON string. Use this to allow testing of
> the XML -> libxl_domain_config conversion process
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  configure.ac                         |   2 +
>  tests/Makefile.am                    |  25 ++++-
>  tests/libxlxml2jsondata/minimal.json | 172 ++++++++++++++++++++++++++++++++
>  tests/libxlxml2jsondata/minimal.xml  |  36 +++++++
>  tests/libxlxml2jsontest.c            | 186 +++++++++++++++++++++++++++++++++++
>  tests/virmocklibxl.c                 |  87 ++++++++++++++++
>  6 files changed, 507 insertions(+), 1 deletion(-)
>  create mode 100644 tests/libxlxml2jsondata/minimal.json
>  create mode 100644 tests/libxlxml2jsondata/minimal.xml
>  create mode 100644 tests/libxlxml2jsontest.c
>  create mode 100644 tests/virmocklibxl.c
>
> diff --git a/configure.ac b/configure.ac
> index 368d094..e87be85 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -875,6 +875,8 @@ if test "$with_libxl" != "no" ; then
>      AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [
>          with_libxl=yes
>          LIBXL_LIBS="$LIBXL_LIBS -lxenlight -lxenctrl"
> +        LIBS="$LIBS -lxenlight -lxenctrl"
> +        AC_CHECK_FUNCS([libxl_domain_config_to_json])
>   

This function exists in Xen 4.2 as well, in libxl.h.

>      ],[
>          if test "$with_libxl" = "yes"; then
>              fail=1
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index f9f2b84..742503d 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -82,6 +82,7 @@ EXTRA_DIST =		\
>  	domainsnapshotxml2xmlout \
>  	fchostdata \
>  	interfaceschemadata \
> +	libxlxml2jsondata \
>  	lxcconf2xmldata \
>  	lxcxml2xmldata \
>  	lxcxml2xmloutdata \
> @@ -216,6 +217,9 @@ if WITH_XEN
>  test_programs += xml2sexprtest sexpr2xmltest \
>  	xmconfigtest xencapstest statstest reconnect
>  endif WITH_XEN
> +if WITH_LIBXL
> +test_programs += libxlxml2jsontest
> +endif WITH_LIBXL
>  if WITH_QEMU
>  test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
>  	qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
> @@ -378,7 +382,9 @@ test_libraries += libqemumonitortestutils.la \
>  		qemuxml2argvmock.la \
>  		$(NULL)
>  endif WITH_QEMU
> -
> +if WITH_LIBXL
> +test_libraries += virmocklibxl.la
> +endif WITH_LIBXL
>  if WITH_BHYVE
>  test_libraries += bhyvexml2argvmock.la
>  endif WITH_BHYVE
> @@ -577,6 +583,23 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
>  	$(QEMUMONITORTESTUTILS_SOURCES)
>  endif ! WITH_QEMU
>  
> +if WITH_LIBXL
> +libxl_LDADDS = ../src/libvirt_driver_libxl_impl.la
> +libxl_LDADDS += $(LDADDS)
> +
> +libxlxml2jsontest_SOURCES = \
> +	libxlxml2jsontest.c testutilsxen.c testutilsxen.h \
> +	testutils.c testutils.h
> +libxlxml2jsontest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS)
> +
> +virmocklibxl_la_SOURCES = \
> +	virmocklibxl.c
> +virmocklibxl_la_CFLAGS = $(AM_CFLAGS)
> +virmocklibxl_la_LDFLAGS = -module -avoid-version \
> +        -rpath /evil/libtool/hack/to/force/shared/lib/creation
> +
> +endif WITH_LIBXL
> +
>  if WITH_LXC
>  
>  lxc_LDADDS = ../src/libvirt_driver_lxc_impl.la
> diff --git a/tests/libxlxml2jsondata/minimal.json b/tests/libxlxml2jsondata/minimal.json
> new file mode 100644
> index 0000000..930e1d8
> --- /dev/null
> +++ b/tests/libxlxml2jsondata/minimal.json
> @@ -0,0 +1,172 @@
> +{
> +    "c_info": {
> +        "type": "pv",
> +        "hap": "<default>",
> +        "oos": "<default>",
> +        "ssidref": 0,
> +        "name": "rhel5pv",
> +        "uuid": "8f07fe28-753f-2729-d76d-bdbd892f949a",
> +        "xsdata": {
> +
> +        },
> +        "platformdata": {
> +
> +        },
> +        "poolid": 0,
> +        "run_hotplug_scripts": "<default>"
> +    },
> +    "b_info": {
> +        "max_vcpus": 4,
> +        "avail_vcpus": [
> +            0,
> +            1,
> +            2,
> +            3
> +        ],
> +        "cpumap": [
> +
> +        ],
> +        "nodemap": [
> +
> +        ],
>   

But 4.2 does not have nodemap,

> +        "numa_placement": "<default>",
> +        "tsc_mode": "default",
> +        "max_memkb": 2560000,
> +        "target_memkb": 307200,
> +        "video_memkb": -1,
> +        "shadow_memkb": -1,
> +        "rtc_timeoffset": 0,
> +        "exec_ssidref": 0,
>   

doesn't have exec_ssidref,

> +        "localtime": "<default>",
> +        "disable_migrate": "<default>",
> +        "cpuid": [
> +
> +        ],
> +        "blkdev_start": null,
> +        "device_model_version": null,
> +        "device_model_stubdomain": "<default>",
> +        "device_model": null,
> +        "device_model_ssidref": 0,
> +        "extra": [
> +
> +        ],
> +        "extra_pv": [
> +
> +        ],
> +        "extra_hvm": [
> +
> +        ],
> +        "sched_params": {
> +            "sched": "unknown",
> +            "weight": 1000,
> +            "cap": -1,
> +            "period": -1,
> +            "slice": -1,
> +            "latency": -1,
> +            "extratime": -1
> +        },
> +        "ioports": [
> +
> +        ],
> +        "irqs": [
> +
> +        ],
> +        "iomem": [
> +
> +        ],
> +        "claim_mode": "<default>",
>   

doesn't have iomem or claim_mode,

> +        "u": {
> +            "kernel": null,
> +            "slack_memkb": -1,
> +            "bootloader": "/usr/bin/pygrub",
> +            "bootloader_args": [
> +
> +            ],
> +            "cmdline": null,
> +            "ramdisk": null,
> +            "e820_host": "<default>"
> +        }
> +    },
> +    "disks": [
> +        {
> +            "backend_domid": 0,
> +            "backend_domname": null,
>   

none of the devices have backend_domname,

> +            "pdev_path": "/var/lib/xen/images/rhel5pv.img",
> +            "vdev": "xvda",
> +            "backend": "tap",
> +            "format": "raw",
> +            "script": null,
> +            "removable": 1,
> +            "readwrite": 1,
> +            "is_cdrom": 0
> +        },
> +        {
> +            "backend_domid": 0,
> +            "backend_domname": null,
> +            "pdev_path": "/root/qcow1-xen.img",
> +            "vdev": "xvdd",
> +            "backend": "qdisk",
> +            "format": "qcow",
> +            "script": null,
> +            "removable": 1,
> +            "readwrite": 1,
> +            "is_cdrom": 0
> +        }
> +    ],
> +    "nics": [
> +        {
> +            "backend_domid": 0,
> +            "backend_domname": null,
> +            "devid": 0,
> +            "mtu": 0,
> +            "model": null,
> +            "mac": "00:16:3e:60:36:ba",
> +            "ip": null,
> +            "bridge": "xenbr0",
> +            "ifname": null,
> +            "script": null,
> +            "nictype": "vif",
> +            "rate_bytes_per_interval": 0,
> +            "rate_interval_usecs": 0,
> +            "gatewaydev": null
> +        }
> +    ],
> +    "pcidevs": [
> +
> +    ],
> +    "vfbs": [
> +        {
> +            "backend_domid": 0,
> +            "backend_domname": null,
> +            "devid": -1,
> +            "vnc": {
> +                "enable": "True",
> +                "listen": "0.0.0.0",
> +                "passwd": null,
> +                "display": 0,
> +                "findunused": "False"
> +            },
> +            "sdl": {
> +                "enable": "<default>",
> +                "opengl": "<default>",
> +                "display": null,
> +                "xauthority": null
> +            },
> +            "keymap": null
> +        }
> +    ],
> +    "vkbs": [
> +        {
> +            "backend_domid": 0,
> +            "backend_domname": null,
> +            "devid": -1
> +        }
> +    ],
> +    "vtpms": [
> +
> +    ],
>   

and doesn't contain vtpms, so the test fails.  Should we base the
expected data on Xen 4.2, or look for some other way to disable the test
on 4.2?

Regards,
Jim

> +    "on_poweroff": null,
> +    "on_reboot": "destroy",
> +    "on_watchdog": null,
> +    "on_crash": "destroy"
> +}
> diff --git a/tests/libxlxml2jsondata/minimal.xml b/tests/libxlxml2jsondata/minimal.xml
> new file mode 100644
> index 0000000..1361028
> --- /dev/null
> +++ b/tests/libxlxml2jsondata/minimal.xml
> @@ -0,0 +1,36 @@
> +<domain type='xen'>
> +  <name>rhel5pv</name>
> +  <uuid>8f07fe28-753f-2729-d76d-bdbd892f949a</uuid>
> +  <memory>2560000</memory>
> +  <currentMemory>307200</currentMemory>
> +  <vcpu>4</vcpu>
> +  <bootloader>/usr/bin/pygrub</bootloader>
> +  <os>
> +    <type arch='i686' machine='xenpv'>linux</type>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <disk type='file' device='disk'>
> +      <driver name='tap' type='aio'/>
> +      <source file='/var/lib/xen/images/rhel5pv.img'/>
> +      <target dev='xvda' bus='xen'/>
> +    </disk>
> +    <disk type='file' device='disk'>
> +      <driver name='tap' type='qcow'/>
> +      <source file='/root/qcow1-xen.img'/>
> +      <target dev='xvdd' bus='xen'/>
> +    </disk>
> +    <interface type='bridge'>
> +      <mac address='00:16:3e:60:36:ba'/>
> +      <source bridge='xenbr0'/>
> +    </interface>
> +    <console type='pty'>
> +      <target port='0'/>
> +    </console>
> +    <input type='mouse' bus='xen'/>
> +    <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'/>
> +  </devices>
> +</domain>
> diff --git a/tests/libxlxml2jsontest.c b/tests/libxlxml2jsontest.c
> new file mode 100644
> index 0000000..a0f7d88
> --- /dev/null
> +++ b/tests/libxlxml2jsontest.c
> @@ -0,0 +1,186 @@
> +/*
> + * Copyright (C) 2014 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Daniel P. Berrange <berrange@redhat.com>
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include <sys/types.h>
> +#include <fcntl.h>
> +
> +#include "testutils.h"
> +
> +#if defined(WITH_LIBXL) && defined(WITH_YAJL) && defined(HAVE_LIBXL_DOMAIN_CONFIG_TO_JSON)
> +
> +# include "internal.h"
> +# include "viralloc.h"
> +# include "libxl/libxl_conf.h"
> +# include "datatypes.h"
> +# include "virstring.h"
> +# include "virmock.h"
> +# include "virjson.h"
> +# include "testutilsxen.h"
> +
> +# define VIR_FROM_THIS VIR_FROM_XEN
> +
> +static const char *abs_top_srcdir;
> +static virCapsPtr xencaps;
> +
> +static int testCompareXMLToJSONFiles(const char *xml,
> +                                     const char *cmdline)
> +{
> +    char *expectargv = NULL;
> +    int ret = -1;
> +    virDomainDefPtr vmdef = NULL;
> +    virPortAllocatorPtr gports = NULL;
> +    libxl_ctx *ctx = NULL;
> +    libxl_domain_config config;
> +    xentoollog_logger *log = NULL;
> +    virDomainXMLOptionPtr xmlopt = NULL;
> +    char *actualargv;
> +
> +    libxl_domain_config_init(&config);
> +
> +    if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, XTL_DEBUG, 0)))
> +        goto cleanup;
> +
> +    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, log) < 0)
> +        goto cleanup;
> +
> +    if (!(gports = virPortAllocatorNew("vnc", 5900, 6000,
> +                                       VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK)))
> +        goto cleanup;
> +
> +    if (!(xmlopt = libxlCreateXMLConf()))
> +        goto cleanup;
> +
> +    if (!(vmdef = virDomainDefParseFile(xml, xencaps, xmlopt,
> +                                        1 << VIR_DOMAIN_VIRT_XEN,
> +                                        VIR_DOMAIN_XML_INACTIVE)))
> +        goto cleanup;
> +
> +    if (libxlBuildDomainConfig(gports, vmdef, ctx, &config) < 0)
> +        goto cleanup;
> +
> +    if (!(actualargv = libxl_domain_config_to_json(ctx, &config))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       "Failed to create JSON doc for xl config");
> +        goto cleanup;
> +    }
> +
> +    virtTestLoadFile(cmdline, &expectargv);
> +
> +    if (!virJSONStringCompare(expectargv, actualargv,
> +                              VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS |
> +                              VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(expectargv);
> +    VIR_FREE(actualargv);
> +    virDomainDefFree(vmdef);
> +    virObjectUnref(gports);
> +    virObjectUnref(xmlopt);
> +    libxl_ctx_free(ctx);
> +    libxl_domain_config_dispose(&config);
> +    xtl_logger_destroy(log);
> +    return ret;
> +}
> +
> +
> +struct testInfo {
> +    const char *name;
> +};
> +
> +static int
> +testCompareXMLToJSONHelper(const void *data)
> +{
> +    int ret = -1;
> +    const struct testInfo *info = data;
> +    char *xml = NULL;
> +    char *args = NULL;
> +
> +    if (virAsprintf(&xml, "%s/libxlxml2jsondata/%s.xml",
> +                    abs_srcdir, info->name) < 0 ||
> +        virAsprintf(&args, "%s/libxlxml2jsondata/%s.json",
> +                    abs_srcdir, info->name) < 0)
> +        goto cleanup;
> +
> +    ret = testCompareXMLToJSONFiles(xml, args);
> +
> + cleanup:
> +    VIR_FREE(xml);
> +    VIR_FREE(args);
> +    return ret;
> +}
> +
> +
> +static int
> +mymain(void)
> +{
> +    int ret = 0;
> +
> +    abs_top_srcdir = getenv("abs_top_srcdir");
> +    if (!abs_top_srcdir)
> +        abs_top_srcdir = abs_srcdir "/..";
> +
> +    /* Set the timezone because we are mocking the time() function.
> +     * If we don't do that, then localtime() may return unpredictable
> +     * results. In order to detect things that just work by a blind
> +     * chance, we need to set an virtual timezone that no libvirt
> +     * developer resides in. */
> +    if (setenv("TZ", "VIR00:30", 1) < 0) {
> +        perror("setenv");
> +        return EXIT_FAILURE;
> +    }
> +
> +    if ((xencaps = testXenCapsInit()) == NULL)
> +        return EXIT_FAILURE;
> +
> +# define DO_TEST(name)                                                  \
> +    do {                                                                \
> +        static struct testInfo info = {                                 \
> +            name,                                                       \
> +        };                                                              \
> +        if (virtTestRun("LibXL XML-2-JSON " name,                       \
> +                        testCompareXMLToJSONHelper, &info) < 0)         \
> +            ret = -1;                                                   \
> +    } while (0)
> +
> +    DO_TEST("minimal");
> +
> +    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmocklibxl.so")
> +
> +#else
> +
> +int main(void)
> +{
> +    return EXIT_AM_SKIP;
> +}
> +
> +#endif /* WITH_XEN && WITH_YAJL && HAVE_LIBXL_DOMAIN_CONFIG_TO_JSON */
> diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
> new file mode 100644
> index 0000000..bc4b53d
> --- /dev/null
> +++ b/tests/virmocklibxl.c
> @@ -0,0 +1,87 @@
> +/*
> + * virmocklibxl.c: mocking of xenstore/libxs for libxl
> + *
> + * Copyright (C) 2014 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Daniel P. Berrange <berrange@redhat.com>
> + */
> +
> +#include <config.h>
> +
> +#if defined(WITH_LIBXL) && defined(WITH_YAJL)
> +# include "virmock.h"
> +# include <sys/stat.h>
> +# include <unistd.h>
> +# include <libxl.h>
> +# include <xenstore.h>
> +# include <xenctrl.h>
> +
> +VIR_MOCK_IMPL_RET_VOID(xs_daemon_open,
> +                       struct xs_handle *)
> +{
> +    VIR_MOCK_REAL_INIT(xs_daemon_open);
> +    return (void*)0x1;
> +}
> +
> +VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
> +                       xc_interface *,
> +                       xentoollog_logger *, logger,
> +                       xentoollog_logger *, dombuild_logger,
> +                       unsigned, open_flags)
> +{
> +    VIR_MOCK_REAL_INIT(xc_interface_open);
> +    return (void*)0x1;
> +}
> +
> +
> +VIR_MOCK_STUB_RET_ARGS(xc_interface_close,
> +                       int, 0,
> +                       xc_interface *, handle)
> +
> +VIR_MOCK_STUB_VOID_ARGS(xs_daemon_close,
> +                        struct xs_handle *, handle)
> +
> +VIR_MOCK_IMPL_RET_ARGS(__xstat, int,
> +                       int, ver,
> +                       const char *, path,
> +                       struct stat *, sb)
> +{
> +    VIR_MOCK_REAL_INIT(__xstat);
> +
> +    if (strstr(path, "xenstored.pid")) {
> +        memset(sb, 0, sizeof(*sb));
> +        return 0;
> +    }
> +
> +    return real___xstat(ver, path, sb);
> +}
> +
> +VIR_MOCK_IMPL_RET_ARGS(stat, int,
> +                       const char *, path,
> +                       struct stat *, sb)
> +{
> +    VIR_MOCK_REAL_INIT(stat);
> +
> +    if (strstr(path, "xenstored.pid")) {
> +        memset(sb, 0, sizeof(*sb));
> +        return 0;
> +    }
> +
> +    return real_stat(path, sb);
> +}
> +
> +#endif /* WITH_LIBXL && WITH_YAJL */
>   

Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

[Jim Fehlig]

Jim Fehlig wrote:
> Daniel P. Berrange wrote:
>   
>> The libxl library allows a libxl_domain_config object to be
>> serialized to a JSON string. Use this to allow testing of
>> the XML -> libxl_domain_config conversion process
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  configure.ac                         |   2 +
>>  tests/Makefile.am                    |  25 ++++-
>>  tests/libxlxml2jsondata/minimal.json | 172 ++++++++++++++++++++++++++++++++
>>  tests/libxlxml2jsondata/minimal.xml  |  36 +++++++
>>  tests/libxlxml2jsontest.c            | 186 +++++++++++++++++++++++++++++++++++
>>  tests/virmocklibxl.c                 |  87 ++++++++++++++++
>>  6 files changed, 507 insertions(+), 1 deletion(-)
>>  create mode 100644 tests/libxlxml2jsondata/minimal.json
>>  create mode 100644 tests/libxlxml2jsondata/minimal.xml
>>  create mode 100644 tests/libxlxml2jsontest.c
>>  create mode 100644 tests/virmocklibxl.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index 368d094..e87be85 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -875,6 +875,8 @@ if test "$with_libxl" != "no" ; then
>>      AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [
>>          with_libxl=yes
>>          LIBXL_LIBS="$LIBXL_LIBS -lxenlight -lxenctrl"
>> +        LIBS="$LIBS -lxenlight -lxenctrl"
>> +        AC_CHECK_FUNCS([libxl_domain_config_to_json])
>>   
>>     
>
> This function exists in Xen 4.2 as well, in libxl.h.
>   

Any ideas on how to handle this?  I'm not aware of an existing macro to
check for func 'foo' defined in header 'bar'.  Is writing a custom macro
along these lines a good solution?  A bad solution I tried was hacking
the test to check libxl version via libxl_get_version_info(), but that
API does not work if not running Xen.

Regards,
Jim

>>      ],[
>>          if test "$with_libxl" = "yes"; then
>>              fail=1
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index f9f2b84..742503d 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -82,6 +82,7 @@ EXTRA_DIST =		\
>>  	domainsnapshotxml2xmlout \
>>  	fchostdata \
>>  	interfaceschemadata \
>> +	libxlxml2jsondata \
>>  	lxcconf2xmldata \
>>  	lxcxml2xmldata \
>>  	lxcxml2xmloutdata \
>> @@ -216,6 +217,9 @@ if WITH_XEN
>>  test_programs += xml2sexprtest sexpr2xmltest \
>>  	xmconfigtest xencapstest statstest reconnect
>>  endif WITH_XEN
>> +if WITH_LIBXL
>> +test_programs += libxlxml2jsontest
>> +endif WITH_LIBXL
>>  if WITH_QEMU
>>  test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
>>  	qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
>> @@ -378,7 +382,9 @@ test_libraries += libqemumonitortestutils.la \
>>  		qemuxml2argvmock.la \
>>  		$(NULL)
>>  endif WITH_QEMU
>> -
>> +if WITH_LIBXL
>> +test_libraries += virmocklibxl.la
>> +endif WITH_LIBXL
>>  if WITH_BHYVE
>>  test_libraries += bhyvexml2argvmock.la
>>  endif WITH_BHYVE
>> @@ -577,6 +583,23 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
>>  	$(QEMUMONITORTESTUTILS_SOURCES)
>>  endif ! WITH_QEMU
>>  
>> +if WITH_LIBXL
>> +libxl_LDADDS = ../src/libvirt_driver_libxl_impl.la
>> +libxl_LDADDS += $(LDADDS)
>> +
>> +libxlxml2jsontest_SOURCES = \
>> +	libxlxml2jsontest.c testutilsxen.c testutilsxen.h \
>> +	testutils.c testutils.h
>> +libxlxml2jsontest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS)
>> +
>> +virmocklibxl_la_SOURCES = \
>> +	virmocklibxl.c
>> +virmocklibxl_la_CFLAGS = $(AM_CFLAGS)
>> +virmocklibxl_la_LDFLAGS = -module -avoid-version \
>> +        -rpath /evil/libtool/hack/to/force/shared/lib/creation
>> +
>> +endif WITH_LIBXL
>> +
>>  if WITH_LXC
>>  
>>  lxc_LDADDS = ../src/libvirt_driver_lxc_impl.la
>> diff --git a/tests/libxlxml2jsondata/minimal.json b/tests/libxlxml2jsondata/minimal.json
>> new file mode 100644
>> index 0000000..930e1d8
>> --- /dev/null
>> +++ b/tests/libxlxml2jsondata/minimal.json
>> @@ -0,0 +1,172 @@
>> +{
>> +    "c_info": {
>> +        "type": "pv",
>> +        "hap": "<default>",
>> +        "oos": "<default>",
>> +        "ssidref": 0,
>> +        "name": "rhel5pv",
>> +        "uuid": "8f07fe28-753f-2729-d76d-bdbd892f949a",
>> +        "xsdata": {
>> +
>> +        },
>> +        "platformdata": {
>> +
>> +        },
>> +        "poolid": 0,
>> +        "run_hotplug_scripts": "<default>"
>> +    },
>> +    "b_info": {
>> +        "max_vcpus": 4,
>> +        "avail_vcpus": [
>> +            0,
>> +            1,
>> +            2,
>> +            3
>> +        ],
>> +        "cpumap": [
>> +
>> +        ],
>> +        "nodemap": [
>> +
>> +        ],
>>   
>>     
>
> But 4.2 does not have nodemap,
>
>   
>> +        "numa_placement": "<default>",
>> +        "tsc_mode": "default",
>> +        "max_memkb": 2560000,
>> +        "target_memkb": 307200,
>> +        "video_memkb": -1,
>> +        "shadow_memkb": -1,
>> +        "rtc_timeoffset": 0,
>> +        "exec_ssidref": 0,
>>   
>>     
>
> doesn't have exec_ssidref,
>
>   
>> +        "localtime": "<default>",
>> +        "disable_migrate": "<default>",
>> +        "cpuid": [
>> +
>> +        ],
>> +        "blkdev_start": null,
>> +        "device_model_version": null,
>> +        "device_model_stubdomain": "<default>",
>> +        "device_model": null,
>> +        "device_model_ssidref": 0,
>> +        "extra": [
>> +
>> +        ],
>> +        "extra_pv": [
>> +
>> +        ],
>> +        "extra_hvm": [
>> +
>> +        ],
>> +        "sched_params": {
>> +            "sched": "unknown",
>> +            "weight": 1000,
>> +            "cap": -1,
>> +            "period": -1,
>> +            "slice": -1,
>> +            "latency": -1,
>> +            "extratime": -1
>> +        },
>> +        "ioports": [
>> +
>> +        ],
>> +        "irqs": [
>> +
>> +        ],
>> +        "iomem": [
>> +
>> +        ],
>> +        "claim_mode": "<default>",
>>   
>>     
>
> doesn't have iomem or claim_mode,
>
>   
>> +        "u": {
>> +            "kernel": null,
>> +            "slack_memkb": -1,
>> +            "bootloader": "/usr/bin/pygrub",
>> +            "bootloader_args": [
>> +
>> +            ],
>> +            "cmdline": null,
>> +            "ramdisk": null,
>> +            "e820_host": "<default>"
>> +        }
>> +    },
>> +    "disks": [
>> +        {
>> +            "backend_domid": 0,
>> +            "backend_domname": null,
>>   
>>     
>
> none of the devices have backend_domname,
>
>   
>> +            "pdev_path": "/var/lib/xen/images/rhel5pv.img",
>> +            "vdev": "xvda",
>> +            "backend": "tap",
>> +            "format": "raw",
>> +            "script": null,
>> +            "removable": 1,
>> +            "readwrite": 1,
>> +            "is_cdrom": 0
>> +        },
>> +        {
>> +            "backend_domid": 0,
>> +            "backend_domname": null,
>> +            "pdev_path": "/root/qcow1-xen.img",
>> +            "vdev": "xvdd",
>> +            "backend": "qdisk",
>> +            "format": "qcow",
>> +            "script": null,
>> +            "removable": 1,
>> +            "readwrite": 1,
>> +            "is_cdrom": 0
>> +        }
>> +    ],
>> +    "nics": [
>> +        {
>> +            "backend_domid": 0,
>> +            "backend_domname": null,
>> +            "devid": 0,
>> +            "mtu": 0,
>> +            "model": null,
>> +            "mac": "00:16:3e:60:36:ba",
>> +            "ip": null,
>> +            "bridge": "xenbr0",
>> +            "ifname": null,
>> +            "script": null,
>> +            "nictype": "vif",
>> +            "rate_bytes_per_interval": 0,
>> +            "rate_interval_usecs": 0,
>> +            "gatewaydev": null
>> +        }
>> +    ],
>> +    "pcidevs": [
>> +
>> +    ],
>> +    "vfbs": [
>> +        {
>> +            "backend_domid": 0,
>> +            "backend_domname": null,
>> +            "devid": -1,
>> +            "vnc": {
>> +                "enable": "True",
>> +                "listen": "0.0.0.0",
>> +                "passwd": null,
>> +                "display": 0,
>> +                "findunused": "False"
>> +            },
>> +            "sdl": {
>> +                "enable": "<default>",
>> +                "opengl": "<default>",
>> +                "display": null,
>> +                "xauthority": null
>> +            },
>> +            "keymap": null
>> +        }
>> +    ],
>> +    "vkbs": [
>> +        {
>> +            "backend_domid": 0,
>> +            "backend_domname": null,
>> +            "devid": -1
>> +        }
>> +    ],
>> +    "vtpms": [
>> +
>> +    ],
>>   
>>     
>
> and doesn't contain vtpms, so the test fails.  Should we base the
> expected data on Xen 4.2, or look for some other way to disable the test
> on 4.2?
>
> Regards,
> Jim
>
>   
>> +    "on_poweroff": null,
>> +    "on_reboot": "destroy",
>> +    "on_watchdog": null,
>> +    "on_crash": "destroy"
>> +}
>> diff --git a/tests/libxlxml2jsondata/minimal.xml b/tests/libxlxml2jsondata/minimal.xml
>> new file mode 100644
>> index 0000000..1361028
>> --- /dev/null
>> +++ b/tests/libxlxml2jsondata/minimal.xml
>> @@ -0,0 +1,36 @@
>> +<domain type='xen'>
>> +  <name>rhel5pv</name>
>> +  <uuid>8f07fe28-753f-2729-d76d-bdbd892f949a</uuid>
>> +  <memory>2560000</memory>
>> +  <currentMemory>307200</currentMemory>
>> +  <vcpu>4</vcpu>
>> +  <bootloader>/usr/bin/pygrub</bootloader>
>> +  <os>
>> +    <type arch='i686' machine='xenpv'>linux</type>
>> +  </os>
>> +  <clock offset='utc'/>
>> +  <on_poweroff>destroy</on_poweroff>
>> +  <on_reboot>restart</on_reboot>
>> +  <on_crash>restart</on_crash>
>> +  <devices>
>> +    <disk type='file' device='disk'>
>> +      <driver name='tap' type='aio'/>
>> +      <source file='/var/lib/xen/images/rhel5pv.img'/>
>> +      <target dev='xvda' bus='xen'/>
>> +    </disk>
>> +    <disk type='file' device='disk'>
>> +      <driver name='tap' type='qcow'/>
>> +      <source file='/root/qcow1-xen.img'/>
>> +      <target dev='xvdd' bus='xen'/>
>> +    </disk>
>> +    <interface type='bridge'>
>> +      <mac address='00:16:3e:60:36:ba'/>
>> +      <source bridge='xenbr0'/>
>> +    </interface>
>> +    <console type='pty'>
>> +      <target port='0'/>
>> +    </console>
>> +    <input type='mouse' bus='xen'/>
>> +    <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'/>
>> +  </devices>
>> +</domain>
>> diff --git a/tests/libxlxml2jsontest.c b/tests/libxlxml2jsontest.c
>> new file mode 100644
>> index 0000000..a0f7d88
>> --- /dev/null
>> +++ b/tests/libxlxml2jsontest.c
>> @@ -0,0 +1,186 @@
>> +/*
>> + * Copyright (C) 2014 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library.  If not, see
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Author: Daniel P. Berrange <berrange@redhat.com>
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <string.h>
>> +
>> +#include <sys/types.h>
>> +#include <fcntl.h>
>> +
>> +#include "testutils.h"
>> +
>> +#if defined(WITH_LIBXL) && defined(WITH_YAJL) && defined(HAVE_LIBXL_DOMAIN_CONFIG_TO_JSON)
>> +
>> +# include "internal.h"
>> +# include "viralloc.h"
>> +# include "libxl/libxl_conf.h"
>> +# include "datatypes.h"
>> +# include "virstring.h"
>> +# include "virmock.h"
>> +# include "virjson.h"
>> +# include "testutilsxen.h"
>> +
>> +# define VIR_FROM_THIS VIR_FROM_XEN
>> +
>> +static const char *abs_top_srcdir;
>> +static virCapsPtr xencaps;
>> +
>> +static int testCompareXMLToJSONFiles(const char *xml,
>> +                                     const char *cmdline)
>> +{
>> +    char *expectargv = NULL;
>> +    int ret = -1;
>> +    virDomainDefPtr vmdef = NULL;
>> +    virPortAllocatorPtr gports = NULL;
>> +    libxl_ctx *ctx = NULL;
>> +    libxl_domain_config config;
>> +    xentoollog_logger *log = NULL;
>> +    virDomainXMLOptionPtr xmlopt = NULL;
>> +    char *actualargv;
>> +
>> +    libxl_domain_config_init(&config);
>> +
>> +    if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, XTL_DEBUG, 0)))
>> +        goto cleanup;
>> +
>> +    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, log) < 0)
>> +        goto cleanup;
>> +
>> +    if (!(gports = virPortAllocatorNew("vnc", 5900, 6000,
>> +                                       VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK)))
>> +        goto cleanup;
>> +
>> +    if (!(xmlopt = libxlCreateXMLConf()))
>> +        goto cleanup;
>> +
>> +    if (!(vmdef = virDomainDefParseFile(xml, xencaps, xmlopt,
>> +                                        1 << VIR_DOMAIN_VIRT_XEN,
>> +                                        VIR_DOMAIN_XML_INACTIVE)))
>> +        goto cleanup;
>> +
>> +    if (libxlBuildDomainConfig(gports, vmdef, ctx, &config) < 0)
>> +        goto cleanup;
>> +
>> +    if (!(actualargv = libxl_domain_config_to_json(ctx, &config))) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       "Failed to create JSON doc for xl config");
>> +        goto cleanup;
>> +    }
>> +
>> +    virtTestLoadFile(cmdline, &expectargv);
>> +
>> +    if (!virJSONStringCompare(expectargv, actualargv,
>> +                              VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS |
>> +                              VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL))
>> +        goto cleanup;
>> +
>> +    ret = 0;
>> +
>> + cleanup:
>> +    VIR_FREE(expectargv);
>> +    VIR_FREE(actualargv);
>> +    virDomainDefFree(vmdef);
>> +    virObjectUnref(gports);
>> +    virObjectUnref(xmlopt);
>> +    libxl_ctx_free(ctx);
>> +    libxl_domain_config_dispose(&config);
>> +    xtl_logger_destroy(log);
>> +    return ret;
>> +}
>> +
>> +
>> +struct testInfo {
>> +    const char *name;
>> +};
>> +
>> +static int
>> +testCompareXMLToJSONHelper(const void *data)
>> +{
>> +    int ret = -1;
>> +    const struct testInfo *info = data;
>> +    char *xml = NULL;
>> +    char *args = NULL;
>> +
>> +    if (virAsprintf(&xml, "%s/libxlxml2jsondata/%s.xml",
>> +                    abs_srcdir, info->name) < 0 ||
>> +        virAsprintf(&args, "%s/libxlxml2jsondata/%s.json",
>> +                    abs_srcdir, info->name) < 0)
>> +        goto cleanup;
>> +
>> +    ret = testCompareXMLToJSONFiles(xml, args);
>> +
>> + cleanup:
>> +    VIR_FREE(xml);
>> +    VIR_FREE(args);
>> +    return ret;
>> +}
>> +
>> +
>> +static int
>> +mymain(void)
>> +{
>> +    int ret = 0;
>> +
>> +    abs_top_srcdir = getenv("abs_top_srcdir");
>> +    if (!abs_top_srcdir)
>> +        abs_top_srcdir = abs_srcdir "/..";
>> +
>> +    /* Set the timezone because we are mocking the time() function.
>> +     * If we don't do that, then localtime() may return unpredictable
>> +     * results. In order to detect things that just work by a blind
>> +     * chance, we need to set an virtual timezone that no libvirt
>> +     * developer resides in. */
>> +    if (setenv("TZ", "VIR00:30", 1) < 0) {
>> +        perror("setenv");
>> +        return EXIT_FAILURE;
>> +    }
>> +
>> +    if ((xencaps = testXenCapsInit()) == NULL)
>> +        return EXIT_FAILURE;
>> +
>> +# define DO_TEST(name)                                                  \
>> +    do {                                                                \
>> +        static struct testInfo info = {                                 \
>> +            name,                                                       \
>> +        };                                                              \
>> +        if (virtTestRun("LibXL XML-2-JSON " name,                       \
>> +                        testCompareXMLToJSONHelper, &info) < 0)         \
>> +            ret = -1;                                                   \
>> +    } while (0)
>> +
>> +    DO_TEST("minimal");
>> +
>> +    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>> +}
>> +
>> +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmocklibxl.so")
>> +
>> +#else
>> +
>> +int main(void)
>> +{
>> +    return EXIT_AM_SKIP;
>> +}
>> +
>> +#endif /* WITH_XEN && WITH_YAJL && HAVE_LIBXL_DOMAIN_CONFIG_TO_JSON */
>> diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
>> new file mode 100644
>> index 0000000..bc4b53d
>> --- /dev/null
>> +++ b/tests/virmocklibxl.c
>> @@ -0,0 +1,87 @@
>> +/*
>> + * virmocklibxl.c: mocking of xenstore/libxs for libxl
>> + *
>> + * Copyright (C) 2014 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library.  If not, see
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Author: Daniel P. Berrange <berrange@redhat.com>
>> + */
>> +
>> +#include <config.h>
>> +
>> +#if defined(WITH_LIBXL) && defined(WITH_YAJL)
>> +# include "virmock.h"
>> +# include <sys/stat.h>
>> +# include <unistd.h>
>> +# include <libxl.h>
>> +# include <xenstore.h>
>> +# include <xenctrl.h>
>> +
>> +VIR_MOCK_IMPL_RET_VOID(xs_daemon_open,
>> +                       struct xs_handle *)
>> +{
>> +    VIR_MOCK_REAL_INIT(xs_daemon_open);
>> +    return (void*)0x1;
>> +}
>> +
>> +VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
>> +                       xc_interface *,
>> +                       xentoollog_logger *, logger,
>> +                       xentoollog_logger *, dombuild_logger,
>> +                       unsigned, open_flags)
>> +{
>> +    VIR_MOCK_REAL_INIT(xc_interface_open);
>> +    return (void*)0x1;
>> +}
>> +
>> +
>> +VIR_MOCK_STUB_RET_ARGS(xc_interface_close,
>> +                       int, 0,
>> +                       xc_interface *, handle)
>> +
>> +VIR_MOCK_STUB_VOID_ARGS(xs_daemon_close,
>> +                        struct xs_handle *, handle)
>> +
>> +VIR_MOCK_IMPL_RET_ARGS(__xstat, int,
>> +                       int, ver,
>> +                       const char *, path,
>> +                       struct stat *, sb)
>> +{
>> +    VIR_MOCK_REAL_INIT(__xstat);
>> +
>> +    if (strstr(path, "xenstored.pid")) {
>> +        memset(sb, 0, sizeof(*sb));
>> +        return 0;
>> +    }
>> +
>> +    return real___xstat(ver, path, sb);
>> +}
>> +
>> +VIR_MOCK_IMPL_RET_ARGS(stat, int,
>> +                       const char *, path,
>> +                       struct stat *, sb)
>> +{
>> +    VIR_MOCK_REAL_INIT(stat);
>> +
>> +    if (strstr(path, "xenstored.pid")) {
>> +        memset(sb, 0, sizeof(*sb));
>> +        return 0;
>> +    }
>> +
>> +    return real_stat(path, sb);
>> +}
>> +
>> +#endif /* WITH_LIBXL && WITH_YAJL */
>>   
>>     
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
>   

Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

[Ian Campbell]

On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote:
> > This function exists in Xen 4.2 as well, in libxl.h.
> >   
> 
> Any ideas on how to handle this?  I'm not aware of an existing macro to
> check for func 'foo' defined in header 'bar'.  Is writing a custom macro
> along these lines a good solution?  A bad solution I tried was hacking
> the test to check libxl version via libxl_get_version_info(), but that
> API does not work if not running Xen.

Given that it exists in everything from 4.2 onwards why do you need to
check for it?

>From the PoV of these tests (or any application generally) I'd have
thought you wouldn't really care if you get this function from libxl.h
directly or indirectly via libxl.h including _libxl_types.h.

Ian.

Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

[Jim Fehlig]

Ian Campbell wrote:
> On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote:
>   
>>> This function exists in Xen 4.2 as well, in libxl.h.
>>>   
>>>       
>> Any ideas on how to handle this?  I'm not aware of an existing macro to
>> check for func 'foo' defined in header 'bar'.  Is writing a custom macro
>> along these lines a good solution?  A bad solution I tried was hacking
>> the test to check libxl version via libxl_get_version_info(), but that
>> API does not work if not running Xen.
>>     
>
> Given that it exists in everything from 4.2 onwards why do you need to
> check for it?
>   

Hrm, right.  I had the half-brained idea to use this to solve the
failures I saw when testing this series against Xen 4.2

https://www.redhat.com/archives/libvir-list/2014-June/msg00170.html

I think the solution to that specific problem is to use Xen 4.2 config
as the baseline.  But it got me thinking about the general problem you
mentioned near the end of this mail

https://www.redhat.com/archives/libvir-list/2014-June/msg00032.html

With virJSONStringCompare in 1/1, Daniel provides a way to handle
existence of new fields showing up in the json.  But what if I want to
write a test where the expected data is not supported on earlier
versions?   E.g. how would I add a test to check conversion of '<tpm
...>' to 'vtpms: [ ]' and expect that to work when running 'make check'
against a 4.2 libxl where vtpms were not yet supported?  I suppose each
such test would have to probe for the feature it checks and skip if not
found.

Regards,
Jim

Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

[Ian Campbell]

On Tue, 2014-06-17 at 12:40 -0600, Jim Fehlig wrote:
> Ian Campbell wrote:
> > On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote:
> >   
> >>> This function exists in Xen 4.2 as well, in libxl.h.
> >>>   
> >>>       
> >> Any ideas on how to handle this?  I'm not aware of an existing macro to
> >> check for func 'foo' defined in header 'bar'.  Is writing a custom macro
> >> along these lines a good solution?  A bad solution I tried was hacking
> >> the test to check libxl version via libxl_get_version_info(), but that
> >> API does not work if not running Xen.
> >>     
> >
> > Given that it exists in everything from 4.2 onwards why do you need to
> > check for it?
> >   
> 
> Hrm, right.  I had the half-brained idea to use this to solve the
> failures I saw when testing this series against Xen 4.2
> 
> https://www.redhat.com/archives/libvir-list/2014-June/msg00170.html
> 
> I think the solution to that specific problem is to use Xen 4.2 config
> as the baseline.  But it got me thinking about the general problem you
> mentioned near the end of this mail
> 
> https://www.redhat.com/archives/libvir-list/2014-June/msg00032.html
> 
> With virJSONStringCompare in 1/1, Daniel provides a way to handle
> existence of new fields showing up in the json.  But what if I want to
> write a test where the expected data is not supported on earlier
> versions?   E.g. how would I add a test to check conversion of '<tpm
> ...>' to 'vtpms: [ ]' and expect that to work when running 'make check'
> against a 4.2 libxl where vtpms were not yet supported?  I suppose each
> such test would have to probe for the feature it checks and skip if not
> found.

Right. You'd probably want to gate such a test case on the corresponding
LIBXL_HAVE_XXX #define from libxl.h.

Ian.

Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

[Daniel P. Berrange]

On Tue, Jun 17, 2014 at 12:40:30PM -0600, Jim Fehlig wrote:
> Ian Campbell wrote:
> > On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote:
> >   
> >>> This function exists in Xen 4.2 as well, in libxl.h.
> >>>   
> >>>       
> >> Any ideas on how to handle this?  I'm not aware of an existing macro to
> >> check for func 'foo' defined in header 'bar'.  Is writing a custom macro
> >> along these lines a good solution?  A bad solution I tried was hacking
> >> the test to check libxl version via libxl_get_version_info(), but that
> >> API does not work if not running Xen.
> >>     
> >
> > Given that it exists in everything from 4.2 onwards why do you need to
> > check for it?
> >   
> 
> Hrm, right.  I had the half-brained idea to use this to solve the
> failures I saw when testing this series against Xen 4.2
> 
> https://www.redhat.com/archives/libvir-list/2014-June/msg00170.html
> 
> I think the solution to that specific problem is to use Xen 4.2 config
> as the baseline.  But it got me thinking about the general problem you
> mentioned near the end of this mail
> 
> https://www.redhat.com/archives/libvir-list/2014-June/msg00032.html
> 
> With virJSONStringCompare in 1/1, Daniel provides a way to handle
> existence of new fields showing up in the json.  But what if I want to
> write a test where the expected data is not supported on earlier
> versions?   E.g. how would I add a test to check conversion of '<tpm
> ...>' to 'vtpms: [ ]' and expect that to work when running 'make check'
> against a 4.2 libxl where vtpms were not yet supported?  I suppose each
> such test would have to probe for the feature it checks and skip if not
> found.

My last approach was to allow the JSON comparator to skip certain
types of missing data but I don't think that's sufficiently flexible
anymore. I'm thinking instead, that we could maintain a set of
context paths which designate things that are new in each version,
which would indicate things that should be skipped with old versions.

So we with 4.3, we'd list

  /c_info/driver_domain
  /c_info/pvh

and any other newly added fields. Then when running the test on 4.2
if the expected XML contained either of those paths, we'd skip them
when comparing against the actual JSON.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|