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

Re: [libvirt] [libvirt-glib 4/4] Add some tests for new capabilities APIs



Running this through valgrind revealed two leaks and an invalid free:

diff --git a/libvirt-gconfig/tests/test-capabilities-parse.c b/libvirt-gconfig/tests/test-capabilities-parse.c
index f895441..eefff2a 100644
--- a/libvirt-gconfig/tests/test-capabilities-parse.c
+++ b/libvirt-gconfig/tests/test-capabilities-parse.c
@@ -44,6 +44,7 @@ static void verify_host_caps(GVirConfigCapabilitiesHost *host_caps)
     g_assert(cpu_caps != NULL);
     str = gvir_config_capabilities_cpu_get_arch(cpu_caps);
     g_assert(g_strcmp0(str, "x86_64") == 0);
+    g_object_unref(G_OBJECT(cpu_caps));
 
     features = gvir_config_capabilities_cpu_get_features(cpu_caps);
     for (iter = features; iter != NULL; iter = iter->next) {
@@ -81,6 +82,7 @@ static void verify_guest_caps(GVirConfigCapabilitiesGuest *guest_caps)
     g_assert(str != NULL);
     str = gvir_config_capabilities_guest_arch_get_emulator(arch_caps);
     g_assert(str != NULL);
+    g_object_unref(G_OBJECT(arch_caps));
 
     domains = gvir_config_capabilities_guest_arch_get_domains(arch_caps);
     for (iter = domains; iter != NULL; iter = iter->next) {
@@ -96,7 +98,7 @@ static void verify_guest_caps(GVirConfigCapabilitiesGuest *guest_caps)
                   g_strcmp0(str, "/usr/bin/qemu-kvm") == 0));
         g_object_unref(G_OBJECT(domain_caps));
     }
-    g_list_free(features);
+    g_list_free(domains);
 }
 
 int main(int argc, char **argv)


