From: Wenchao Xia <xiawenc@linux.vnet.ibm.com> Since in nested KVM, libvirt-cim doesn't handler it well now, so add this option to manually disable KVM and fall back to qemu. If there is something fail caused by nested KVM happend, please consider to set this option to be true for development and testing. Signed-off-by: Xu Wang <cngesaint@outlook.com> --- libvirt-cim.conf | 10 ++++++++++ libxkutil/misc_util.c | 8 ++++++++ libxkutil/misc_util.h | 1 + src/Virt_VirtualSystemManagementService.c | 7 +++++++ 4 files changed, 26 insertions(+), 0 deletions(-) diff --git a/libvirt-cim.conf b/libvirt-cim.conf index 37d7b0f..f378e4c 100644 --- a/libvirt-cim.conf +++ b/libvirt-cim.conf @@ -30,3 +30,13 @@ # Default value: NULL, that is not set. # # migrate_ssh_temp_key = "/root/vm_migrate_tmp_id_rsa"; + +# force_use_qemu (bool) +# Since in nested KVM, libvirt-cim doesn't handler it well now, so add this +# option to manually disable KVM and fall back to qemu. If there is something +# fail caused by nested KVM happend, please consider to set this option to be +# true for development and testing. +# Possible values: {true,false} +# Default value: false +# +# force_use_qemu = false; diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c index 00eb4b1..4c0b0a1 100644 --- a/libxkutil/misc_util.c +++ b/libxkutil/misc_util.c @@ -227,6 +227,14 @@ static int is_read_only(void) return prop.value_bool; } +bool get_force_use_qemu(void) +{ + static LibvirtcimConfigProperty prop = { + "force_use_qemu", CONFIG_BOOL, {0}, 0}; + libvirt_cim_config_get(&prop); + return prop.value_bool; +} + const char *get_mig_ssh_tmp_key(void) { static LibvirtcimConfigProperty prop = { diff --git a/libxkutil/misc_util.h b/libxkutil/misc_util.h index 0f52290..9e6b419 100644 --- a/libxkutil/misc_util.h +++ b/libxkutil/misc_util.h @@ -154,6 +154,7 @@ int virt_set_status(const CMPIBroker *broker, /* get libvirt-cim config */ const char *get_mig_ssh_tmp_key(void); +bool get_force_use_qemu(void); /* * Local Variables: diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index cbb646d..81ec064 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -394,6 +394,13 @@ static bool system_has_kvm(const char *pfx) virConnectPtr conn; char *caps = NULL; bool kvm = false; + bool force_use_qemu = get_force_use_qemu(); + + /* sometimes disable KVM to avoid problem in nested KVM */ + if (force_use_qemu) { + CU_DEBUG("Enter force use qemu mode!"); + return false; + } conn = connect_by_classname(_BROKER, pfx, &s); if ((conn == NULL) || (s.rc != CMPI_RC_OK)) { -- 1.7.1

