On 05/24/2013 03:34 AM, cngesaint@outlook.com wrote:

From: Xu Wang <cngesaint@outlook.com>

Under nested KVM environment libvirt-cim could recognize kvm support correctly now.

Signed-off-by: Xu Wang <cngesaint@outlook.com> --- libxkutil/device_parsing.c | 19 +++++++++++++++++++ libxkutil/device_parsing.h | 2 ++ src/Virt_VirtualSystemManagementService.c | 22 +++++++++++++++++++--- 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 436415a..58d9094 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -396,6 +396,25 @@ err: return 0; }

+int parse_domain_type(xmlNodePtr node, char **value) +{ + xmlNodePtr child = NULL; + + child = node->children; + while (child != NULL) { + if (XSTREQ(child->name, "domain")) { + *value = get_attr_value(child, "type");

This returns either a NULL or strdup()'d value. It seems by the makeup of this loop that we will continue down a "child" forward link list potentially overwriting a previous value and leaking memory. Is there something that should cause us to exit the loop?

+ } + if (parse_domain_type(child, value) != 1) + goto err;

So a child could have a child.

+ child = child->next; + } + + return 1;

We potentially could return a 1, but a NULL 'val' - is that expected.

+err: + return 0;

This function does not return CMPI_RC_OK or whatever else so the caller shouldn't be comparing against that...

+} + 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 6f6b0b4..733324f 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -221,6 +221,8 @@ 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);

+int parse_domain_type(xmlNodePtr node, char **value); + #define XSTREQ(x, y) (STREQ((char *)x, y)) #define STRPROP(d, p, n) (d->p = get_node_content(n))

diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 7b7261a..5210fdf 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -393,8 +393,13 @@ static bool system_has_kvm(const char *pfx) CMPIStatus s; virConnectPtr conn; char *caps = NULL; - bool kvm = false; bool disable_kvm = get_disable_kvm(); + char *val = NULL; + int ret; + xmlDocPtr doc; + xmlNodePtr node;

I think both of these need to be initialized to NULL because of the free() below.

+ int len; + bool kvm = false;

/* sometimes disable KVM to avoid problem in nested KVM */ if (disable_kvm) { @@ -408,10 +413,21 @@ static bool system_has_kvm(const char *pfx) }

caps = virConnectGetCapabilities(conn); - if (caps != NULL) - kvm = (strstr(caps, "kvm") != NULL); + if (caps != NULL) { + len = strlen(caps) + 1; + doc = xmlParseMemory(caps, len);

if (doc == NULL) { /* Error message ? Do we care */ goto cleanup; }

+ node = xmlDocGetRootElement(doc); if (node == NULL) { /* Error message ? Do we care */ goto cleanup; } + ret = parse_domain_type(node, &val); + CU_DEBUG("domain type is %s.", val); ^^^^^ Move inside the if statement below, val could be NULL

+ if (ret == CMPI_RC_OK) {

== 1) {

+ if(STREQC(val, "kvm")) ^^ Add a space between "if" and "("

+ kvm = true; + } + }

} else { /* Do we care what failure means here?? */ } Alternatively, if (parse_domain_type(node, &val) && STREQC(val, "kvm")) { kvm = true; } else { if (val) CU_DEBUG("domain type is %s.", val); } cleanup:

free(caps); + free(doc); + free(val);

virConnectClose(conn);