On Wed, May 09, 2012 at 04:16:16AM +0300, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
> 
> ---
>  libvirt-gconfig/tests/Makefile.am                 |    4 +-
>  libvirt-gconfig/tests/test-capabilities-parse.c   |  159 +++++++++++
>  libvirt-gconfig/tests/test-capabilities-parse.xml |  294 +++++++++++++++++++++
>  3 files changed, 456 insertions(+), 1 deletions(-)
>  create mode 100644 libvirt-gconfig/tests/test-capabilities-parse.c
>  create mode 100644 libvirt-gconfig/tests/test-capabilities-parse.xml
> 
> diff --git a/libvirt-gconfig/tests/Makefile.am b/libvirt-gconfig/tests/Makefile.am
> index 5061fd9..4d1a564 100644
> --- a/libvirt-gconfig/tests/Makefile.am
> +++ b/libvirt-gconfig/tests/Makefile.am
> @@ -1,4 +1,4 @@
> -noinst_PROGRAMS = test-domain-create test-domain-parse
> +noinst_PROGRAMS = test-domain-create test-domain-parse test-capabilities-parse
>  
>  AM_CFLAGS = \
>  		$(GOBJECT2_CFLAGS) \
> @@ -14,3 +14,5 @@ LDADD = \
>  test_domain_create_SOURCES = test-domain-create.c
>  
>  test_domain_parse_SOURCES = test-domain-parse.c
> +
> +test_capabilities_parse_SOURCES = test-capabilities-parse.c
> diff --git a/libvirt-gconfig/tests/test-capabilities-parse.c b/libvirt-gconfig/tests/test-capabilities-parse.c
> new file mode 100644
> index 0000000..f895441
> --- /dev/null
> +++ b/libvirt-gconfig/tests/test-capabilities-parse.c
> @@ -0,0 +1,159 @@
> +/*
> + * test-capabilities-parse.c: test libvirt-gconfig capabilities parsing
> + *
> + * Copyright (C) 2011-2012 Red Hat, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included
> + * in all copies or substantial portions of the Software.
> + *
> + * The Software is provided "as is", without warranty of any kind, express
> + * or implied, including but not limited to the warranties of
> + * merchantability, fitness for a particular purpose and noninfringement.
> + * In no event shall the authors or copyright holders be liable for any
> + * claim, damages or other liability, whether in an action of contract,
> + * tort or otherwise, arising from, out of or in connection with the
> + * software or the use or other dealings in the Software.
> + *
> + * Authors: Zeeshan Ali <zeenix redhat com>
> + *          Christophe Fergeau <cfergeau redhat com>
> + */
> +
> +#include <config.h>
> +
> +#include <string.h>
> +#include <libvirt-gconfig/libvirt-gconfig.h>
> +
> +static void verify_host_caps(GVirConfigCapabilitiesHost *host_caps)
> +{
> +    GVirConfigCapabilitiesCpu *cpu_caps;
> +    GList *features, *iter;
> +    const char *str;
> +
> +    g_assert(host_caps != NULL);
> +    str = gvir_config_capabilities_host_get_uuid(host_caps);
> +    g_assert(g_strcmp0(str, "cd6a24b3-46f8-01aa-bb39-c39aa2123730") == 0);
> +    cpu_caps = gvir_config_capabilities_host_get_cpu(host_caps);
> +    g_assert(cpu_caps != NULL);
> +    str = gvir_config_capabilities_cpu_get_arch(cpu_caps);
> +    g_assert(g_strcmp0(str, "x86_64") == 0);
> +
> +    features = gvir_config_capabilities_cpu_get_features(cpu_caps);
> +    for (iter = features; iter != NULL; iter = iter->next) {
> +        g_assert(iter->data != NULL);
> +        g_object_unref(G_OBJECT(iter->data));
> +    }
> +    g_list_free(features);
> +}
> +
> +static void verify_guest_caps(GVirConfigCapabilitiesGuest *guest_caps)
> +{
> +    GVirConfigCapabilitiesGuestArch *arch_caps;
> +    GList *features, *domains, *iter;
> +    const char *str;
> +
> +    g_assert(guest_caps != NULL);
> +    g_assert(gvir_config_capabilities_guest_get_os_type(guest_caps) ==
> +             GVIR_CONFIG_DOMAIN_OS_TYPE_HVM);
> +
> +    features = gvir_config_capabilities_guest_get_features(guest_caps);
> +    for (iter = features; iter != NULL; iter = iter->next) {
> +        GVirConfigCapabilitiesGuestFeature *feature_caps;
> +
> +        feature_caps = GVIR_CONFIG_CAPABILITIES_GUEST_FEATURE(iter->data);
> +        g_assert(feature_caps != NULL);
> +        str = gvir_config_capabilities_guest_feature_get_name(feature_caps);
> +        g_assert(str != NULL);
> +        g_object_unref(G_OBJECT(feature_caps));
> +    }
> +    g_list_free(features);
> +
> +    arch_caps = gvir_config_capabilities_guest_get_arch(guest_caps);
> +    g_assert(arch_caps != NULL);
> +    str = gvir_config_capabilities_guest_arch_get_name(arch_caps);
> +    g_assert(str != NULL);
> +    str = gvir_config_capabilities_guest_arch_get_emulator(arch_caps);
> +    g_assert(str != NULL);
> +
> +    domains = gvir_config_capabilities_guest_arch_get_domains(arch_caps);
> +    for (iter = domains; iter != NULL; iter = iter->next) {
> +        GVirConfigCapabilitiesGuestDomain *domain_caps;
> +        GVirConfigDomainVirtType virt_type;
> +
> +        domain_caps = GVIR_CONFIG_CAPABILITIES_GUEST_DOMAIN(iter->data);
> +        g_assert(domain_caps != NULL);
> +        virt_type = gvir_config_capabilities_guest_domain_get_virt_type(domain_caps);
> +        str = gvir_config_capabilities_guest_domain_get_emulator(domain_caps);
> +        g_assert((virt_type == GVIR_CONFIG_DOMAIN_VIRT_QEMU && str == NULL) ||
> +                 (virt_type == GVIR_CONFIG_DOMAIN_VIRT_KVM &&
> +                  g_strcmp0(str, "/usr/bin/qemu-kvm") == 0));
> +        g_object_unref(G_OBJECT(domain_caps));
> +    }
> +    g_list_free(features);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    GVirConfigCapabilities *caps;
> +    GVirConfigCapabilitiesHost *host_caps;
> +    GList *guests_caps, *iter;
> +    char *xml;
> +    GError *error = NULL;
> +
> +    gvir_config_init(&argc, &argv);
> +    if (argc != 2) {
> +        g_print("Usage: %s filename\n", argv[0]);
> +        g_print("Attempt to parse the libvirt XML definition from filename\n");
> +        return 1;
> +    }
> +
> +    g_file_get_contents(argv[1], &xml, NULL, &error);
> +    if (error != NULL) {
> +        g_print("Couldn't read %s: %s\n", argv[1], error->message);
> +        return 2;
> +    }
> +
> +    g_type_init();

Not needed, gvir_config_init calls it

> +
> +    caps = gvir_config_capabilities_new_from_xml(xml, &error);
> +    if (error != NULL) {
> +        g_print("Couldn't parse %s: %s\n", argv[1], error->message);
> +        return 3;
> +    }
> +    g_assert(caps != NULL);
> +    gvir_config_object_validate(GVIR_CONFIG_OBJECT(caps), &error);
> +    if (error != NULL) {
> +        g_print("%s format is invalid: %s\n", argv[1], error->message);
> +        g_clear_error(&error);
> +    }
> +
> +    host_caps = gvir_config_capabilities_get_host(caps);
> +    verify_host_caps(host_caps);
> +    g_object_unref(G_OBJECT(host_caps));
> +
> +    guests_caps = gvir_config_capabilities_get_guests(caps);
> +    for (iter = guests_caps; iter != NULL; iter = iter->next) {
> +        GVirConfigCapabilitiesGuest *guest_caps;
> +
> +        guest_caps = GVIR_CONFIG_CAPABILITIES_GUEST(iter->data);
> +        verify_guest_caps(guest_caps);
> +        g_object_unref(G_OBJECT(guest_caps));
> +    }
> +    g_list_free(guests_caps);
> +
> +    g_free(xml);

I'd move this much closer to gvir_config_capabilities_new_from_xml so that
it's obvious it's no longer useful after this, but feel free to keep things
this way

> +
> +    xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(caps));
> +    g_print("%s\n", xml);
> +    g_free(xml);
> +    g_object_unref(G_OBJECT(caps));
> +
> +    return 0;
> +}
> diff --git a/libvirt-gconfig/tests/test-capabilities-parse.xml b/libvirt-gconfig/tests/test-capabilities-parse.xml
> new file mode 100644
> index 0000000..796c81d
> --- /dev/null
> +++ b/libvirt-gconfig/tests/test-capabilities-parse.xml
> @@ -0,0 +1,294 @@
> +<capabilities>
> +
> +  <host>
> +    <uuid>cd6a24b3-46f8-01aa-bb39-c39aa2123730</uuid>
> +    <cpu>
> +      <arch>x86_64</arch>
> +      <model>Westmere</model>
> +      <vendor>Intel</vendor>
> +      <topology sockets="1" cores="2" threads="2"/>
> +      <feature name="rdtscp"/>
> +      <feature name="pdcm"/>
> +      <feature name="xtpr"/>
> +      <feature name="tm2"/>
> +      <feature name="est"/>
> +      <feature name="smx"/>
> +      <feature name="vmx"/>
> +      <feature name="ds_cpl"/>
> +      <feature name="monitor"/>
> +      <feature name="dtes64"/>
> +      <feature name="pclmuldq"/>
> +      <feature name="pbe"/>
> +      <feature name="tm"/>
> +      <feature name="ht"/>
> +      <feature name="ss"/>
> +      <feature name="acpi"/>
> +      <feature name="ds"/>
> +      <feature name="vme"/>
> +    </cpu>
> +    <power_management>
> +      <suspend_mem/>
> +      <suspend_disk/>
> +    </power_management>
> +    <migration_features>
> +      <live/>
> +      <uri_transports>
> +        <uri_transport>tcp</uri_transport>
> +      </uri_transports>
> +    </migration_features>
> +    <secmodel>
> +      <model>selinux</model>
> +      <doi>0</doi>
> +    </secmodel>
> +  </host>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="i686">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-x86_64</emulator>
> +      <machine>pc-1.1</machine>
> +      <machine canonical="pc-1.1">pc</machine>
> +      <machine>pc-1.0</machine>
> +      <machine>pc-0.15</machine>
> +      <machine>pc-0.14</machine>
> +      <machine>pc-0.13</machine>
> +      <machine>pc-0.12</machine>
> +      <machine>pc-0.11</machine>
> +      <machine>pc-0.10</machine>
> +      <machine>isapc</machine>
> +      <domain type="qemu">
> +      </domain>
> +      <domain type="kvm">
> +        <emulator>/usr/bin/qemu-kvm</emulator>
> +        <machine>pc-1.1</machine>
> +        <machine canonical="pc-1.1">pc</machine>
> +        <machine>pc-1.0</machine>
> +        <machine>pc-0.15</machine>
> +        <machine>pc-0.14</machine>
> +        <machine>pc-0.13</machine>
> +        <machine>pc-0.12</machine>
> +        <machine>pc-0.11</machine>
> +        <machine>pc-0.10</machine>
> +        <machine>isapc</machine>
> +      </domain>
> +    </arch>
> +    <features>
> +      <cpuselection/>
> +      <pae/>
> +      <nonpae/>
> +      <acpi default="on" toggle="yes"/>
> +      <apic default="on" toggle="no"/>
> +    </features>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="x86_64">
> +      <wordsize>64</wordsize>
> +      <emulator>/usr/bin/qemu-system-x86_64</emulator>
> +      <machine>pc-1.1</machine>
> +      <machine canonical="pc-1.1">pc</machine>
> +      <machine>pc-1.0</machine>
> +      <machine>pc-0.15</machine>
> +      <machine>pc-0.14</machine>
> +      <machine>pc-0.13</machine>
> +      <machine>pc-0.12</machine>
> +      <machine>pc-0.11</machine>
> +      <machine>pc-0.10</machine>
> +      <machine>isapc</machine>
> +      <domain type="qemu">
> +      </domain>
> +      <domain type="kvm">
> +        <emulator>/usr/bin/qemu-kvm</emulator>
> +        <machine>pc-1.1</machine>
> +        <machine canonical="pc-1.1">pc</machine>
> +        <machine>pc-1.0</machine>
> +        <machine>pc-0.15</machine>
> +        <machine>pc-0.14</machine>
> +        <machine>pc-0.13</machine>
> +        <machine>pc-0.12</machine>
> +        <machine>pc-0.11</machine>
> +        <machine>pc-0.10</machine>
> +        <machine>isapc</machine>
> +      </domain>
> +    </arch>
> +    <features>
> +      <cpuselection/>
> +      <acpi default="on" toggle="yes"/>
> +      <apic default="on" toggle="no"/>
> +    </features>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="arm">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-arm</emulator>
> +      <machine>integratorcp</machine>
> +      <machine>collie</machine>
> +      <machine>nuri</machine>
> +      <machine>smdkc210</machine>
> +      <machine>connex</machine>
> +      <machine>verdex</machine>
> +      <machine>highbank</machine>
> +      <machine>mainstone</machine>
> +      <machine>musicpal</machine>
> +      <machine>n800</machine>
> +      <machine>n810</machine>
> +      <machine>sx1</machine>
> +      <machine>sx1-v1</machine>
> +      <machine>cheetah</machine>
> +      <machine>realview-eb</machine>
> +      <machine>realview-eb-mpcore</machine>
> +      <machine>realview-pb-a8</machine>
> +      <machine>realview-pbx-a9</machine>
> +      <machine>akita</machine>
> +      <machine>spitz</machine>
> +      <machine>borzoi</machine>
> +      <machine>terrier</machine>
> +      <machine>lm3s811evb</machine>
> +      <machine>lm3s6965evb</machine>
> +      <machine>tosa</machine>
> +      <machine>versatilepb</machine>
> +      <machine>versatileab</machine>
> +      <machine>vexpress-a9</machine>
> +      <machine>vexpress-a15</machine>
> +      <machine>xilinx-zynq-a9</machine>
> +      <machine>z2</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="microblaze">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-microblaze</emulator>
> +      <machine>petalogix-s3adsp1800</machine>
> +      <machine>petalogix-ml605</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="microblazeel">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-microblazeel</emulator>
> +      <machine>petalogix-s3adsp1800</machine>
> +      <machine>petalogix-ml605</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="mips">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-mips</emulator>
> +      <machine>malta</machine>
> +      <machine>magnum</machine>
> +      <machine>pica61</machine>
> +      <machine>mipssim</machine>
> +      <machine>mips</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="mipsel">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-mipsel</emulator>
> +      <machine>malta</machine>
> +      <machine>magnum</machine>
> +      <machine>pica61</machine>
> +      <machine>mipssim</machine>
> +      <machine>mips</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="sparc">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-sparc</emulator>
> +      <machine>SS-5</machine>
> +      <machine>leon3_generic</machine>
> +      <machine>SS-10</machine>
> +      <machine>SS-600MP</machine>
> +      <machine>SS-20</machine>
> +      <machine>Voyager</machine>
> +      <machine>LX</machine>
> +      <machine>SS-4</machine>
> +      <machine>SPARCClassic</machine>
> +      <machine>SPARCbook</machine>
> +      <machine>SS-1000</machine>
> +      <machine>SS-2000</machine>
> +      <machine>SS-2</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="ppc">
> +      <wordsize>32</wordsize>
> +      <emulator>/usr/bin/qemu-system-ppc</emulator>
> +      <machine>g3beige</machine>
> +      <machine>ref405ep</machine>
> +      <machine>taihu</machine>
> +      <machine>bamboo</machine>
> +      <machine>mac99</machine>
> +      <machine>prep</machine>
> +      <machine>mpc8544ds</machine>
> +      <machine>virtex-ml507</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="ppc64">
> +      <wordsize>64</wordsize>
> +      <emulator>/usr/bin/qemu-system-ppc64</emulator>
> +      <machine>mac99</machine>
> +      <machine>ref405ep</machine>
> +      <machine>taihu</machine>
> +      <machine>bamboo</machine>
> +      <machine>g3beige</machine>
> +      <machine>prep</machine>
> +      <machine>mpc8544ds</machine>
> +      <machine>virtex-ml507</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +    <features>
> +      <cpuselection/>
> +    </features>
> +  </guest>
> +
> +  <guest>
> +    <os_type>hvm</os_type>
> +    <arch name="s390x">
> +      <wordsize>64</wordsize>
> +      <emulator>/usr/bin/qemu-system-s390x</emulator>
> +      <machine>s390-virtio</machine>
> +      <machine canonical="s390-virtio">s390</machine>
> +      <domain type="qemu">
> +      </domain>
> +    </arch>
> +  </guest>
> +
> +</capabilities>
> +
> +

And I'd remove these 2 blank lines.

ACK with these issues fixed.

Christophe

Attachment: pgp930aJGYxt3.pgp
Description: PGP signature


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