[Libvirt-cim] [PATCH 1/4] VSSD: Add properties for arch and machine

Viktor Mihajlovski mihajlov at linux.vnet.ibm.com
Thu Aug 15 14:48:19 UTC 2013


For architectures like s390 the machine type is relevant for
the proper guest construction. We add the necessary properties
to the schema and the C structures and the necessary code
for CIM-to-libvirt mapping.

While doing this I noticed that the union fields in os_info
were set by means of XML parsing which doesn't take into account
that certain fields are depending on the virtualization type.
This could lead both to memory overwrites and memory leaks.
Fixed by using temporary variables and type-based setting of fields

Signed-off-by: Viktor Mihajlovski <mihajlov at linux.vnet.ibm.com>
---
 libxkutil/device_parsing.c                |   85 ++++++++++++++++++++++-------
 libxkutil/device_parsing.h                |    2 +
 libxkutil/xmlgen.c                        |    6 ++
 schema/VSSD.mof                           |    6 ++
 src/Virt_VSSD.c                           |    9 +++
 src/Virt_VirtualSystemManagementService.c |   14 +++++
 6 files changed, 101 insertions(+), 21 deletions(-)

diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
index ffdf682..df7a87a 100644
--- a/libxkutil/device_parsing.c
+++ b/libxkutil/device_parsing.c
@@ -1077,23 +1077,41 @@ int parse_fq_devid(const char *devid, char **host, char **device)
         return 1;
 }
 
