[PATCHv2 0/5] Initial Enablement of S390

The current libvirt-cim implementation makes some assumptions that are only true for x86 architectures. As we want to enable libvirt-cim for s390 we need to makes sure that valid libvirt guest definitions are being built for that architecture while not breaking the existing implementation. Patch 1 fixes potential memory access problems. Patches 2 and 3 introduce two new properties arch and machine, effectively a pass-through of the underlying libvirt properties, for the necessary distinction between x86 and other guests, and suppress the default framebuffer for s390. Patches 4 and 5 make sure that a minimal SVPC guest definition (VSSD and RASD) will result in a correct libvirt guest definition for the current hypervisor. Boris Fiuczynski (2): libxkutil: Provide easy access to the libvirt capabilities VSSM: Set default values based on libvirt capabilities on DefineSystem calls Viktor Mihajlovski (3): libxkutil: Improve domain.os_info cleanup VSSD: Add properties for arch and machine S390: Avoid the generation of default input and graphics libxkutil/Makefile.am | 2 + libxkutil/capability_parsing.c | 556 +++++++++++++++++++++++++++++ libxkutil/capability_parsing.h | 97 +++++ libxkutil/device_parsing.c | 114 +++--- libxkutil/device_parsing.h | 4 +- libxkutil/xml_parse_test.c | 201 ++++++++++- libxkutil/xmlgen.c | 6 + schema/VSSD.mof | 6 + src/Virt_VSSD.c | 9 + src/Virt_VirtualSystemManagementService.c | 165 +++++---- 10 files changed, 1033 insertions(+), 127 deletions(-) create mode 100644 libxkutil/capability_parsing.c create mode 100644 libxkutil/capability_parsing.h V2 Changes # Split original patch 1 into 2 patches (fix, new feature), otherwise unchanged # Patch 3 also considers QEMU domains now, was KVM only # Patches 4 and 5 address memory leaks and overwrites # cimtest run completes without regressions on x86 -- 1.7.9.5

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@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 73 +++++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 21 deletions(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index ffdf682..500c724 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -1077,23 +1077,37 @@ 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 *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); + } 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 +1125,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 +1142,39 @@ 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.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; + 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(kernel); + free(initrd); + free(cmdline); + free(boot); + free(init); + cleanup_bootlist(blist, bl_size); return 1; } @@ -1334,15 +1370,10 @@ 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); + 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); -- 1.7.9.5

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. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 12 ++++++++++++ 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, 49 insertions(+) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 500c724..df7a87a 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -1090,6 +1090,8 @@ 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; @@ -1100,6 +1102,8 @@ static int parse_os(struct domain *dominfo, xmlNode *os) for (child = os->children; child != NULL; child = child->next) { if (XSTREQ(child->name, "type")) { STRPROP(dominfo, os_info.pv.type, 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")) @@ -1147,9 +1151,13 @@ static int parse_os(struct domain *dominfo, xmlNode *os) 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; loader = NULL; + arch = NULL; + machine = NULL; blist = NULL; bl_size = 0; break; @@ -1169,6 +1177,8 @@ static int parse_os(struct domain *dominfo, xmlNode *os) break; } + free(arch); + free(machine); free(kernel); free(initrd); free(cmdline); @@ -1372,6 +1382,8 @@ void cleanup_dominfo(struct domain **dominfo) (dom->type == DOMAIN_KVM) || (dom->type == DOMAIN_QEMU)) { free(dom->os_info.fv.type); free(dom->os_info.fv.loader); + 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) { 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

KVM guests for the s390 architecture do not support graphics and input devices. We use the os_info.fv.arch property to recognize such guests and skip the default device generation for those. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_VirtualSystemManagementService.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) V2 Changes: suppress for KVM and QEMU s390x domains, was KVM only diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 3df878f..301f046 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -583,6 +583,12 @@ static bool default_graphics_device(struct domain *domain) if (domain->type == DOMAIN_LXC) return true; + if ((domain->type == DOMAIN_KVM || domain->type == DOMAIN_QEMU) && + domain->os_info.fv.arch != NULL && + (XSTREQ(domain->os_info.fv.arch, "s390") || + XSTREQ(domain->os_info.fv.arch, "s390x" ))) + return true; + free(domain->dev_graphics); domain->dev_graphics = calloc(1, sizeof(*domain->dev_graphics)); if (domain->dev_graphics == NULL) { @@ -605,6 +611,12 @@ static bool default_input_device(struct domain *domain) if (domain->type == DOMAIN_LXC) return true; + if ((domain->type == DOMAIN_KVM || domain->type == DOMAIN_QEMU) && + domain->os_info.fv.arch != NULL && + (XSTREQ(domain->os_info.fv.arch, "s390") || + XSTREQ(domain->os_info.fv.arch, "s390x" ))) + return true; + free(domain->dev_input); domain->dev_input = calloc(1, sizeof(*domain->dev_input)); if (domain->dev_input == NULL) { -- 1.7.9.5

From: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Introspecting the libvirt capabilities and creating an internal capabilities data structure. Methods are provided for retrieving default values regarding architecture, machine and emulator for easy of use in the provider code. Changed the KVM detection to use the capabilities instead of unique XML parsing. Further, xml_parse_test was extendend to display hypervisor capabilities and defaults. Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- libxkutil/Makefile.am | 2 + libxkutil/capability_parsing.c | 556 ++++++++++++++++++++++++++++++++++++++++ libxkutil/capability_parsing.h | 97 +++++++ libxkutil/device_parsing.c | 29 --- libxkutil/device_parsing.h | 2 - libxkutil/xml_parse_test.c | 201 ++++++++++++++- 6 files changed, 853 insertions(+), 34 deletions(-) create mode 100644 libxkutil/capability_parsing.c create mode 100644 libxkutil/capability_parsing.h V2 Changes + Removed memory leaks + Removed occurrence of crashes when multiple guests are defined + Corrected search pattern for default architecture and machine + Added host CPU architecture parsing + Extended xml_parse_test with better testing capabilities coverage + Wrong free on cap guest struct + Fixed use_kvm method diff --git a/libxkutil/Makefile.am b/libxkutil/Makefile.am index 8d436ad..dd7be55 100644 --- a/libxkutil/Makefile.am +++ b/libxkutil/Makefile.am @@ -7,6 +7,7 @@ noinst_HEADERS = \ cs_util.h \ misc_util.h \ device_parsing.h \ + capability_parsing.h \ xmlgen.h \ infostore.h \ pool_parsing.h \ @@ -20,6 +21,7 @@ libxkutil_la_SOURCES = \ cs_util_instance.c \ misc_util.c \ device_parsing.c \ + capability_parsing.c \ xmlgen.c \ infostore.c \ pool_parsing.c \ diff --git a/libxkutil/capability_parsing.c b/libxkutil/capability_parsing.c new file mode 100644 index 0000000..e3c0f2b --- /dev/null +++ b/libxkutil/capability_parsing.c @@ -0,0 +1,556 @@ +/* + * Copyright IBM Corp. 2013 + * + * Authors: + * Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +#include <stdio.h> +#include <string.h> +#include <stdlib.h> +#include <stdbool.h> +#include <inttypes.h> +#include <sys/stat.h> +#include <stdint.h> + +#include <libcmpiutil/libcmpiutil.h> +#include <libvirt/libvirt.h> +#include <libxml/xpath.h> +#include <libxml/parser.h> +#include <libxml/tree.h> + +#include "misc_util.h" +#include "capability_parsing.h" +#include "xmlgen.h" +#include "../src/svpc_types.h" + +static void cleanup_cap_machine(struct cap_machine *machine) +{ + if (machine == NULL) + return; + free(machine->name); + free(machine->canonical_name); +} + +static void cleanup_cap_domain_info(struct cap_domain_info *cgdi) +{ + int i; + if (cgdi == NULL) + return; + free(cgdi->emulator); + free(cgdi->loader); + for (i = 0; i < cgdi->num_machines; i++) + cleanup_cap_machine(&cgdi->machines[i]); + free(cgdi->machines); +} + +static void cleanup_cap_domain(struct cap_domain *cgd) +{ + if (cgd == NULL) + return; + free(cgd->typestr); + cleanup_cap_domain_info(&cgd->guest_domain_info); +} + +static void cleanup_cap_arch(struct cap_arch *cga) +{ + int i; + if (cga == NULL) + return; + free(cga->name); + cleanup_cap_domain_info(&cga->default_domain_info); + for (i = 0; i < cga->num_domains; i++) + cleanup_cap_domain(&cga->domains[i]); + free(cga->domains); +} + +static void cleanup_cap_guest(struct cap_guest *cg) +{ + if (cg == NULL) + return; + free(cg->ostype); + cleanup_cap_arch(&cg->arch); +} + +static void cleanup_cap_host(struct cap_host *ch) +{ + if (ch == NULL) + return; + free(ch->cpu_arch); +} + +void cleanup_capabilities(struct capabilities **caps) +{ + int i; + struct capabilities *cap; + + if ((caps == NULL) || (*caps == NULL)) + return; + + cap = *caps; + cleanup_cap_host(&cap->host); + for (i = 0; i < cap->num_guests; i++) + cleanup_cap_guest(&cap->guests[i]); + + free(cap->guests); + free(cap); + *caps = NULL; +} + +static void extend_cap_machines(struct cap_domain_info *cg_domaininfo, + char *name, char *canonical_name) +{ + struct cap_machine *tmp_list = NULL; + tmp_list = realloc(cg_domaininfo->machines, + (cg_domaininfo->num_machines + 1) * + sizeof(struct cap_machine)); + + if (tmp_list == NULL) { + /* Nothing you can do. Just go on. */ + CU_DEBUG("Could not alloc space for " + "guest domain info list"); + return; + } + cg_domaininfo->machines = tmp_list; + + struct cap_machine *cap_gm = + &cg_domaininfo->machines[cg_domaininfo->num_machines]; + cap_gm->name = name; + cap_gm->canonical_name = canonical_name; + cg_domaininfo->num_machines++; +} + +static void parse_cap_domain_info(struct cap_domain_info *cg_domaininfo, + xmlNode *domain_child_node) +{ + CU_DEBUG("Capabilities guest domain info element node: %s", + domain_child_node->name); + + if (XSTREQ(domain_child_node->name, "emulator")) { + cg_domaininfo->emulator = + get_node_content(domain_child_node); + } else if (XSTREQ(domain_child_node->name, "loader")) { + cg_domaininfo->loader = + get_node_content(domain_child_node); + } else if (XSTREQ(domain_child_node->name, "machine")) { + extend_cap_machines(cg_domaininfo, + get_node_content(domain_child_node), + get_attr_value(domain_child_node, + "canonical")); + } +} + +static void parse_cap_domain(struct cap_domain *cg_domain, + xmlNode *guest_dom) +{ + CU_DEBUG("Capabilities guest domain node: %s", guest_dom->name); + + xmlNode *child; + + cg_domain->typestr = get_attr_value(guest_dom, "type"); + + for (child = guest_dom->children; child != NULL; child = child->next) + parse_cap_domain_info(&cg_domain->guest_domain_info, child); +} + +static void parse_cap_arch(struct cap_arch *cg_archinfo, + xmlNode *arch) +{ + CU_DEBUG("Capabilities arch node: %s", arch->name); + + xmlNode *child; + + cg_archinfo->name = get_attr_value(arch, "name"); + + for (child = arch->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "wordsize")) { + char *wordsize_str; + unsigned int wordsize; + wordsize_str = get_node_content(child); + /* Default to 0 wordsize if garbage */ + if (wordsize_str == NULL || + sscanf(wordsize_str, "%i", &wordsize) != 1) + wordsize = 0; + free(wordsize_str); + cg_archinfo->wordsize = wordsize; + } else if (XSTREQ(child->name, "domain")) { + struct cap_domain *tmp_list = NULL; + tmp_list = realloc(cg_archinfo->domains, + (cg_archinfo->num_domains + 1) * + sizeof(struct cap_domain)); + if (tmp_list == NULL) { + /* Nothing you can do. Just go on. */ + CU_DEBUG("Could not alloc space for " + "guest domain"); + continue; + } + memset(&tmp_list[cg_archinfo->num_domains], + 0, sizeof(struct cap_domain)); + cg_archinfo->domains = tmp_list; + parse_cap_domain(&cg_archinfo-> + domains[cg_archinfo->num_domains], + child); + cg_archinfo->num_domains++; + } else { + /* Check for the default domain child nodes */ + parse_cap_domain_info(&cg_archinfo->default_domain_info, + child); + } + } +} + +static void parse_cap_guests(xmlNodeSet *nsv, struct cap_guest *cap_guests) +{ + xmlNode **nodes = nsv->nodeTab; + xmlNode *child; + int numGuestNodes = nsv->nodeNr; + int i; + + for (i = 0; i < numGuestNodes; i++) { + for (child = nodes[i]->children; child != NULL; + child = child->next) { + if (XSTREQ(child->name, "os_type")) { + STRPROP((&cap_guests[i]), ostype, child); + } else if (XSTREQ(child->name, "arch")) { + parse_cap_arch(&cap_guests[i].arch, child); + } + } + } +} + +static int parse_cap_host_cpu(struct cap_host *cap_host, xmlNode *cpu) +{ + xmlNode *child; + + for (child = cpu->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "arch")) { + cap_host->cpu_arch = get_node_content(child); + if (cap_host->cpu_arch != NULL) + return 1; /* success - host arch node found */ + else { + CU_DEBUG("Host architecture is not defined"); + break; + } + } + } + return 0; /* error - no arch node or empty arch node */ +} + +static int parse_cap_host(xmlNodeSet *nsv, struct cap_host *cap_host) +{ + xmlNode **nodes = nsv->nodeTab; + xmlNode *child; + if (nsv->nodeNr < 1) + return 0; /* error no node below host */ + + for (child = nodes[0]->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "cpu")) + return parse_cap_host_cpu(cap_host, child); + } + return 0; /* error - no cpu node */ +} + +static void compare_copy_domain_info_machines( + struct cap_domain_info *def_gdomi, + struct cap_domain_info *cap_gadomi) +{ + int i,j; + int org_l = cap_gadomi->num_machines; + char *cp_name = NULL; + char *cp_canonical_name = NULL; + bool found; + + for (i = 0; i < def_gdomi->num_machines; i++) { + found = false; + for (j = 0; j < org_l; j++) { + if (STREQC(def_gdomi->machines[i].name, + cap_gadomi->machines[j].name)) { + found = true; + continue; + /* found match => check next default */ + } + } + if (!found) { /* no match => insert default */ + cp_name = NULL; + cp_canonical_name = NULL; + if (def_gdomi->machines[i].name != NULL) + cp_name = strdup(def_gdomi->machines[i].name); + if (def_gdomi->machines[i].canonical_name != NULL) + cp_canonical_name = + strdup(def_gdomi-> + machines[i].canonical_name); + + extend_cap_machines(cap_gadomi, + cp_name, + cp_canonical_name); + } + } +} + +static void extend_defaults_cap_guests(struct capabilities *caps) +{ + struct cap_arch *cap_garch; + struct cap_domain_info *cap_gadomi; + struct cap_domain_info *def_gdomi; + int i,j; + + if (caps == NULL) + return; + + for (i = 0; i < caps->num_guests; i++) { + cap_garch = &caps->guests[i].arch; + def_gdomi = &cap_garch->default_domain_info; + + for (j = 0; j < cap_garch->num_domains; j++) { + /* compare guest_domain_info */ + cap_gadomi = &cap_garch->domains[j].guest_domain_info; + if (cap_gadomi->emulator == NULL && + def_gdomi->emulator != NULL) + cap_gadomi->emulator = + strdup(def_gdomi->emulator); + if (cap_gadomi->loader == NULL && + def_gdomi->loader != NULL) + cap_gadomi->loader = strdup(def_gdomi->loader); + + compare_copy_domain_info_machines(def_gdomi, + cap_gadomi); + } + } +} + +static int _get_capabilities(const char *xml, struct capabilities *caps) +{ + int len; + int ret = 0; + + xmlDoc *xmldoc = NULL; + xmlXPathContext *xpathctx = NULL; + xmlXPathObject *xpathobj = NULL; + const xmlChar *xpathhoststr = (xmlChar *)"//capabilities//host"; + const xmlChar *xpathgueststr = (xmlChar *)"//capabilities//guest"; + xmlNodeSet *nsv; + + len = strlen(xml) + 1; + + if ((xmldoc = xmlParseMemory(xml, len)) == NULL) + goto err; + + if ((xpathctx = xmlXPathNewContext(xmldoc)) == NULL) + goto err; + + /* host node */ + if ((xpathobj = xmlXPathEvalExpression(xpathhoststr, xpathctx)) == NULL) + goto err; + if (xmlXPathNodeSetIsEmpty(xpathobj->nodesetval)) { + CU_DEBUG("No capabilities host node found!"); + goto err; + } + + nsv = xpathobj->nodesetval; + if (!parse_cap_host(nsv, &caps->host)) + goto err; + xmlXPathFreeObject(xpathobj); + + /* all guest nodes */ + if ((xpathobj = xmlXPathEvalExpression(xpathgueststr, xpathctx)) == NULL) + goto err; + if (xmlXPathNodeSetIsEmpty(xpathobj->nodesetval)) { + CU_DEBUG("No capabilities guest nodes found!"); + goto err; + } + + nsv = xpathobj->nodesetval; + caps->guests = calloc(nsv->nodeNr, sizeof(struct cap_guest)); + if (caps->guests == NULL) + goto err; + caps->num_guests = nsv->nodeNr; + + parse_cap_guests(nsv, caps->guests); + extend_defaults_cap_guests(caps); + ret = 1; + + err: + xmlXPathFreeObject(xpathobj); + xmlXPathFreeContext(xpathctx); + xmlFreeDoc(xmldoc); + return ret; +} + +int get_caps_from_xml(const char *xml, struct capabilities **caps) +{ + CU_DEBUG("In get_caps_from_xml"); + + free(*caps); + *caps = calloc(1, sizeof(struct capabilities)); + if (*caps == NULL) + goto err; + + if (_get_capabilities(xml, *caps) == 0) + goto err; + + return 1; + + err: + free(*caps); + *caps = NULL; + return 0; +} + +int get_capabilities(virConnectPtr conn, struct capabilities **caps) +{ + char *caps_xml = NULL; + int ret = 0; + + if (conn == NULL) { + CU_DEBUG("Unable to connect to libvirt."); + return 0; + } + + caps_xml = virConnectGetCapabilities(conn); + + if (caps_xml == NULL) { + CU_DEBUG("Unable to get capabilities xml."); + return 0; + } + + ret = get_caps_from_xml(caps_xml, caps); + + free(caps_xml); + + return ret; +} + +struct cap_domain_info *findDomainInfo(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type) +{ + int i,j; + struct cap_arch *ar; + + for (i = 0; i < caps->num_guests; i++) { + if (os_type == NULL || + STREQC(caps->guests[i].ostype, os_type)) { + ar = &caps->guests[i].arch; + if (arch == NULL || STREQC(ar->name,arch)) + for (j = 0; j < ar->num_domains; j++) + if (domain_type == NULL || + STREQC(ar->domains[j].typestr, + domain_type)) + return &ar->domains[j]. + guest_domain_info; + } + } + return NULL; +} + +static char *_findDefArch(struct capabilities *caps, + const char *os_type, + const char *host_arch) +{ + char *ret = NULL; + int i; + + if (os_type == NULL) + return NULL; + + for (i = 0; i < caps->num_guests; i++) + if (STREQC(caps->guests[i].ostype, os_type) && + (host_arch == NULL || (host_arch != NULL && + STREQC(caps->guests[i].arch.name, host_arch)))) { + ret = caps->guests[i].arch.name; + break; + } + return ret; +} + +char *get_default_arch(struct capabilities *caps, + const char *os_type) +{ + char *ret = NULL; + + if (caps != NULL) { + if (os_type != NULL) { + /* search first guest matching os_type and host arch */ + ret = _findDefArch(caps, os_type, caps->host.cpu_arch); + if (ret== NULL) /* search first matching guest */ + ret = _findDefArch(caps, os_type, NULL); + } + } + return ret; +} + +char *get_default_machine( + struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type) +{ + char *ret = NULL; + struct cap_domain_info *di; + + if (caps != NULL) { + di = findDomainInfo(caps, os_type, arch, domain_type); + if (di != NULL && di->num_machines > 0) { + ret = di->machines[0].canonical_name; + if (ret == NULL) { + ret = di->machines[0].name; + } + } + } + return ret; +} + +char *get_default_emulator(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type) +{ + char *ret = NULL; + struct cap_domain_info *di; + + if (caps != NULL) { + di = findDomainInfo(caps, os_type, arch, domain_type); + if (di != NULL) + ret = di->emulator; + } + return ret; +} + +bool use_kvm(struct capabilities *caps) { + if (host_supports_kvm(caps) && !get_disable_kvm()) + return true; + else + return false; +} + +bool host_supports_kvm(struct capabilities *caps) +{ + bool kvm = false; + if (caps != NULL) + if (findDomainInfo(caps, NULL, NULL, "kvm") != NULL) + kvm = true; + return kvm; +} +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */ diff --git a/libxkutil/capability_parsing.h b/libxkutil/capability_parsing.h new file mode 100644 index 0000000..d258f62 --- /dev/null +++ b/libxkutil/capability_parsing.h @@ -0,0 +1,97 @@ +/* + * Copyright IBM Corp. 2013 + * + * Authors: + * Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +#ifndef __CAPABILITY_PARSING_H +#define __CAPABILITY_PARSING_H + +#include <stdint.h> +#include <stdbool.h> + +struct cap_host { + char *cpu_arch; +}; + +struct cap_machine { + char *name; + char *canonical_name; +}; + +struct cap_domain_info { + char *emulator; + char *loader; + int num_machines; + struct cap_machine *machines; +}; + +struct cap_domain { + char *typestr; + struct cap_domain_info guest_domain_info; +}; + +struct cap_arch { + char *name; + unsigned int wordsize; + struct cap_domain_info default_domain_info; + int num_domains; + struct cap_domain *domains; +}; + +struct cap_guest { + char *ostype; + struct cap_arch arch; +}; + +struct capabilities { + struct cap_host host; + int num_guests; + struct cap_guest *guests; +}; + +int get_caps_from_xml(const char *xml, struct capabilities **caps); +int get_capabilities(virConnectPtr conn, struct capabilities **caps); +char *get_default_arch(struct capabilities *caps, + const char *os_type); +char *get_default_machine(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type); +char *get_default_emulator(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type); +struct cap_domain_info *findDomainInfo(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type); +bool use_kvm(struct capabilities *caps); +bool host_supports_kvm(struct capabilities *caps); +void cleanup_capabilities(struct capabilities **caps); + +#endif + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */ diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index df7a87a..449f51c 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -397,35 +397,6 @@ err: return 0; } -bool has_kvm_domain_type(xmlNodePtr node) -{ - xmlNodePtr child = NULL; - char *type = NULL; - bool ret = false; - - child = node->children; - while (child != NULL) { - if (XSTREQ(child->name, "domain")) { - type = get_attr_value(child, "type"); - if (XSTREQ(type, "kvm")) { - ret = true; - goto out; - } - } - - if (has_kvm_domain_type(child) == 1) { - ret = true; - goto out; - } - - child = child->next; - } - - out: - free(type); - return ret; -} - static int parse_net_device(xmlNode *inode, struct virt_device **vdevs) { struct virt_device *vdev = NULL; diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h index 379d48c..a39e881 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -223,8 +223,6 @@ int attach_device(virDomainPtr dom, struct virt_device *dev); int detach_device(virDomainPtr dom, struct virt_device *dev); int change_device(virDomainPtr dom, struct virt_device *dev); -bool has_kvm_domain_type(xmlNodePtr node); - #define XSTREQ(x, y) (STREQ((char *)x, y)) #define STRPROP(d, p, n) (d->p = get_node_content(n)) diff --git a/libxkutil/xml_parse_test.c b/libxkutil/xml_parse_test.c index 384593d..de2c88a 100644 --- a/libxkutil/xml_parse_test.c +++ b/libxkutil/xml_parse_test.c @@ -6,6 +6,7 @@ #include <libvirt/libvirt.h> #include "device_parsing.h" +#include "capability_parsing.h" #include "xmlgen.h" static void print_value(FILE *d, const char *name, const char *val) @@ -25,6 +26,36 @@ static void print_u32(FILE *d, const char *name, uint32_t val) } #endif +static char *get_ostype(struct domain *dom) +{ + if (dom->type == DOMAIN_XENPV) { + return dom->os_info.pv.type; + } else if ((dom->type == DOMAIN_XENFV) || + (dom->type == DOMAIN_KVM) || + (dom->type == DOMAIN_QEMU)) { + return dom->os_info.fv.type; + } else if (dom->type == DOMAIN_LXC) { + return dom->os_info.lxc.type; + } else { + return NULL; + } +} + +static char *get_domaintype(struct domain *dom) +{ + if (dom->type == DOMAIN_XENPV || dom->type == DOMAIN_XENFV) { + return "xen"; + } else if (dom->type == DOMAIN_KVM) { + return "kvm"; + } else if (dom->type == DOMAIN_QEMU) { + return "qemu"; + } else if (dom->type == DOMAIN_LXC) { + return "lxc"; + } else { + return NULL; + } +} + static void print_os(struct domain *dom, FILE *d) { @@ -183,6 +214,98 @@ static char *read_from_file(FILE *file) return xml; } +static void print_cap_domain_info(struct cap_domain_info *capgdiinfo, + FILE *d) +{ + struct cap_machine capgminfo; + int i; + + if (capgdiinfo==NULL) + return; + + if (capgdiinfo->emulator!=NULL) + print_value(d, " Emulator", capgdiinfo->emulator); + if (capgdiinfo->loader!=NULL) + print_value(d, " Loader", capgdiinfo->loader); + for (i = 0; i < capgdiinfo->num_machines; i++) { + capgminfo = capgdiinfo->machines[i]; + fprintf(d, " Machine name : %-15s canonical name : %s\n", + capgminfo.name, capgminfo.canonical_name); + } + fprintf(d, "\n"); +} + +static void print_cap_domains(struct cap_arch caparchinfo, + FILE *d) +{ + struct cap_domain capgdinfo; + int i; + for (i = 0; i < caparchinfo.num_domains; i++) { + capgdinfo = caparchinfo.domains[i]; + print_value(d, " Type", capgdinfo.typestr); + print_cap_domain_info(&capgdinfo.guest_domain_info, d); + } +} + +static void print_cap_arch(struct cap_arch caparchinfo, + FILE *d) +{ + print_value(d, " Arch name", caparchinfo.name); + fprintf(d, " Arch wordsize : %i\n", caparchinfo.wordsize); + fprintf(d, "\n -- Default guest domain settings --\n"); + print_cap_domain_info(&caparchinfo.default_domain_info, d); + fprintf(d, " -- Guest domains (%i) --\n", caparchinfo.num_domains); + print_cap_domains(caparchinfo, d); +} + +static void print_cap_guest(struct cap_guest *capginfo, + FILE *d) +{ + print_value(d, "Guest OS type", capginfo->ostype); + print_cap_arch(capginfo->arch, d); +} + +static void print_cap_host(struct cap_host *caphinfo, + FILE *d) +{ + print_value(d, "Host CPU architecture", caphinfo->cpu_arch); +} + +static void print_capabilities(struct capabilities *capsinfo, + FILE *d) +{ + int i; + fprintf(d, "\n### Capabilities ###\n"); + fprintf(d, "-- Host --\n"); + print_cap_host(&capsinfo->host, d); + fprintf(d, "\n-- Guest (%i) --\n", capsinfo->num_guests); + for (i = 0; i < capsinfo->num_guests; i++) + print_cap_guest(&capsinfo->guests[i], d); +} + +static int capinfo_for_dom(const char *uri, + struct domain *dominfo, + struct capabilities **capsinfo) +{ + virConnectPtr conn = NULL; + char *caps_xml = NULL; + int ret = 0; + + conn = virConnectOpen(uri); + if (conn == NULL) { + printf("Unable to connect to libvirt\n"); + goto out; + } + + ret = get_capabilities(conn, capsinfo); + + out: + free(caps_xml); + virConnectClose(conn); + + return ret; +} + static int dominfo_from_dom(const char *uri, const char *domain, struct domain **d) @@ -246,12 +369,13 @@ static int dominfo_from_file(const char *fname, struct domain **d) static void usage(void) { printf("xml_parse_test -f [FILE | -] [--xml]\n" - "xml_parse_test -d domain [--uri URI] [--xml]\n" + "xml_parse_test -d domain [--uri URI] [--xml] [--cap]\n" "\n" "-f,--file FILE Parse domain XML from file (or stdin if -)\n" "-d,--domain DOM Display dominfo for a domain from libvirt\n" "-u,--uri URI Connect to libvirt with URI\n" "-x,--xml Dump generated XML instead of summary\n" + "-c,--cap Display the libvirt default capability values for the specified domain\n" "-h,--help Display this help message\n"); } @@ -262,7 +386,10 @@ int main(int argc, char **argv) char *uri = "xen"; char *file = NULL; bool xml = false; + bool cap = false; struct domain *dominfo = NULL; + struct capabilities *capsinfo = NULL; + struct cap_domain_info *capgdinfo = NULL; int ret; static struct option lopts[] = { @@ -270,13 +397,14 @@ int main(int argc, char **argv) {"uri", 1, 0, 'u'}, {"xml", 0, 0, 'x'}, {"file", 1, 0, 'f'}, + {"cap", 0, 0, 'c'}, {"help", 0, 0, 'h'}, {0, 0, 0, 0}}; while (1) { int optidx = 0; - c = getopt_long(argc, argv, "d:u:f:xh", lopts, &optidx); + c = getopt_long(argc, argv, "d:u:f:xch", lopts, &optidx); if (c == -1) break; @@ -297,11 +425,14 @@ int main(int argc, char **argv) xml = true; break; + case 'c': + cap = true; + break; + case '?': case 'h': usage(); return c == '?'; - }; } @@ -326,6 +457,70 @@ int main(int argc, char **argv) print_devices(dominfo, stdout); } + if (cap && file == NULL) { + ret = capinfo_for_dom(uri, dominfo, &capsinfo); + if (ret == 0) { + printf("Unable to get capsinfo\n"); + return 3; + } else { + print_capabilities(capsinfo, stdout); + const char *os_type = get_ostype(dominfo); + const char *dom_type = get_domaintype(dominfo); + const char *def_arch = get_default_arch(capsinfo, os_type); + + fprintf(stdout, "-- KVM is used: %s\n\n", (use_kvm(capsinfo)?"true":"false")); + fprintf(stdout, "-- For all following default OS type=%s\n", os_type); + fprintf(stdout, "-- Default Arch : %s\n", def_arch); + + fprintf(stdout, + "-- Default Machine for arch=NULL : %s\n", + get_default_machine(capsinfo, os_type, NULL, NULL)); + fprintf(stdout, + "-- Default Machine for arch=%s and domain type=NULL : %s\n", + def_arch, + get_default_machine(capsinfo, os_type, def_arch, NULL)); + fprintf(stdout, + "-- Default Machine for arch=%s and domain type=%s : %s\n", + def_arch, dom_type, + get_default_machine(capsinfo, os_type, def_arch, dom_type)); + fprintf(stdout, + "-- Default Machine for arch=NULL and domain type=%s : %s\n", + dom_type, + get_default_machine(capsinfo, os_type, NULL, dom_type)); + + fprintf(stdout, + "-- Default Emulator for arch=NULL : %s\n", + get_default_emulator(capsinfo, os_type, NULL, NULL)); + fprintf(stdout, + "-- Default Emulator for arch=%s and domain type=NULL : %s\n", + def_arch, + get_default_emulator(capsinfo, os_type, def_arch, NULL)); + fprintf(stdout, + "-- Default Emulator for arch=%s and domain type=%s : %s\n", + def_arch, dom_type, + get_default_emulator(capsinfo, os_type, def_arch, dom_type)); + fprintf(stdout, + "-- Default Emulator for arch=NULL and domain type=%s : %s\n", + dom_type, + get_default_emulator(capsinfo, os_type, NULL, dom_type)); + + fprintf(stdout, "\n-- Default Domain Search for: \n" + "guest type=hvm - guest arch=* - guest domain type=kvm\n"); + capgdinfo = findDomainInfo(capsinfo, "hvm", NULL, "kvm"); + print_cap_domain_info(capgdinfo, stdout); + + fprintf(stdout, "-- Default Domain Search for: \n" + "guest type=* - guest arch=* - guest domain type=*\n"); + capgdinfo = findDomainInfo(capsinfo, NULL, NULL, NULL); + print_cap_domain_info(capgdinfo, stdout); + + cleanup_capabilities(&capsinfo); + } + } else if (cap) { + printf("Need a data source (--domain) to get default capabilities\n"); + return 4; + } + return 0; } -- 1.7.9.5

On 08/29/2013 11:18 AM, Viktor Mihajlovski wrote:
From: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Introspecting the libvirt capabilities and creating an internal capabilities data structure. Methods are provided for retrieving default values regarding architecture, machine and emulator for easy of use in the provider code.
Changed the KVM detection to use the capabilities instead of unique XML parsing.
Further, xml_parse_test was extendend to display hypervisor capabilities and defaults.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
--- libxkutil/Makefile.am | 2 + libxkutil/capability_parsing.c | 556 ++++++++++++++++++++++++++++++++++++++++ libxkutil/capability_parsing.h | 97 +++++++ libxkutil/device_parsing.c | 29 --- libxkutil/device_parsing.h | 2 - libxkutil/xml_parse_test.c | 201 ++++++++++++++- 6 files changed, 853 insertions(+), 34 deletions(-) create mode 100644 libxkutil/capability_parsing.c create mode 100644 libxkutil/capability_parsing.h
V2 Changes + Removed memory leaks + Removed occurrence of crashes when multiple guests are defined + Corrected search pattern for default architecture and machine + Added host CPU architecture parsing + Extended xml_parse_test with better testing capabilities coverage + Wrong free on cap guest struct + Fixed use_kvm method
diff --git a/libxkutil/Makefile.am b/libxkutil/Makefile.am index 8d436ad..dd7be55 100644 --- a/libxkutil/Makefile.am +++ b/libxkutil/Makefile.am @@ -7,6 +7,7 @@ noinst_HEADERS = \ cs_util.h \ misc_util.h \ device_parsing.h \ + capability_parsing.h \ xmlgen.h \ infostore.h \ pool_parsing.h \ @@ -20,6 +21,7 @@ libxkutil_la_SOURCES = \ cs_util_instance.c \ misc_util.c \ device_parsing.c \ + capability_parsing.c \ xmlgen.c \ infostore.c \ pool_parsing.c \ diff --git a/libxkutil/capability_parsing.c b/libxkutil/capability_parsing.c new file mode 100644 index 0000000..e3c0f2b --- /dev/null +++ b/libxkutil/capability_parsing.c @@ -0,0 +1,556 @@ +/* + * Copyright IBM Corp. 2013 + * + * Authors: + * Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
Forgot to mention this last time - I went through an exercise where I removed the address here since the above is apparently their old address.... See commit id '391634e2c'. The difference is essentially: - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. Also see: http://www.gnu.org/licenses/gpl-howto.html
+ */ +#include <stdio.h> +#include <string.h> +#include <stdlib.h> +#include <stdbool.h> +#include <inttypes.h> +#include <sys/stat.h> +#include <stdint.h> + +#include <libcmpiutil/libcmpiutil.h> +#include <libvirt/libvirt.h> +#include <libxml/xpath.h> +#include <libxml/parser.h> +#include <libxml/tree.h> + +#include "misc_util.h" +#include "capability_parsing.h" +#include "xmlgen.h" +#include "../src/svpc_types.h" + +static void cleanup_cap_machine(struct cap_machine *machine) +{ + if (machine == NULL) + return; + free(machine->name); + free(machine->canonical_name); +} + +static void cleanup_cap_domain_info(struct cap_domain_info *cgdi) +{ + int i; + if (cgdi == NULL) + return; + free(cgdi->emulator); + free(cgdi->loader); + for (i = 0; i < cgdi->num_machines; i++) + cleanup_cap_machine(&cgdi->machines[i]); + free(cgdi->machines); +} + +static void cleanup_cap_domain(struct cap_domain *cgd) +{ + if (cgd == NULL) + return; + free(cgd->typestr); + cleanup_cap_domain_info(&cgd->guest_domain_info); +} + +static void cleanup_cap_arch(struct cap_arch *cga) +{ + int i; + if (cga == NULL) + return; + free(cga->name); + cleanup_cap_domain_info(&cga->default_domain_info); + for (i = 0; i < cga->num_domains; i++) + cleanup_cap_domain(&cga->domains[i]); + free(cga->domains); +} + +static void cleanup_cap_guest(struct cap_guest *cg) +{ + if (cg == NULL) + return; + free(cg->ostype); + cleanup_cap_arch(&cg->arch); +} + +static void cleanup_cap_host(struct cap_host *ch) +{ + if (ch == NULL) + return; + free(ch->cpu_arch); +} + +void cleanup_capabilities(struct capabilities **caps) +{ + int i; + struct capabilities *cap; + + if ((caps == NULL) || (*caps == NULL)) + return; + + cap = *caps; + cleanup_cap_host(&cap->host); + for (i = 0; i < cap->num_guests; i++) + cleanup_cap_guest(&cap->guests[i]); + + free(cap->guests); + free(cap); + *caps = NULL; +} + +static void extend_cap_machines(struct cap_domain_info *cg_domaininfo, + char *name, char *canonical_name) +{ + struct cap_machine *tmp_list = NULL; + tmp_list = realloc(cg_domaininfo->machines, + (cg_domaininfo->num_machines + 1) * + sizeof(struct cap_machine)); + + if (tmp_list == NULL) { + /* Nothing you can do. Just go on. */ + CU_DEBUG("Could not alloc space for " + "guest domain info list"); + return; + } + cg_domaininfo->machines = tmp_list; + + struct cap_machine *cap_gm = + &cg_domaininfo->machines[cg_domaininfo->num_machines]; + cap_gm->name = name; + cap_gm->canonical_name = canonical_name; + cg_domaininfo->num_machines++; +} + +static void parse_cap_domain_info(struct cap_domain_info *cg_domaininfo, + xmlNode *domain_child_node) +{ + CU_DEBUG("Capabilities guest domain info element node: %s", + domain_child_node->name); + + if (XSTREQ(domain_child_node->name, "emulator")) { + cg_domaininfo->emulator = + get_node_content(domain_child_node); + } else if (XSTREQ(domain_child_node->name, "loader")) { + cg_domaininfo->loader = + get_node_content(domain_child_node); + } else if (XSTREQ(domain_child_node->name, "machine")) { + extend_cap_machines(cg_domaininfo, + get_node_content(domain_child_node), + get_attr_value(domain_child_node, + "canonical")); + } +} + +static void parse_cap_domain(struct cap_domain *cg_domain, + xmlNode *guest_dom) +{ + CU_DEBUG("Capabilities guest domain node: %s", guest_dom->name); + + xmlNode *child; + + cg_domain->typestr = get_attr_value(guest_dom, "type"); + + for (child = guest_dom->children; child != NULL; child = child->next) + parse_cap_domain_info(&cg_domain->guest_domain_info, child); +} + +static void parse_cap_arch(struct cap_arch *cg_archinfo, + xmlNode *arch) +{ + CU_DEBUG("Capabilities arch node: %s", arch->name); + + xmlNode *child; + + cg_archinfo->name = get_attr_value(arch, "name"); + + for (child = arch->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "wordsize")) { + char *wordsize_str; + unsigned int wordsize; + wordsize_str = get_node_content(child); + /* Default to 0 wordsize if garbage */ + if (wordsize_str == NULL || + sscanf(wordsize_str, "%i", &wordsize) != 1) + wordsize = 0; + free(wordsize_str); + cg_archinfo->wordsize = wordsize; + } else if (XSTREQ(child->name, "domain")) { + struct cap_domain *tmp_list = NULL; + tmp_list = realloc(cg_archinfo->domains, + (cg_archinfo->num_domains + 1) * + sizeof(struct cap_domain)); + if (tmp_list == NULL) { + /* Nothing you can do. Just go on. */ + CU_DEBUG("Could not alloc space for " + "guest domain"); + continue; + } + memset(&tmp_list[cg_archinfo->num_domains], + 0, sizeof(struct cap_domain)); + cg_archinfo->domains = tmp_list; + parse_cap_domain(&cg_archinfo-> + domains[cg_archinfo->num_domains], + child); + cg_archinfo->num_domains++; + } else { + /* Check for the default domain child nodes */ + parse_cap_domain_info(&cg_archinfo->default_domain_info, + child); + } + } +} + +static void parse_cap_guests(xmlNodeSet *nsv, struct cap_guest *cap_guests) +{ + xmlNode **nodes = nsv->nodeTab; + xmlNode *child; + int numGuestNodes = nsv->nodeNr; + int i; + + for (i = 0; i < numGuestNodes; i++) { + for (child = nodes[i]->children; child != NULL; + child = child->next) { + if (XSTREQ(child->name, "os_type")) { + STRPROP((&cap_guests[i]), ostype, child); + } else if (XSTREQ(child->name, "arch")) { + parse_cap_arch(&cap_guests[i].arch, child); + } + } + } +} + +static int parse_cap_host_cpu(struct cap_host *cap_host, xmlNode *cpu) +{ + xmlNode *child; + + for (child = cpu->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "arch")) { + cap_host->cpu_arch = get_node_content(child); + if (cap_host->cpu_arch != NULL) + return 1; /* success - host arch node found */ + else { + CU_DEBUG("Host architecture is not defined"); + break; + } + } + } + return 0; /* error - no arch node or empty arch node */ +} + +static int parse_cap_host(xmlNodeSet *nsv, struct cap_host *cap_host) +{ + xmlNode **nodes = nsv->nodeTab; + xmlNode *child; + if (nsv->nodeNr < 1) + return 0; /* error no node below host */ + + for (child = nodes[0]->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "cpu")) + return parse_cap_host_cpu(cap_host, child); + } + return 0; /* error - no cpu node */ +} + +static void compare_copy_domain_info_machines( + struct cap_domain_info *def_gdomi, + struct cap_domain_info *cap_gadomi) +{ + int i,j; + int org_l = cap_gadomi->num_machines; + char *cp_name = NULL; + char *cp_canonical_name = NULL; + bool found; + + for (i = 0; i < def_gdomi->num_machines; i++) { + found = false; + for (j = 0; j < org_l; j++) { + if (STREQC(def_gdomi->machines[i].name, + cap_gadomi->machines[j].name)) { + found = true; + continue; + /* found match => check next default */ + } + } + if (!found) { /* no match => insert default */ + cp_name = NULL; + cp_canonical_name = NULL; + if (def_gdomi->machines[i].name != NULL) + cp_name = strdup(def_gdomi->machines[i].name); + if (def_gdomi->machines[i].canonical_name != NULL) + cp_canonical_name = + strdup(def_gdomi-> + machines[i].canonical_name); + + extend_cap_machines(cap_gadomi, + cp_name, + cp_canonical_name); + } + } +} + +static void extend_defaults_cap_guests(struct capabilities *caps) +{ + struct cap_arch *cap_garch; + struct cap_domain_info *cap_gadomi; + struct cap_domain_info *def_gdomi; + int i,j; + + if (caps == NULL) + return; + + for (i = 0; i < caps->num_guests; i++) { + cap_garch = &caps->guests[i].arch; + def_gdomi = &cap_garch->default_domain_info; + + for (j = 0; j < cap_garch->num_domains; j++) { + /* compare guest_domain_info */ + cap_gadomi = &cap_garch->domains[j].guest_domain_info; + if (cap_gadomi->emulator == NULL && + def_gdomi->emulator != NULL) + cap_gadomi->emulator = + strdup(def_gdomi->emulator); + if (cap_gadomi->loader == NULL && + def_gdomi->loader != NULL) + cap_gadomi->loader = strdup(def_gdomi->loader); + + compare_copy_domain_info_machines(def_gdomi, + cap_gadomi); + } + } +} + +static int _get_capabilities(const char *xml, struct capabilities *caps) +{ + int len; + int ret = 0; + + xmlDoc *xmldoc = NULL; + xmlXPathContext *xpathctx = NULL; + xmlXPathObject *xpathobj = NULL; + const xmlChar *xpathhoststr = (xmlChar *)"//capabilities//host"; + const xmlChar *xpathgueststr = (xmlChar *)"//capabilities//guest"; + xmlNodeSet *nsv; + + len = strlen(xml) + 1; + + if ((xmldoc = xmlParseMemory(xml, len)) == NULL) + goto err; + + if ((xpathctx = xmlXPathNewContext(xmldoc)) == NULL) + goto err; + + /* host node */ + if ((xpathobj = xmlXPathEvalExpression(xpathhoststr, xpathctx)) == NULL) + goto err; + if (xmlXPathNodeSetIsEmpty(xpathobj->nodesetval)) { + CU_DEBUG("No capabilities host node found!"); + goto err; + } + + nsv = xpathobj->nodesetval; + if (!parse_cap_host(nsv, &caps->host)) + goto err; + xmlXPathFreeObject(xpathobj); + + /* all guest nodes */ + if ((xpathobj = xmlXPathEvalExpression(xpathgueststr, xpathctx)) == NULL) + goto err; + if (xmlXPathNodeSetIsEmpty(xpathobj->nodesetval)) { + CU_DEBUG("No capabilities guest nodes found!"); + goto err; + } + + nsv = xpathobj->nodesetval; + caps->guests = calloc(nsv->nodeNr, sizeof(struct cap_guest)); + if (caps->guests == NULL) + goto err; + caps->num_guests = nsv->nodeNr; + + parse_cap_guests(nsv, caps->guests); + extend_defaults_cap_guests(caps); + ret = 1; + + err: + xmlXPathFreeObject(xpathobj); + xmlXPathFreeContext(xpathctx); + xmlFreeDoc(xmldoc); + return ret; +} + +int get_caps_from_xml(const char *xml, struct capabilities **caps) +{ + CU_DEBUG("In get_caps_from_xml"); + + free(*caps); + *caps = calloc(1, sizeof(struct capabilities)); + if (*caps == NULL) + goto err; + + if (_get_capabilities(xml, *caps) == 0) + goto err; + + return 1; + + err: + free(*caps); + *caps = NULL; + return 0; +} + +int get_capabilities(virConnectPtr conn, struct capabilities **caps) +{ + char *caps_xml = NULL; + int ret = 0; + + if (conn == NULL) { + CU_DEBUG("Unable to connect to libvirt."); + return 0; + } + + caps_xml = virConnectGetCapabilities(conn); + + if (caps_xml == NULL) { + CU_DEBUG("Unable to get capabilities xml."); + return 0; + } + + ret = get_caps_from_xml(caps_xml, caps); + + free(caps_xml); + + return ret; +} + +struct cap_domain_info *findDomainInfo(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type) +{ + int i,j; + struct cap_arch *ar; + + for (i = 0; i < caps->num_guests; i++) { + if (os_type == NULL || + STREQC(caps->guests[i].ostype, os_type)) { + ar = &caps->guests[i].arch; + if (arch == NULL || STREQC(ar->name,arch)) + for (j = 0; j < ar->num_domains; j++) + if (domain_type == NULL || + STREQC(ar->domains[j].typestr, + domain_type)) + return &ar->domains[j]. + guest_domain_info; + } + } + return NULL; +} + +static char *_findDefArch(struct capabilities *caps, + const char *os_type, + const char *host_arch) +{ + char *ret = NULL; + int i; + + if (os_type == NULL) + return NULL;
Technically unnecessary... See comment below
+ + for (i = 0; i < caps->num_guests; i++)
I know there's just one statement inside, but consider using {} for consistency and on the off chance someone misses this in the future.
+ if (STREQC(caps->guests[i].ostype, os_type) && + (host_arch == NULL || (host_arch != NULL && + STREQC(caps->guests[i].arch.name, host_arch)))) { + ret = caps->guests[i].arch.name; + break; + } + return ret; +} + +char *get_default_arch(struct capabilities *caps, + const char *os_type) +{ + char *ret = NULL; + + if (caps != NULL) { + if (os_type != NULL) {
Checks could be combined - reducing indention a bit.
+ /* search first guest matching os_type and host arch */ + ret = _findDefArch(caps, os_type, caps->host.cpu_arch); + if (ret== NULL) /* search first matching guest */
Need space ^
+ ret = _findDefArch(caps, os_type, NULL);
Since this is the only caller to _findDefArch and we know "caps != NULL && os_type != NULL", then the check in _findDefArch() becomes unnecessary
+ } + } + return ret; +} + +char *get_default_machine( + struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type) +{ + char *ret = NULL; + struct cap_domain_info *di; + + if (caps != NULL) { + di = findDomainInfo(caps, os_type, arch, domain_type); + if (di != NULL && di->num_machines > 0) { + ret = di->machines[0].canonical_name; + if (ret == NULL) { + ret = di->machines[0].name; + }
Alternatively ret = di->machines[0].canonical_name ? di->machines[0].canonical_name : di->machines[0].name;
+ } + } + return ret; +} + +char *get_default_emulator(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type) +{ + char *ret = NULL; + struct cap_domain_info *di; + + if (caps != NULL) { + di = findDomainInfo(caps, os_type, arch, domain_type); + if (di != NULL) + ret = di->emulator; + } + return ret; +} + +bool use_kvm(struct capabilities *caps) { + if (host_supports_kvm(caps) && !get_disable_kvm()) + return true; + else
else is unnecessary
+ return false; +} + +bool host_supports_kvm(struct capabilities *caps) +{ + bool kvm = false; + if (caps != NULL)
Again consider {} here so for consistency and prevention...
+ if (findDomainInfo(caps, NULL, NULL, "kvm") != NULL) + kvm = true; + return kvm; +} +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */ diff --git a/libxkutil/capability_parsing.h b/libxkutil/capability_parsing.h new file mode 100644 index 0000000..d258f62 --- /dev/null +++ b/libxkutil/capability_parsing.h @@ -0,0 +1,97 @@ +/* + * Copyright IBM Corp. 2013 + * + * Authors: + * Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
Adjust the info here too.
+ */ +#ifndef __CAPABILITY_PARSING_H +#define __CAPABILITY_PARSING_H + +#include <stdint.h> +#include <stdbool.h> + +struct cap_host { + char *cpu_arch; +}; + +struct cap_machine { + char *name; + char *canonical_name; +}; + +struct cap_domain_info { + char *emulator; + char *loader; + int num_machines; + struct cap_machine *machines; +}; + +struct cap_domain { + char *typestr; + struct cap_domain_info guest_domain_info; +}; + +struct cap_arch { + char *name; + unsigned int wordsize; + struct cap_domain_info default_domain_info; + int num_domains; + struct cap_domain *domains; +}; + +struct cap_guest { + char *ostype; + struct cap_arch arch; +}; + +struct capabilities { + struct cap_host host; + int num_guests; + struct cap_guest *guests; +}; + +int get_caps_from_xml(const char *xml, struct capabilities **caps); +int get_capabilities(virConnectPtr conn, struct capabilities **caps); +char *get_default_arch(struct capabilities *caps, + const char *os_type); +char *get_default_machine(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type); +char *get_default_emulator(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type); +struct cap_domain_info *findDomainInfo(struct capabilities *caps, + const char *os_type, + const char *arch, + const char *domain_type); +bool use_kvm(struct capabilities *caps); +bool host_supports_kvm(struct capabilities *caps); +void cleanup_capabilities(struct capabilities **caps); + +#endif + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */ diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index df7a87a..449f51c 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -397,35 +397,6 @@ err: return 0; }
-bool has_kvm_domain_type(xmlNodePtr node) -{ - xmlNodePtr child = NULL; - char *type = NULL; - bool ret = false; - - child = node->children; - while (child != NULL) { - if (XSTREQ(child->name, "domain")) { - type = get_attr_value(child, "type"); - if (XSTREQ(type, "kvm")) { - ret = true; - goto out; - } - } - - if (has_kvm_domain_type(child) == 1) { - ret = true; - goto out; - } - - child = child->next; - } - - out: - free(type); - return ret; -} - static int parse_net_device(xmlNode *inode, struct virt_device **vdevs) { struct virt_device *vdev = NULL; diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h index 379d48c..a39e881 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -223,8 +223,6 @@ int attach_device(virDomainPtr dom, struct virt_device *dev); int detach_device(virDomainPtr dom, struct virt_device *dev); int change_device(virDomainPtr dom, struct virt_device *dev);
-bool has_kvm_domain_type(xmlNodePtr node); - #define XSTREQ(x, y) (STREQ((char *)x, y)) #define STRPROP(d, p, n) (d->p = get_node_content(n))
diff --git a/libxkutil/xml_parse_test.c b/libxkutil/xml_parse_test.c index 384593d..de2c88a 100644 --- a/libxkutil/xml_parse_test.c +++ b/libxkutil/xml_parse_test.c @@ -6,6 +6,7 @@ #include <libvirt/libvirt.h>
#include "device_parsing.h" +#include "capability_parsing.h" #include "xmlgen.h"
static void print_value(FILE *d, const char *name, const char *val) @@ -25,6 +26,36 @@ static void print_u32(FILE *d, const char *name, uint32_t val) } #endif
+static char *get_ostype(struct domain *dom) +{ + if (dom->type == DOMAIN_XENPV) { + return dom->os_info.pv.type; + } else if ((dom->type == DOMAIN_XENFV) || + (dom->type == DOMAIN_KVM) || + (dom->type == DOMAIN_QEMU)) { + return dom->os_info.fv.type; + } else if (dom->type == DOMAIN_LXC) { + return dom->os_info.lxc.type; + } else { + return NULL; + } +} + +static char *get_domaintype(struct domain *dom) +{ + if (dom->type == DOMAIN_XENPV || dom->type == DOMAIN_XENFV) { + return "xen"; + } else if (dom->type == DOMAIN_KVM) { + return "kvm"; + } else if (dom->type == DOMAIN_QEMU) { + return "qemu"; + } else if (dom->type == DOMAIN_LXC) { + return "lxc"; + } else { + return NULL; + } +} + static void print_os(struct domain *dom, FILE *d) { @@ -183,6 +214,98 @@ static char *read_from_file(FILE *file) return xml; }
+static void print_cap_domain_info(struct cap_domain_info *capgdiinfo, + FILE *d) +{ + struct cap_machine capgminfo; + int i; + + if (capgdiinfo==NULL)
NIT: No spacing (e.g. " == ")
+ return; + + if (capgdiinfo->emulator!=NULL)
Ditto
+ print_value(d, " Emulator", capgdiinfo->emulator); + if (capgdiinfo->loader!=NULL)
Ditto The remainder seemed fine. John
+ print_value(d, " Loader", capgdiinfo->loader); + for (i = 0; i < capgdiinfo->num_machines; i++) { + capgminfo = capgdiinfo->machines[i]; + fprintf(d, " Machine name : %-15s canonical name : %s\n", + capgminfo.name, capgminfo.canonical_name); + } + fprintf(d, "\n"); +} + +static void print_cap_domains(struct cap_arch caparchinfo, + FILE *d) +{ + struct cap_domain capgdinfo; + int i; + for (i = 0; i < caparchinfo.num_domains; i++) { + capgdinfo = caparchinfo.domains[i]; + print_value(d, " Type", capgdinfo.typestr); + print_cap_domain_info(&capgdinfo.guest_domain_info, d); + } +} + +static void print_cap_arch(struct cap_arch caparchinfo, + FILE *d) +{ + print_value(d, " Arch name", caparchinfo.name); + fprintf(d, " Arch wordsize : %i\n", caparchinfo.wordsize); + fprintf(d, "\n -- Default guest domain settings --\n"); + print_cap_domain_info(&caparchinfo.default_domain_info, d); + fprintf(d, " -- Guest domains (%i) --\n", caparchinfo.num_domains); + print_cap_domains(caparchinfo, d); +} + +static void print_cap_guest(struct cap_guest *capginfo, + FILE *d) +{ + print_value(d, "Guest OS type", capginfo->ostype); + print_cap_arch(capginfo->arch, d); +} + +static void print_cap_host(struct cap_host *caphinfo, + FILE *d) +{ + print_value(d, "Host CPU architecture", caphinfo->cpu_arch); +} + +static void print_capabilities(struct capabilities *capsinfo, + FILE *d) +{ + int i; + fprintf(d, "\n### Capabilities ###\n"); + fprintf(d, "-- Host --\n"); + print_cap_host(&capsinfo->host, d); + fprintf(d, "\n-- Guest (%i) --\n", capsinfo->num_guests); + for (i = 0; i < capsinfo->num_guests; i++) + print_cap_guest(&capsinfo->guests[i], d); +} + +static int capinfo_for_dom(const char *uri, + struct domain *dominfo, + struct capabilities **capsinfo) +{ + virConnectPtr conn = NULL; + char *caps_xml = NULL; + int ret = 0; + + conn = virConnectOpen(uri); + if (conn == NULL) { + printf("Unable to connect to libvirt\n"); + goto out; + } + + ret = get_capabilities(conn, capsinfo); + + out: + free(caps_xml); + virConnectClose(conn); + + return ret; +} + static int dominfo_from_dom(const char *uri, const char *domain, struct domain **d) @@ -246,12 +369,13 @@ static int dominfo_from_file(const char *fname, struct domain **d) static void usage(void) { printf("xml_parse_test -f [FILE | -] [--xml]\n" - "xml_parse_test -d domain [--uri URI] [--xml]\n" + "xml_parse_test -d domain [--uri URI] [--xml] [--cap]\n" "\n" "-f,--file FILE Parse domain XML from file (or stdin if -)\n" "-d,--domain DOM Display dominfo for a domain from libvirt\n" "-u,--uri URI Connect to libvirt with URI\n" "-x,--xml Dump generated XML instead of summary\n" + "-c,--cap Display the libvirt default capability values for the specified domain\n" "-h,--help Display this help message\n"); }
@@ -262,7 +386,10 @@ int main(int argc, char **argv) char *uri = "xen"; char *file = NULL; bool xml = false; + bool cap = false; struct domain *dominfo = NULL; + struct capabilities *capsinfo = NULL; + struct cap_domain_info *capgdinfo = NULL; int ret;
static struct option lopts[] = { @@ -270,13 +397,14 @@ int main(int argc, char **argv) {"uri", 1, 0, 'u'}, {"xml", 0, 0, 'x'}, {"file", 1, 0, 'f'}, + {"cap", 0, 0, 'c'}, {"help", 0, 0, 'h'}, {0, 0, 0, 0}};
while (1) { int optidx = 0;
- c = getopt_long(argc, argv, "d:u:f:xh", lopts, &optidx); + c = getopt_long(argc, argv, "d:u:f:xch", lopts, &optidx); if (c == -1) break;
@@ -297,11 +425,14 @@ int main(int argc, char **argv) xml = true; break;
+ case 'c': + cap = true; + break; + case '?': case 'h': usage(); return c == '?'; - }; }
@@ -326,6 +457,70 @@ int main(int argc, char **argv) print_devices(dominfo, stdout); }
+ if (cap && file == NULL) { + ret = capinfo_for_dom(uri, dominfo, &capsinfo); + if (ret == 0) { + printf("Unable to get capsinfo\n"); + return 3; + } else { + print_capabilities(capsinfo, stdout); + const char *os_type = get_ostype(dominfo); + const char *dom_type = get_domaintype(dominfo); + const char *def_arch = get_default_arch(capsinfo, os_type); + + fprintf(stdout, "-- KVM is used: %s\n\n", (use_kvm(capsinfo)?"true":"false")); + fprintf(stdout, "-- For all following default OS type=%s\n", os_type); + fprintf(stdout, "-- Default Arch : %s\n", def_arch); + + fprintf(stdout, + "-- Default Machine for arch=NULL : %s\n", + get_default_machine(capsinfo, os_type, NULL, NULL)); + fprintf(stdout, + "-- Default Machine for arch=%s and domain type=NULL : %s\n", + def_arch, + get_default_machine(capsinfo, os_type, def_arch, NULL)); + fprintf(stdout, + "-- Default Machine for arch=%s and domain type=%s : %s\n", + def_arch, dom_type, + get_default_machine(capsinfo, os_type, def_arch, dom_type)); + fprintf(stdout, + "-- Default Machine for arch=NULL and domain type=%s : %s\n", + dom_type, + get_default_machine(capsinfo, os_type, NULL, dom_type)); + + fprintf(stdout, + "-- Default Emulator for arch=NULL : %s\n", + get_default_emulator(capsinfo, os_type, NULL, NULL)); + fprintf(stdout, + "-- Default Emulator for arch=%s and domain type=NULL : %s\n", + def_arch, + get_default_emulator(capsinfo, os_type, def_arch, NULL)); + fprintf(stdout, + "-- Default Emulator for arch=%s and domain type=%s : %s\n", + def_arch, dom_type, + get_default_emulator(capsinfo, os_type, def_arch, dom_type)); + fprintf(stdout, + "-- Default Emulator for arch=NULL and domain type=%s : %s\n", + dom_type, + get_default_emulator(capsinfo, os_type, NULL, dom_type)); + + fprintf(stdout, "\n-- Default Domain Search for: \n" + "guest type=hvm - guest arch=* - guest domain type=kvm\n"); + capgdinfo = findDomainInfo(capsinfo, "hvm", NULL, "kvm"); + print_cap_domain_info(capgdinfo, stdout); + + fprintf(stdout, "-- Default Domain Search for: \n" + "guest type=* - guest arch=* - guest domain type=*\n"); + capgdinfo = findDomainInfo(capsinfo, NULL, NULL, NULL); + print_cap_domain_info(capgdinfo, stdout); + + cleanup_capabilities(&capsinfo); + } + } else if (cap) { + printf("Need a data source (--domain) to get default capabilities\n"); + return 4; + } + return 0; }

Hi John, please try to squash in the following patch, courtesy of Boris -- diff --git a/libxkutil/capability_parsing.c b/libxkutil/capability_parsing.c index e3c0f2b..2acd45b 100644 --- a/libxkutil/capability_parsing.c +++ b/libxkutil/capability_parsing.c @@ -15,8 +15,8 @@ * 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, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. */ #include <stdio.h> #include <string.h> @@ -464,16 +464,14 @@ static char *_findDefArch(struct capabilities *caps, char *ret = NULL; int i; - if (os_type == NULL) - return NULL; - - for (i = 0; i < caps->num_guests; i++) + for (i = 0; i < caps->num_guests; i++) { if (STREQC(caps->guests[i].ostype, os_type) && (host_arch == NULL || (host_arch != NULL && STREQC(caps->guests[i].arch.name, host_arch)))) { ret = caps->guests[i].arch.name; break; } + } return ret; } @@ -482,13 +480,11 @@ char *get_default_arch(struct capabilities *caps, { char *ret = NULL; - if (caps != NULL) { - if (os_type != NULL) { - /* search first guest matching os_type and host arch */ - ret = _findDefArch(caps, os_type, caps->host.cpu_arch); - if (ret== NULL) /* search first matching guest */ - ret = _findDefArch(caps, os_type, NULL); - } + if (caps != NULL && os_type != NULL) { + /* search first guest matching os_type and host arch */ + ret = _findDefArch(caps, os_type, caps->host.cpu_arch); + if (ret == NULL) /* search first matching guest */ + ret = _findDefArch(caps, os_type, NULL); } return ret; } @@ -505,10 +501,9 @@ char *get_default_machine( if (caps != NULL) { di = findDomainInfo(caps, os_type, arch, domain_type); if (di != NULL && di->num_machines > 0) { - ret = di->machines[0].canonical_name; - if (ret == NULL) { - ret = di->machines[0].name; - } + ret = di->machines[0].canonical_name ? + di->machines[0].canonical_name : + di->machines[0].name; } } return ret; @@ -533,16 +528,16 @@ char *get_default_emulator(struct capabilities *caps, bool use_kvm(struct capabilities *caps) { if (host_supports_kvm(caps) && !get_disable_kvm()) return true; - else - return false; + return false; } bool host_supports_kvm(struct capabilities *caps) { bool kvm = false; - if (caps != NULL) + if (caps != NULL) { if (findDomainInfo(caps, NULL, NULL, "kvm") != NULL) kvm = true; + } return kvm; } /* diff --git a/libxkutil/capability_parsing.h b/libxkutil/capability_parsing.h index d258f62..41a4933 100644 --- a/libxkutil/capability_parsing.h +++ b/libxkutil/capability_parsing.h @@ -15,8 +15,8 @@ * 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, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. */ #ifndef __CAPABILITY_PARSING_H #define __CAPABILITY_PARSING_H diff --git a/libxkutil/xml_parse_test.c b/libxkutil/xml_parse_test.c index de2c88a..af5e508 100644 --- a/libxkutil/xml_parse_test.c +++ b/libxkutil/xml_parse_test.c @@ -220,12 +220,12 @@ static void print_cap_domain_info(struct cap_domain_info *capgdiinfo, struct cap_machine capgminfo; int i; - if (capgdiinfo==NULL) + if (capgdiinfo == NULL) return; - if (capgdiinfo->emulator!=NULL) + if (capgdiinfo->emulator != NULL) print_value(d, " Emulator", capgdiinfo->emulator); - if (capgdiinfo->loader!=NULL) + if (capgdiinfo->loader != NULL) print_value(d, " Loader", capgdiinfo->loader); for (i = 0; i < capgdiinfo->num_machines; i++) { capgminfo = capgdiinfo->machines[i]; Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> In the DefineSystem call the architecture, machine and emulator for KVM are set to the hypervisor-specific default values if they did not get provided. This now allows architecture based decision making in the CIM providers to work for all platforms. Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_VirtualSystemManagementService.c | 161 ++++++++++++++--------------- 1 file changed, 78 insertions(+), 83 deletions(-) V2 Changes + Removed memory leaks + Restricted setting machine and emulator defaults to KVM/QEMU cases diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 301f046..53b9691 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -35,6 +35,7 @@ #include "cs_util.h" #include "misc_util.h" #include "device_parsing.h" +#include "capability_parsing.h" #include "xmlgen.h" #include <libcmpiutil/libcmpiutil.h> @@ -388,59 +389,6 @@ static bool fv_set_emulator(struct domain *domain, return true; } -static bool system_has_kvm(const char *pfx) -{ - CMPIStatus s; - virConnectPtr conn = NULL; - char *caps = NULL; - bool disable_kvm = get_disable_kvm(); - xmlDocPtr doc = NULL; - xmlNodePtr node = NULL; - int len; - bool kvm = false; - - /* sometimes disable KVM to avoid problem in nested KVM */ - if (disable_kvm) { - CU_DEBUG("Enter disable kvm mode!"); - goto out; - } - - conn = connect_by_classname(_BROKER, pfx, &s); - if ((conn == NULL) || (s.rc != CMPI_RC_OK)) { - goto out; - } - - caps = virConnectGetCapabilities(conn); - if (caps != NULL) { - len = strlen(caps) + 1; - - doc = xmlParseMemory(caps, len); - if (doc == NULL) { - CU_DEBUG("xmlParseMemory() call failed!"); - goto out; - } - - node = xmlDocGetRootElement(doc); - if (node == NULL) { - CU_DEBUG("xmlDocGetRootElement() call failed!"); - goto out; - } - - if (has_kvm_domain_type(node)) { - CU_DEBUG("The system support kvm!"); - kvm = true; - } - } - -out: - free(caps); - free(doc); - - virConnectClose(conn); - - return kvm; -} - static int bootord_vssd_to_domain(CMPIInstance *inst, struct domain *domain) { @@ -511,53 +459,91 @@ static int bootord_vssd_to_domain(CMPIInstance *inst, static int fv_vssd_to_domain(CMPIInstance *inst, struct domain *domain, - const char *pfx) + const char *pfx, + virConnectPtr conn) { - int ret; + int ret = 1; + int retr; const char *val; + const char *domtype; + const char *ostype = "hvm"; + struct capabilities *capsinfo = NULL; + + get_capabilities(conn, &capsinfo); if (STREQC(pfx, "KVM")) { - if (system_has_kvm(pfx)) + if (use_kvm(capsinfo)) { domain->type = DOMAIN_KVM; - else + domtype = "kvm"; + } else { domain->type = DOMAIN_QEMU; + domtype = "qemu"; + } } else if (STREQC(pfx, "Xen")) { domain->type = DOMAIN_XENFV; + domtype = NULL; } else { CU_DEBUG("Unknown fullvirt domain type: %s", pfx); - return 0; + ret = 0; + goto out; } - ret = bootord_vssd_to_domain(inst, domain); - if (ret != 1) - return 0; - - ret = cu_get_str_prop(inst, "Emulator", &val); - if (ret != CMPI_RC_OK) - val = NULL; - else if (disk_type_from_file(val) == DISK_UNKNOWN) { - CU_DEBUG("Emulator path does not exist: %s", val); - return 0; + retr = bootord_vssd_to_domain(inst, domain); + if (retr != 1) { + ret = 0; + goto out; } - 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) + retr = cu_get_str_prop(inst, "Arch", &val); + if (retr != CMPI_RC_OK) { + if (capsinfo != NULL) { /* set default */ + val = get_default_arch(capsinfo, ostype); + CU_DEBUG("Set Arch to default: %s", val); + } else + val = NULL; + } + if (val != NULL) 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) + retr = cu_get_str_prop(inst, "Machine", &val); + if (retr != CMPI_RC_OK) { + if (capsinfo != NULL && domtype != NULL) { /* set default */ + val = get_default_machine(capsinfo, ostype, + domain->os_info.fv.arch, + domtype); + CU_DEBUG("Set Machine to default: %s", val); + } else + val = NULL; + } + if (val != NULL) domain->os_info.fv.machine = strdup(val); - else - domain->os_info.fv.machine = NULL; - return 1; + retr = cu_get_str_prop(inst, "Emulator", &val); + if (retr != CMPI_RC_OK) { + if (capsinfo != NULL && domtype != NULL) { /* set default */ + val = get_default_emulator(capsinfo, ostype, + domain->os_info.fv.arch, + domtype); + CU_DEBUG("Set Emulator to default: %s", val); + } else + val = NULL; + } + if (val != NULL && disk_type_from_file(val) == DISK_UNKNOWN) { + CU_DEBUG("Emulator path does not exist: %s", val); + ret = 0; + goto out; + } + + if (!fv_set_emulator(domain, val)) { + ret = 0; + goto out; + } + + out: + cleanup_capabilities(&capsinfo); + return ret; } static int lxc_vssd_to_domain(CMPIInstance *inst, @@ -663,6 +649,8 @@ static int vssd_to_domain(CMPIInstance *inst, bool bool_val; bool fullvirt; CMPIObjectPath *opathp = NULL; + virConnectPtr conn = NULL; + CMPIStatus s = { CMPI_RC_OK, NULL }; opathp = CMGetObjectPath(inst, NULL); @@ -748,9 +736,16 @@ static int vssd_to_domain(CMPIInstance *inst, } } - if (fullvirt || STREQC(pfx, "KVM")) - ret = fv_vssd_to_domain(inst, domain, pfx); - else if (STREQC(pfx, "Xen")) + if (fullvirt || STREQC(pfx, "KVM")) { + conn = connect_by_classname(_BROKER, cn, &s); + if (conn == NULL || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("libvirt connection failed"); + ret = 0; + goto out; + } + ret = fv_vssd_to_domain(inst, domain, pfx, conn); + virConnectClose(conn); + } else if (STREQC(pfx, "Xen")) ret = xenpv_vssd_to_domain(inst, domain); else if (STREQC(pfx, "LXC")) ret = lxc_vssd_to_domain(inst, domain); -- 1.7.9.5

On 08/29/2013 11:18 AM, Viktor Mihajlovski wrote:
From: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
In the DefineSystem call the architecture, machine and emulator for KVM are set to the hypervisor-specific default values if they did not get provided. This now allows architecture based decision making in the CIM providers to work for all platforms.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
--- src/Virt_VirtualSystemManagementService.c | 161 ++++++++++++++--------------- 1 file changed, 78 insertions(+), 83 deletions(-)
V2 Changes + Removed memory leaks + Restricted setting machine and emulator defaults to KVM/QEMU cases
Two NITs below, but ACK in any case, John
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 301f046..53b9691 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -35,6 +35,7 @@ #include "cs_util.h" #include "misc_util.h" #include "device_parsing.h" +#include "capability_parsing.h" #include "xmlgen.h"
#include <libcmpiutil/libcmpiutil.h> @@ -388,59 +389,6 @@ static bool fv_set_emulator(struct domain *domain, return true; }
-static bool system_has_kvm(const char *pfx) -{ - CMPIStatus s; - virConnectPtr conn = NULL; - char *caps = NULL; - bool disable_kvm = get_disable_kvm(); - xmlDocPtr doc = NULL; - xmlNodePtr node = NULL; - int len; - bool kvm = false; - - /* sometimes disable KVM to avoid problem in nested KVM */ - if (disable_kvm) { - CU_DEBUG("Enter disable kvm mode!"); - goto out; - } - - conn = connect_by_classname(_BROKER, pfx, &s); - if ((conn == NULL) || (s.rc != CMPI_RC_OK)) { - goto out; - } - - caps = virConnectGetCapabilities(conn); - if (caps != NULL) { - len = strlen(caps) + 1; - - doc = xmlParseMemory(caps, len); - if (doc == NULL) { - CU_DEBUG("xmlParseMemory() call failed!"); - goto out; - } - - node = xmlDocGetRootElement(doc); - if (node == NULL) { - CU_DEBUG("xmlDocGetRootElement() call failed!"); - goto out; - } - - if (has_kvm_domain_type(node)) { - CU_DEBUG("The system support kvm!"); - kvm = true; - } - } - -out: - free(caps); - free(doc); - - virConnectClose(conn); - - return kvm; -} - static int bootord_vssd_to_domain(CMPIInstance *inst, struct domain *domain) { @@ -511,53 +459,91 @@ static int bootord_vssd_to_domain(CMPIInstance *inst,
static int fv_vssd_to_domain(CMPIInstance *inst, struct domain *domain, - const char *pfx) + const char *pfx, + virConnectPtr conn) { - int ret; + int ret = 1; + int retr; const char *val; + const char *domtype;
Could have just initialized == NULL; here...
+ const char *ostype = "hvm"; + struct capabilities *capsinfo = NULL; + + get_capabilities(conn, &capsinfo);
if (STREQC(pfx, "KVM")) { - if (system_has_kvm(pfx)) + if (use_kvm(capsinfo)) { domain->type = DOMAIN_KVM; - else + domtype = "kvm"; + } else { domain->type = DOMAIN_QEMU; + domtype = "qemu"; + } } else if (STREQC(pfx, "Xen")) { domain->type = DOMAIN_XENFV; + domtype = NULL;
Thus not needing this one... nor anyone else in the future forgetting to set/initialize it...
} else { CU_DEBUG("Unknown fullvirt domain type: %s", pfx); - return 0; + ret = 0; + goto out; }
- ret = bootord_vssd_to_domain(inst, domain); - if (ret != 1) - return 0; - - ret = cu_get_str_prop(inst, "Emulator", &val); - if (ret != CMPI_RC_OK) - val = NULL; - else if (disk_type_from_file(val) == DISK_UNKNOWN) { - CU_DEBUG("Emulator path does not exist: %s", val); - return 0; + retr = bootord_vssd_to_domain(inst, domain); + if (retr != 1) { + ret = 0; + goto out; }
- 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) + retr = cu_get_str_prop(inst, "Arch", &val); + if (retr != CMPI_RC_OK) { + if (capsinfo != NULL) { /* set default */ + val = get_default_arch(capsinfo, ostype); + CU_DEBUG("Set Arch to default: %s", val); + } else + val = NULL; + } + if (val != NULL) 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) + retr = cu_get_str_prop(inst, "Machine", &val); + if (retr != CMPI_RC_OK) { + if (capsinfo != NULL && domtype != NULL) { /* set default */ + val = get_default_machine(capsinfo, ostype, + domain->os_info.fv.arch, + domtype); + CU_DEBUG("Set Machine to default: %s", val); + } else + val = NULL; + } + if (val != NULL) domain->os_info.fv.machine = strdup(val); - else - domain->os_info.fv.machine = NULL;
- return 1; + retr = cu_get_str_prop(inst, "Emulator", &val); + if (retr != CMPI_RC_OK) { + if (capsinfo != NULL && domtype != NULL) { /* set default */ + val = get_default_emulator(capsinfo, ostype, + domain->os_info.fv.arch, + domtype); + CU_DEBUG("Set Emulator to default: %s", val); + } else + val = NULL; + } + if (val != NULL && disk_type_from_file(val) == DISK_UNKNOWN) { + CU_DEBUG("Emulator path does not exist: %s", val); + ret = 0; + goto out; + } + + if (!fv_set_emulator(domain, val)) { + ret = 0; + goto out; + } + + out: + cleanup_capabilities(&capsinfo); + return ret; }
static int lxc_vssd_to_domain(CMPIInstance *inst, @@ -663,6 +649,8 @@ static int vssd_to_domain(CMPIInstance *inst, bool bool_val; bool fullvirt; CMPIObjectPath *opathp = NULL; + virConnectPtr conn = NULL; + CMPIStatus s = { CMPI_RC_OK, NULL };
opathp = CMGetObjectPath(inst, NULL); @@ -748,9 +736,16 @@ static int vssd_to_domain(CMPIInstance *inst, } }
- if (fullvirt || STREQC(pfx, "KVM")) - ret = fv_vssd_to_domain(inst, domain, pfx); - else if (STREQC(pfx, "Xen")) + if (fullvirt || STREQC(pfx, "KVM")) { + conn = connect_by_classname(_BROKER, cn, &s); + if (conn == NULL || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("libvirt connection failed");
Since you're not returning CMPIStatus, then really no need to check s.rc although it's no big deal either as conn will be NULL and I see no way for conn to be non-NULL and s.rc != CMPI_RC_OK. If there were, then you'd have to close the connection...
+ ret = 0; + goto out; + } + ret = fv_vssd_to_domain(inst, domain, pfx, conn); + virConnectClose(conn); + } else if (STREQC(pfx, "Xen")) ret = xenpv_vssd_to_domain(inst, domain); else if (STREQC(pfx, "LXC")) ret = lxc_vssd_to_domain(inst, domain);

Hi John, could you squash this in? -- diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 53b9691..79dec73 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -465,7 +465,7 @@ static int fv_vssd_to_domain(CMPIInstance *inst, int ret = 1; int retr; const char *val; - const char *domtype; + const char *domtype = NULL; const char *ostype = "hvm"; struct capabilities *capsinfo = NULL; @@ -481,7 +481,6 @@ static int fv_vssd_to_domain(CMPIInstance *inst, } } else if (STREQC(pfx, "Xen")) { domain->type = DOMAIN_XENFV; - domtype = NULL; } else { CU_DEBUG("Unknown fullvirt domain type: %s", pfx); ret = 0; @@ -738,8 +737,10 @@ static int vssd_to_domain(CMPIInstance *inst, if (fullvirt || STREQC(pfx, "KVM")) { conn = connect_by_classname(_BROKER, cn, &s); - if (conn == NULL || (s.rc != CMPI_RC_OK)) { - CU_DEBUG("libvirt connection failed"); + if (conn == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Error connecting to libvirt"); ret = 0; goto out; } Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 08/29/2013 11:18 AM, Viktor Mihajlovski wrote:
The current libvirt-cim implementation makes some assumptions that are only true for x86 architectures. As we want to enable libvirt-cim for s390 we need to makes sure that valid libvirt guest definitions are being built for that architecture while not breaking the existing implementation.
Patch 1 fixes potential memory access problems.
Patches 2 and 3 introduce two new properties arch and machine, effectively a pass-through of the underlying libvirt properties, for the necessary distinction between x86 and other guests, and suppress the default framebuffer for s390.
Patches 4 and 5 make sure that a minimal SVPC guest definition (VSSD and RASD) will result in a correct libvirt guest definition for the current hypervisor.
Boris Fiuczynski (2): libxkutil: Provide easy access to the libvirt capabilities VSSM: Set default values based on libvirt capabilities on DefineSystem calls
Viktor Mihajlovski (3): libxkutil: Improve domain.os_info cleanup VSSD: Add properties for arch and machine S390: Avoid the generation of default input and graphics
libxkutil/Makefile.am | 2 + libxkutil/capability_parsing.c | 556 +++++++++++++++++++++++++++++ libxkutil/capability_parsing.h | 97 +++++ libxkutil/device_parsing.c | 114 +++--- libxkutil/device_parsing.h | 4 +- libxkutil/xml_parse_test.c | 201 ++++++++++- libxkutil/xmlgen.c | 6 + schema/VSSD.mof | 6 + src/Virt_VSSD.c | 9 + src/Virt_VirtualSystemManagementService.c | 165 +++++---- 10 files changed, 1033 insertions(+), 127 deletions(-) create mode 100644 libxkutil/capability_parsing.c create mode 100644 libxkutil/capability_parsing.h
V2 Changes # Split original patch 1 into 2 patches (fix, new feature), otherwise unchanged # Patch 3 also considers QEMU domains now, was KVM only # Patches 4 and 5 address memory leaks and overwrites # cimtest run completes without regressions on x86
Things look good - cimtest and coverity tests both were happy. Since I had already ACK'd patch 1 which is now patch 1 & 2, consider the ACK still valid. Also the old patch 2 ACK is still valid even with the self find of the missing qemu check. I posted nits/questions separately about patches 4 & 5. Since I believe you still need me to push, feel free to post either just a set of diffs for me to squash into or a v3. Is there a desire to get this into a RHEL release eventually or into the "next" RHEL release? I'd rather let this "soak" a while and then work on getting a complete libvirt-cim into a future 6.n release over what I have to do now which is patch 0.6.1 with 0.6.3 based patches... John

On 08/29/2013 10:52 PM, John Ferlan wrote:
Things look good - cimtest and coverity tests both were happy.
Since I had already ACK'd patch 1 which is now patch 1 & 2, consider the ACK still valid. Also the old patch 2 ACK is still valid even with the self find of the missing qemu check.
I posted nits/questions separately about patches 4 & 5. Since I believe you still need me to push, feel free to post either just a set of diffs for me to squash into or a v3.
We will gladly accept your kind offer. Diffs are on the way.
Is there a desire to get this into a RHEL release eventually or into the "next" RHEL release? I'd rather let this "soak" a while and then work on getting a complete libvirt-cim into a future 6.n release over what I have to do now which is patch 0.6.1 with 0.6.3 based patches...
No reason to hurry, I'd also rather see this done well than quick. And - not meant to scare you :) - we are not done yet. There's a number of patches that we're still working on and will send out for review soon. Not all are strictly s390 related, this first series isn't either, but are needed to sensibly manage guests on s390. In case you (and others) are interested, the next patches will deal with: - full-functional consoles (mandatory for s390) - device addresses - per device boot order - and some other minor libvirt feature exploitation support Stay tuned... -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 08/29/2013 11:18 AM, Viktor Mihajlovski wrote:
The current libvirt-cim implementation makes some assumptions that are only true for x86 architectures. As we want to enable libvirt-cim for s390 we need to makes sure that valid libvirt guest definitions are being built for that architecture while not breaking the existing implementation.
Patch 1 fixes potential memory access problems.
Patches 2 and 3 introduce two new properties arch and machine, effectively a pass-through of the underlying libvirt properties, for the necessary distinction between x86 and other guests, and suppress the default framebuffer for s390.
Patches 4 and 5 make sure that a minimal SVPC guest definition (VSSD and RASD) will result in a correct libvirt guest definition for the current hypervisor.
Boris Fiuczynski (2): libxkutil: Provide easy access to the libvirt capabilities VSSM: Set default values based on libvirt capabilities on DefineSystem calls
Viktor Mihajlovski (3): libxkutil: Improve domain.os_info cleanup VSSD: Add properties for arch and machine S390: Avoid the generation of default input and graphics
libxkutil/Makefile.am | 2 + libxkutil/capability_parsing.c | 556 +++++++++++++++++++++++++++++ libxkutil/capability_parsing.h | 97 +++++ libxkutil/device_parsing.c | 114 +++--- libxkutil/device_parsing.h | 4 +- libxkutil/xml_parse_test.c | 201 ++++++++++- libxkutil/xmlgen.c | 6 + schema/VSSD.mof | 6 + src/Virt_VSSD.c | 9 + src/Virt_VirtualSystemManagementService.c | 165 +++++---- 10 files changed, 1033 insertions(+), 127 deletions(-) create mode 100644 libxkutil/capability_parsing.c create mode 100644 libxkutil/capability_parsing.h
V2 Changes # Split original patch 1 into 2 patches (fix, new feature), otherwise unchanged # Patch 3 also considers QEMU domains now, was KVM only # Patches 4 and 5 address memory leaks and overwrites # cimtest run completes without regressions on x86
This series (with the squashed in changes) is now pushed upstream. Thanks, John

On 08/30/2013 02:38 PM, John Ferlan wrote:
This series (with the squashed in changes) is now pushed upstream.
Thanks! -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
John Ferlan
-
Viktor Mihajlovski