if lldptool command or vsi support output updates, update items in the libvirt-cim.conf file. vsi support output could support maximum 8 items. Signed-off-by: Xu Wang <cngesaint@outlook.com> --- libvirt-cim.conf | 16 +++++++++++ libxkutil/misc_util.c | 18 ++++++++++++ libxkutil/misc_util.h | 2 + src/Virt_SwitchService.c | 66 ++++++++++++++++++++++++++++++++++++++------- 4 files changed, 91 insertions(+), 11 deletions(-) diff --git a/libvirt-cim.conf b/libvirt-cim.conf index f378e4c..0e8f399 100644 --- a/libvirt-cim.conf +++ b/libvirt-cim.conf @@ -40,3 +40,19 @@ # Default value: false # # force_use_qemu = false; + +# lldptool_query_options (string) +# Defines the command used in SwitchService to query VEPA support, will be +# used as "lldptool -i [INTERFACE] [OPTIONS]" +# +# lldptool_query_options = "-t -g ncb -V evbcfg"; + +# vsi_support_key_string (string) +# Defines the string used in SwitchService to search in lldptool's output +# When lldptool updates its output, please set this value and update the +# output set, maximum 8 items. use comma to devide items. If there is +# no need to change the output set, please do not leave it void. Because +# this value has higher level than default. +# +# vsi_support_key_string = "{ supported forwarding mode: (0x40) reflective relay," +# " supported capabilities: (0x7) RTE ECP VDP}"; diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c index 4c0b0a1..6ce8dca 100644 --- a/libxkutil/misc_util.c +++ b/libxkutil/misc_util.c @@ -244,6 +244,24 @@ const char *get_mig_ssh_tmp_key(void) return prop.value_string; } +const char *get_lldptool_query_options(void) +{ + static LibvirtcimConfigProperty prop = { + "lldptool_query_options", CONFIG_STRING, {0}, 0}; + + libvirt_cim_config_get(&prop); + return prop.value_string; +} + +const char *get_vsi_support_key_string(void) +{ + static LibvirtcimConfigProperty prop = { + "vsi_support_key_string", CONFIG_STRING, {0}, 0};; + + libvirt_cim_config_get(&prop); + return prop.value_string; +} + virConnectPtr connect_by_classname(const CMPIBroker *broker, const char *classname, CMPIStatus *s) diff --git a/libxkutil/misc_util.h b/libxkutil/misc_util.h index 9e6b419..4eb588d 100644 --- a/libxkutil/misc_util.h +++ b/libxkutil/misc_util.h @@ -155,6 +155,8 @@ int virt_set_status(const CMPIBroker *broker, /* get libvirt-cim config */ const char *get_mig_ssh_tmp_key(void); bool get_force_use_qemu(void); +const char *get_lldptool_query_options(void); +const char *get_vsi_support_key_string(void); /* * Local Variables: diff --git a/src/Virt_SwitchService.c b/src/Virt_SwitchService.c index 8991426..983ebc0 100644 --- a/src/Virt_SwitchService.c +++ b/src/Virt_SwitchService.c @@ -46,12 +46,29 @@ static CMPIStatus check_vsi_support(char *command) CMPIStatus s = {CMPI_RC_OK, NULL}; char buff[MAX_LEN]; FILE *stream = NULL; - const char *searchStr[] = {" supported forwarding mode: " - "(0x40) reflective relay", - " supported capabilities: " - "(0x07) RTE ECP VDP", - NULL}; - int matched = 0; + char *searchStr[8]; /* maximum items of vsi support output */ + int count = 0; + const char *user_settings = get_vsi_support_key_string(); + char *vsi_support_key_string = NULL; + char *delim = "{},"; + int matched = 0; + char *temp = NULL; + + if (!user_settings) { + /* default supported output set, 8 maximum */ + user_settings = "{ supported forwarding mode: " + "(0x40) reflective relay," + " supported capabilities: " + "(0x7) RTE ECP VDP}"; + } + + vsi_support_key_string = strdup(user_settings); + if (vsi_support_key_string == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Strdup vsi_support_key_string failed!"); + goto out; + } // Run lldptool command to find vsi support. stream = popen(command, "r"); @@ -63,6 +80,25 @@ static CMPIStatus check_vsi_support(char *command) goto out; } + /* Slice vsi_support_key_string into items */ + searchStr[count] = strtok_r(vsi_support_key_string, delim, &temp); + if (searchStr[count] == NULL) { + CU_DEBUG("searchStr fetch failed when calling strtok_r!"); + } else { + CU_DEBUG("searchStr[%d]: %s", count, searchStr[count]); + count++; + } + + while ((searchStr[count] = strtok_r(NULL, delim, &temp))) { + if (count >= 7) { + CU_DEBUG("WARN: searchStr is full, left aborted!"); + break; + } else { + CU_DEBUG("searchStr[%d]: %s", count, searchStr[count]); + count++; + } + } + // Read the output of the command. while (fgets(buff, MAX_LEN, stream) != NULL) { int i = 0; @@ -81,16 +117,18 @@ static CMPIStatus check_vsi_support(char *command) } /* All the search strings were found in the output of this command. */ - if (matched == 2) { + if (matched == count) { cu_statusf(_BROKER, &s, CMPI_RC_OK, "VSI supported"); - goto out;; + goto out; } } + cu_statusf(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, "No VSI Support found"); - out: + out: + free(vsi_support_key_string); if (stream != NULL) pclose(stream); return s; @@ -214,6 +252,7 @@ static CMPIStatus get_switchservice(const CMPIObjectPath *reference, int i; char **if_list; char cmd[MAX_LEN]; + const char *lldptool_query_options = NULL; *_inst = NULL; conn = connect_by_classname(broker, CLASSNAME(reference), &s); @@ -257,10 +296,15 @@ static CMPIStatus get_switchservice(const CMPIObjectPath *reference, CU_DEBUG("Found %d interfaces", count); + lldptool_query_options = get_lldptool_query_options(); + if (!lldptool_query_options) { + lldptool_query_options = "-t -g ncb -V evbcfg"; + } for (i=0; i<count; i++) { - sprintf(cmd, "lldptool -i %s -t -V evbcfg", if_list[i]); - CU_DEBUG("running command %s ...", cmd); + sprintf(cmd, "lldptool -i %s %s", + if_list[i], lldptool_query_options); + CU_DEBUG("running command [%s]", cmd); s = check_vsi_support(cmd); if (s.rc == CMPI_RC_OK) { vsi = true; -- 1.7.1