+static void cleanup_bootlist(char **blist, unsigned blist_ct)
+{
+        while (blist_ct > 0) {
+                free(blist[--blist_ct]);
+        }
+        free(blist);
+}
+
 static int parse_os(struct domain *dominfo, xmlNode *os)
 {
         xmlNode *child;
         char **blist = NULL;
         unsigned bl_size = 0;
+        char *arch = NULL;
+        char *machine = NULL;
+        char *kernel = NULL;
+        char *initrd = NULL;
+        char *cmdline = NULL;
+        char *loader = NULL;
+        char *boot = NULL;
+        char *init = NULL;
 
         for (child = os->children; child != NULL; child = child->next) {
-                if (XSTREQ(child->name, "type"))
+                if (XSTREQ(child->name, "type")) {
                         STRPROP(dominfo, os_info.pv.type, child);
-                else if (XSTREQ(child->name, "kernel"))
-                        STRPROP(dominfo, os_info.pv.kernel, child);
+                        arch = get_attr_value(child, "arch");
+                        machine = get_attr_value(child, "machine");
+                } else if (XSTREQ(child->name, "kernel"))
+                        kernel = get_node_content(child);
                 else if (XSTREQ(child->name, "initrd"))
-                        STRPROP(dominfo, os_info.pv.initrd, child);
+                        initrd = get_node_content(child);
                 else if (XSTREQ(child->name, "cmdline"))
-                        STRPROP(dominfo, os_info.pv.cmdline, child);
+                        cmdline = get_node_content(child);
                 else if (XSTREQ(child->name, "loader"))
-                        STRPROP(dominfo, os_info.fv.loader, child);
+                        loader = get_node_content(child);
                 else if (XSTREQ(child->name, "boot")) {
                         char **tmp_list = NULL;
 
@@ -1111,7 +1129,7 @@ static int parse_os(struct domain *dominfo, xmlNode *os)
                         blist[bl_size] = get_attr_value(child, "dev");
                         bl_size++;
                 } else if (XSTREQ(child->name, "init"))
-                        STRPROP(dominfo, os_info.lxc.init, child);
+                        init = get_node_content(child);
         }
 
         if ((STREQC(dominfo->os_info.fv.type, "hvm")) &&
@@ -1128,17 +1146,45 @@ static int parse_os(struct domain *dominfo, xmlNode *os)
         else
                 dominfo->type = -1;
 
-        if (STREQC(dominfo->os_info.fv.type, "hvm")) {
+        switch (dominfo->type) {
+        case DOMAIN_XENFV:
+        case DOMAIN_KVM:
+        case DOMAIN_QEMU:
+                dominfo->os_info.fv.loader = loader;
+                dominfo->os_info.fv.arch = arch;
+                dominfo->os_info.fv.machine = machine;
                 dominfo->os_info.fv.bootlist_ct = bl_size;
                 dominfo->os_info.fv.bootlist = blist;
-        } else {
-            int i;
-
-            for (i = 0; i < bl_size; i++)
-                free(blist[i]);
-            free(blist);
+                loader = NULL;
+                arch = NULL;
+                machine = NULL;
+                blist = NULL;
+                bl_size = 0;
+                break;
+        case DOMAIN_XENPV:
+                dominfo->os_info.pv.kernel = kernel;
+                dominfo->os_info.pv.initrd = initrd;
+                dominfo->os_info.pv.cmdline = cmdline;
+                kernel = NULL;
+                initrd = NULL;
+                cmdline = NULL;
+                break;
+        case DOMAIN_LXC:
+                dominfo->os_info.lxc.init = init;
+                init = NULL;
+                break;
+        default:
+                break;
         }
 
+        free(arch);
+        free(machine);
+        free(kernel);
+        free(initrd);
+        free(cmdline);
+        free(boot);
+        free(init);
+        cleanup_bootlist(blist, bl_size);
         return 1;
 }
 
@@ -1334,15 +1380,12 @@ void cleanup_dominfo(struct domain **dominfo)
                 free(dom->os_info.pv.cmdline);
         } else if ((dom->type == DOMAIN_XENFV) ||
                    (dom->type == DOMAIN_KVM) || (dom->type == DOMAIN_QEMU)) {
-                int i;
-
                 free(dom->os_info.fv.type);
                 free(dom->os_info.fv.loader);
-                
-                for (i = 0; i < dom->os_info.fv.bootlist_ct; i++) {
-                        free(dom->os_info.fv.bootlist[i]);
-                } 
-                free(dom->os_info.fv.bootlist);
+                free(dom->os_info.fv.arch);
+                free(dom->os_info.fv.machine);
+                cleanup_bootlist(dom->os_info.fv.bootlist,
+                                 dom->os_info.fv.bootlist_ct);
         } else if (dom->type == DOMAIN_LXC) {
                 free(dom->os_info.lxc.type);
                 free(dom->os_info.lxc.init);
diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h
index 2b6d3d1..379d48c 100644
--- a/libxkutil/device_parsing.h
+++ b/libxkutil/device_parsing.h
@@ -136,6 +136,8 @@ struct pv_os_info {
 
 struct fv_os_info {
         char *type; /* Should always be 'hvm' */
+        char *arch;
+        char *machine;
         char *loader;
         unsigned bootlist_ct;
         char **bootlist;
diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
index 4287d42..f19830f 100644
--- a/libxkutil/xmlgen.c
+++ b/libxkutil/xmlgen.c
@@ -802,6 +802,12 @@ static char *_kvm_os_xml(xmlNodePtr root, struct domain *domain)
         if (tmp == NULL)
                 return XML_ERROR;
 
+        if (os->arch)
+                xmlNewProp(tmp, BAD_CAST "arch", BAD_CAST os->arch);
+
+        if (os->machine)
+                xmlNewProp(tmp, BAD_CAST "machine", BAD_CAST os->machine);
+
         ret = _fv_bootlist_xml(root, os);
         if (ret == 0)
                 return XML_ERROR;
diff --git a/schema/VSSD.mof b/schema/VSSD.mof
index 0359d67..2734d8e 100644
--- a/schema/VSSD.mof
+++ b/schema/VSSD.mof
@@ -48,6 +48,12 @@ class KVM_VirtualSystemSettingData : Virt_VirtualSystemSettingData
   [Description ("The emulator the guest should use during runtime.")]
   string Emulator;
 
+  [Description ("The guest's architecture.")]
+  string Arch;
+
+  [Description ("The guest's machine type")]
+  string Machine;
+
 };
 
 [Description (
diff --git a/src/Virt_VSSD.c b/src/Virt_VSSD.c
index 3363b38..67e56aa 100644
--- a/src/Virt_VSSD.c
+++ b/src/Virt_VSSD.c
@@ -121,6 +121,15 @@ static CMPIStatus _set_fv_prop(const CMPIBroker *broker,
                 goto out;
         }
 
+        if (dominfo->os_info.fv.arch != NULL)
+                CMSetProperty(inst, "Arch",
+                              (CMPIValue *)dominfo->os_info.fv.arch,
+                              CMPI_chars);
+
+        if (dominfo->os_info.fv.machine != NULL)
+                CMSetProperty(inst, "Machine",
+                              (CMPIValue *)dominfo->os_info.fv.machine,
+                              CMPI_chars);
  out:
         return s;
 }
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
index 8ced2d6..3df878f 100644
--- a/src/Virt_VirtualSystemManagementService.c
+++ b/src/Virt_VirtualSystemManagementService.c
@@ -543,6 +543,20 @@ static int fv_vssd_to_domain(CMPIInstance *inst,
         if (!fv_set_emulator(domain, val))
                 return 0;
 
+        free(domain->os_info.fv.arch);
+        ret = cu_get_str_prop(inst, "Arch", &val);
+        if (ret == CMPI_RC_OK)
+                domain->os_info.fv.arch = strdup(val);
+        else
+                domain->os_info.fv.arch = NULL;
+
+        free(domain->os_info.fv.machine);
+        ret = cu_get_str_prop(inst, "Machine", &val);
+        if (ret == CMPI_RC_OK)
+                domain->os_info.fv.machine = strdup(val);
+        else
+                domain->os_info.fv.machine = NULL;
+
         return 1;
 }
 
-- 
1.7.9.5




More information about the Libvirt-cim mailing list