[Libvirt-cim] [PATCH 4/4] VSSM: Set default values based on libvirt capabilities on DefineSystem calls
John Ferlan
jferlan at redhat.com
Fri Aug 23 21:48:27 UTC 2013
On 08/15/2013 10:48 AM, Viktor Mihajlovski wrote:
> From: Boris Fiuczynski <fiuczy at 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 at linux.vnet.ibm.com>
> Reviewed-by: Viktor Mihajlovski <mihajlov at linux.vnet.ibm.com>
> ---
> src/Virt_VirtualSystemManagementService.c | 130 ++++++++++++-----------------
> 1 file changed, 55 insertions(+), 75 deletions(-)
>
This patch had some coverity - resource_leak issues... See below
And fails cimtest miserably... I'll dig some more later to see if I find something,
but I have to leave for the day now...
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
> index 3fad33b..b0d81d1 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,13 +459,17 @@ 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;
> const char *val;
> + struct capabilities *capsinfo = NULL;
> +
> + get_capabilities(conn, &capsinfo);
(1) Event alloc_arg: "get_capabilities(virConnectPtr, struct capabilities **)" allocates memory that is stored into "capsinfo". [details]
Also see events: [leaked_storage]
Secondary to that if get_capabilities() fails, then there's bound to be
some nasty failures after this...
>
> if (STREQC(pfx, "KVM")) {
> - if (system_has_kvm(pfx))
> + if (use_kvm(capsinfo))
> domain->type = DOMAIN_KVM;
> else
> domain->type = DOMAIN_QEMU;
...
478 } else {
479 CU_DEBUG("Unknown fullvirt domain type: %s", pfx);
(6) Event leaked_storage: Variable "capsinfo" going out of scope leaks the storage it points to.
Also see events: [alloc_arg]
480 return 0;
481 }
...
'capsinfo' is leaked
> @@ -532,30 +484,49 @@ static int fv_vssd_to_domain(CMPIInstance *inst,
> if (ret != 1)
> return 0;
Same here
>
> - 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;
> - }
> -
> - 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)
> + if (ret != CMPI_RC_OK) {
> + if (capsinfo != NULL) { /* set default */
> + val = get_default_arch(capsinfo, "hvm");
> + 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)
> + if (ret != CMPI_RC_OK) {
> + if (capsinfo != NULL) { /* set default */
> + val = get_default_machine(capsinfo, "hvm",
> + domain->os_info.fv.arch,
> + "kvm");
> + 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;
> +
> + ret = cu_get_str_prop(inst, "Emulator", &val);
> + if (ret != CMPI_RC_OK) {
> + if (capsinfo != NULL) { /* set default */
> + val = get_default_emulator(capsinfo, "hvm",
> + domain->os_info.fv.arch,
> + "kvm");
> + 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);
> + return 0;
Same here
> + }
> +
> + if (!fv_set_emulator(domain, val))
> + return 0;
Same here
>
> return 1;
> }
> @@ -663,6 +634,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 +721,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);
>
More information about the Libvirt-cim
mailing list