On 05/02/2013 03:10 AM, Xu Wang wrote:

Original code will report xml text missing when a disk is not accessable,

Not resolved from last review: s/accessable/accessible

make user confuse. This patch will report the real error to tip user check its system health state on the server.

I also still see no example of what's being fixed.

Signed-off-by: Xu Wang <cngesaint@outlook.com> --- src/Virt_VirtualSystemManagementService.c | 70 +++++++++++++++++++++-------- 1 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 81ec064..1652cf2 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -964,11 +964,13 @@ static const char *net_rasd_to_vdev(CMPIInstance *inst, }veillard@redhat.com

static const char *disk_rasd_to_vdev(CMPIInstance *inst, - struct virt_device *dev) + struct virt_device *dev, + char **p_error) { const char *val = NULL; uint16_t type; bool read = false; + int rc;

CU_DEBUG("Enter disk_rasd_to_vdev"); if (cu_get_str_prop(inst, "VirtualDevice", &val) != CMPI_RC_OK) @@ -984,6 +986,18 @@ static const char *disk_rasd_to_vdev(CMPIInstance *inst, dev->dev.disk.source = strdup(val); dev->dev.disk.disk_type = disk_type_from_file(val);

+ if ((!XSTREQ(dev->dev.disk.source, "/dev/null")) && (dev->dev.disk.disk_type == DISK_UNKNOWN)) {

So again, there's no checking for NULL on strdup() for dev.disk.source which will cause a seg fault here. NACK and did not review remainder John

+ /* on success or fail caller should try free it */ + rc = asprintf(p_error, "Device %s, Address %s, " + "make sure Address can be accessed on host system.", + dev->dev.disk.virtual_dev, dev->dev.disk.source); + if (rc == -1) { + CU_DEBUG("error during recording exception!"); + p_error = NULL; + } + return "Can't get a valid disk type, "; + } + if (cu_get_u16_prop(inst, "EmulatedType", &type) != CMPI_RC_OK) type = VIRT_DISK_TYPE_DISK;

@@ -1452,10 +1466,11 @@ static const char *input_rasd_to_vdev(CMPIInstance *inst, static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst, struct virt_device *dev, uint16_t type, - const char *ns) + const char *ns, + char **p_error) { if (type == CIM_RES_TYPE_DISK) { - return disk_rasd_to_vdev(inst, dev); + return disk_rasd_to_vdev(inst, dev, p_error); } else if (type == CIM_RES_TYPE_NET) { return net_rasd_to_vdev(inst, dev, ns); } else if (type == CIM_RES_TYPE_MEM) { @@ -1494,7 +1509,8 @@ static const char *_container_rasd_to_vdev(CMPIInstance *inst, static const char *rasd_to_vdev(CMPIInstance *inst, struct domain *domain, struct virt_device *dev, - const char *ns) + const char *ns, + char **p_error) { uint16_t type; CMPIObjectPath *op; @@ -1516,7 +1532,7 @@ static const char *rasd_to_vdev(CMPIInstance *inst, if (domain->type == DOMAIN_LXC) msg = _container_rasd_to_vdev(inst, dev, type, ns); else - msg = _sysvirt_rasd_to_vdev(inst, dev, type, ns); + msg = _sysvirt_rasd_to_vdev(inst, dev, type, ns, p_error); out: if (msg && op) CU_DEBUG("rasd_to_vdev(%s): %s", CLASSNAME(op), msg); @@ -1560,7 +1576,8 @@ static char *add_device_nodup(struct virt_device *dev,

static const char *classify_resources(CMPIArray *resources, const char *ns, - struct domain *domain) + struct domain *domain, + char **p_error) { int i; uint16_t type; @@ -1613,13 +1630,15 @@ static const char *classify_resources(CMPIArray *resources, msg = rasd_to_vdev(inst, domain, &domain->dev_vcpu[0], - ns); + ns, + p_error); } else if (type == CIM_RES_TYPE_MEM) { domain->dev_mem_ct = 1; msg = rasd_to_vdev(inst, domain, &domain->dev_mem[0], - ns); + ns, + p_error); } else if (type == CIM_RES_TYPE_DISK) { struct virt_device dev; int dcount = count + domain->dev_disk_ct; @@ -1628,7 +1647,8 @@ static const char *classify_resources(CMPIArray *resources, msg = rasd_to_vdev(inst, domain, &dev, - ns); + ns, + p_error); if (msg == NULL) msg = add_device_nodup(&dev, domain->dev_disk, @@ -1646,7 +1666,8 @@ static const char *classify_resources(CMPIArray *resources, msg = rasd_to_vdev(inst, domain, &dev, - ns); + ns, + p_error); if (msg == NULL) msg = add_device_nodup(&dev, domain->dev_net, @@ -1676,7 +1697,8 @@ static const char *classify_resources(CMPIArray *resources, msg = rasd_to_vdev(inst, domain, &dev, - ns); + ns, + p_error); if (msg == NULL) msg = add_device_nodup(&dev, domain->dev_graphics, @@ -1687,7 +1709,8 @@ static const char *classify_resources(CMPIArray *resources, msg = rasd_to_vdev(inst, domain, &domain->dev_input[0], - ns); + ns, + p_error); } if (msg != NULL) return msg; @@ -2083,6 +2106,7 @@ static CMPIInstance *create_system(const CMPIContext *context, struct inst_list list; const char *props[] = {NULL}; struct domain *domain = NULL; + char *error_msg = NULL;

inst_list_init(&list);

@@ -2113,12 +2137,13 @@ static CMPIInstance *create_system(const CMPIContext *context, if (s->rc != CMPI_RC_OK) goto out;

- msg = classify_resources(resources, NAMESPACE(ref), domain); + msg = classify_resources(resources, NAMESPACE(ref), domain, &error_msg); if (msg != NULL) { - CU_DEBUG("Failed to classify resources: %s", msg); + CU_DEBUG("Failed to classify resources: %s, %s", + msg, error_msg); cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED, - "ResourceSettings Error: %s", msg); + "ResourceSettings Error: %s, %s", msg, error_msg); goto out; }

@@ -2159,6 +2184,7 @@ static CMPIInstance *create_system(const CMPIContext *context,

out: + free(error_msg); cleanup_dominfo(&domain); free(xml); inst_list_free(&list); @@ -2638,6 +2664,7 @@ static CMPIStatus resource_add(struct domain *dominfo, struct virt_device *dev; int *count = NULL; const char *msg = NULL; + char *error_msg = NULL;

op = CMGetObjectPath(rasd, &s); if ((op == NULL) || (s.rc != CMPI_RC_OK)) @@ -2677,12 +2704,12 @@ static CMPIStatus resource_add(struct domain *dominfo, dev = &list[*count];

dev->type = type; - msg = rasd_to_vdev(rasd, dominfo, dev, ns); + msg = rasd_to_vdev(rasd, dominfo, dev, ns, &error_msg); if (msg != NULL) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, - "Add resource failed: %s", - msg); + "Add resource failed: %s, %s", + msg, error_msg); goto out; }

@@ -2702,6 +2729,8 @@ static CMPIStatus resource_add(struct domain *dominfo, (*count)++;

out: + free(error_msg); + return s; }

@@ -2718,6 +2747,7 @@ static CMPIStatus resource_mod(struct domain *dominfo, int *count; int i; const char *msg = NULL; + char *error_msg = NULL;

CU_DEBUG("Enter resource_mod"); if (devid == NULL) { @@ -2749,7 +2779,7 @@ static CMPIStatus resource_mod(struct domain *dominfo, struct virt_device *dev = &list[i];

if (STREQ(dev->id, devid)) { - msg = rasd_to_vdev(rasd, dominfo, dev, ns); + msg = rasd_to_vdev(rasd, dominfo, dev, ns, &error_msg); if (msg != NULL) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, @@ -2793,6 +2823,8 @@ static CMPIStatus resource_mod(struct domain *dominfo, }

out: + free(error_msg); + return s; }

Date: Wed, 8 May 2013 14:49:08 -0400 From: jferlan@redhat.com To: libvirt-cim@redhat.com Subject: Re: [Libvirt-cim] [PATCH V2 1/3] make force use qemu configurable

On 05/02/2013 03:10 AM, Xu Wang wrote:

...

From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>

Since in nested KVM, libvirt-cim doesn't handler it well now, so add this option to manually disable KVM and fall back to qemu. If there is something fail caused by nested KVM happend, please consider to set this option to be true for development and testing.

Signed-off-by: Xu Wang <cngesaint@outlook.com> --- libvirt-cim.conf | 10 ++++++++++ libxkutil/misc_util.c | 8 ++++++++ libxkutil/misc_util.h | 1 + src/Virt_VirtualSystemManagementService.c | 7 +++++++ 4 files changed, 26 insertions(+), 0 deletions(-)

The commit message is still a bit confusing, but I suppose not enough to hold up adding the capability. Hopefully anyone forced to make changes as a result of being able to develop/test further in the nested environment will duly note where and what had to change... There maybe something unsuitable configuration in the xml files led to the error under nested KVM. Such as qemu is OK under the same condition.

...

diff --git a/libvirt-cim.conf b/libvirt-cim.conf index 37d7b0f..f378e4c 100644 --- a/libvirt-cim.conf +++ b/libvirt-cim.conf @@ -30,3 +30,13 @@ # Default value: NULL, that is not set. # # migrate_ssh_temp_key = "/root/vm_migrate_tmp_id_rsa"; + +# force_use_qemu (bool) +# Since in nested KVM, libvirt-cim doesn't handler it well now, so add this +# option to manually disable KVM and fall back to qemu. If there is something +# fail caused by nested KVM happend, please consider to set this option to be +# true for development and testing.

Consider the following explanation to replace the above 4 lines :

When executing in a nested KVM environment libvirt-cim may fail unexpectedly with a TBS error. Setting "force_use_qemu" option to true disables use of KVM and falls back to using QEMU directly.

NOTE: TBS should be replaced with the error or the clue that someone would need in order to determine they should be using/setting this to true. Remarking about development or testing is irrelevant. Thank you very much for the above words. I think they are much better than

---------------------------------------- old comment. So the new version patch will use them.

John

...

+# Possible values: {true,false} +# Default value: false +# +# force_use_qemu = false; diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c index 00eb4b1..4c0b0a1 100644 --- a/libxkutil/misc_util.c +++ b/libxkutil/misc_util.c @@ -227,6 +227,14 @@ static int is_read_only(void) return prop.value_bool; }

+bool get_force_use_qemu(void) +{ + static LibvirtcimConfigProperty prop = { + "force_use_qemu", CONFIG_BOOL, {0}, 0}; + libvirt_cim_config_get(&prop); + return prop.value_bool; +} + const char *get_mig_ssh_tmp_key(void) { static LibvirtcimConfigProperty prop = { diff --git a/libxkutil/misc_util.h b/libxkutil/misc_util.h index 0f52290..9e6b419 100644 --- a/libxkutil/misc_util.h +++ b/libxkutil/misc_util.h @@ -154,6 +154,7 @@ int virt_set_status(const CMPIBroker *broker,

/* get libvirt-cim config */ const char *get_mig_ssh_tmp_key(void); +bool get_force_use_qemu(void);

/* * Local Variables: diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index cbb646d..81ec064 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -394,6 +394,13 @@ static bool system_has_kvm(const char *pfx) virConnectPtr conn; char *caps = NULL; bool kvm = false; + bool force_use_qemu = get_force_use_qemu(); + + /* sometimes disable KVM to avoid problem in nested KVM */ + if (force_use_qemu) { + CU_DEBUG("Enter force use qemu mode!"); + return false; + }

conn = connect_by_classname(_BROKER, pfx, &s); if ((conn == NULL) || (s.rc != CMPI_RC_OK)) {

_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim