5:30 p.m.
From: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
Since in nested KVM, libvirt-cim doesn't handle it well now, add
this option to make it run well with qemu wchi help development
and test.
Signed-off-by: Xu Wang <cngesaint(a)outlook.com>
---
libvirt-cim.conf | 8 ++++++++
libxkutil/misc_util.c | 8 ++++++++
libxkutil/misc_util.h | 1 +
src/Virt_VirtualSystemManagementService.c | 7 +++++++
4 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/libvirt-cim.conf b/libvirt-cim.conf
index 37d7b0f..3244ee3 100644
--- a/libvirt-cim.conf
+++ b/libvirt-cim.conf
@@ -30,3 +30,11 @@
# 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 make it run well with qemu which help development and test.
+# 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..4e93ef0 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();
+
+ /* hack for 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
5:30 p.m.
New subject: [PATCH 2/3] VSMS: tip error for invalid disk resource
From: Xu Wang <cngesaint(a)outlook.com>
Original code will report xml text missing when a disk is not accessable,
make user confuse. This patch will report the real error to tip user check
its system health state on the server.
Signed-off-by: Xu Wang <cngesaint(a)outlook.com>
---
src/Virt_VirtualSystemManagementService.c | 65 +++++++++++++++++++++--------
1 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/src/Virt_VirtualSystemManagementService.c
b/src/Virt_VirtualSystemManagementService.c
index 4e93ef0..d252e12 100644
--- a/src/Virt_VirtualSystemManagementService.c
+++ b/src/Virt_VirtualSystemManagementService.c
@@ -964,11 +964,13 @@ static const char *net_rasd_to_vdev(CMPIInstance *inst,
}
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,17 @@ 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 (dev->dev.disk.disk_type == DISK_UNKNOWN) {
+ /* 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!");
+ }
+ 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 +1465,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 +1508,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 +1531,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 +1575,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 +1629,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 +1646,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 +1665,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 +1696,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 +1708,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 +2105,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 +2136,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 +2183,7 @@ static CMPIInstance *create_system(const CMPIContext *context,
out:
+ free(error_msg);
cleanup_dominfo(&domain);
free(xml);
inst_list_free(&list);
@@ -2638,6 +2663,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,7 +2703,7 @@ 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,
@@ -2702,6 +2728,8 @@ static CMPIStatus resource_add(struct domain *dominfo,
(*count)++;
out:
+ free(error_msg);
+
return s;
}
@@ -2718,6 +2746,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 +2778,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 +2822,8 @@ static CMPIStatus resource_mod(struct domain *dominfo,
}
out:
+ free(error_msg);
+
return s;
}
--
1.7.1
1:05 a.m.
New subject: [PATCH 2/3] VSMS: tip error for invalid disk resource
On 04/23/2013 05:30 AM, cngesaint(a)outlook.com wrote:
From: Xu Wang <cngesaint(a)outlook.com>
Original code will report xml text missing when a disk is not accessable,
make user confuse. This patch will report the real error to tip user check
s/accessable/accessible/
its system health state on the server.
Can you provide an example test or command - so that it's "testable"?
Whether that's by adding a new cimtest or some other means. There seems
to be two errors serviced by DISK_UNKNOWN - the first one is a failure
on a 'stat64()' and the second is the st_mode not being a BLK device, a
REG (file), or a DIR (file system). How are they differentiated?
Seems to me earlier checks should determine that the path doesn't exist
while this check should be limited to invalid format. My other
experience with CIM enum's is that there's supposed to be an "UNKNOWN"
and "OTHER" values, where UNKNOWN was always 0 and OTHER was always 1.
I may be the "OTHER" name space incorrect it's been a while...
Signed-off-by: Xu Wang <cngesaint(a)outlook.com>
---
src/Virt_VirtualSystemManagementService.c | 65 +++++++++++++++++++++--------
1 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/src/Virt_VirtualSystemManagementService.c
b/src/Virt_VirtualSystemManagementService.c
index 4e93ef0..d252e12 100644
--- a/src/Virt_VirtualSystemManagementService.c
+++ b/src/Virt_VirtualSystemManagementService.c
@@ -964,11 +964,13 @@ static const char *net_rasd_to_vdev(CMPIInstance *inst,
}
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,17 @@ 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 (dev->dev.disk.disk_type == DISK_UNKNOWN) {
+ /* 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);
There's no error checking on whether the strdup()'s succeeded and thus
this could cause problems with %s and NULL strings. For that matter
there's very little error checking w/r/t strdup() failures so you're
following existing potential issues...
+ if (rc == -1) {
+ CU_DEBUG("error during recording exception!");
Since asprintf() says the parameter 1 is "undefined" if rc == -1, so be
safe and set p_error to NULL again...
+ }
+ return "Can't get a valid disk type, ";
Looks like a cut-n-paste - just snip the ", ". Other error returns
don't use the ", " list marker...
+ }
+
if (cu_get_u16_prop(inst, "EmulatedType", &type) != CMPI_RC_OK)
type = VIRT_DISK_TYPE_DISK;
@@ -1452,10 +1465,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 +1508,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 +1531,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 +1575,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 +1629,15 @@ static const char *classify_resources(CMPIArray *resources,
msg = rasd_to_vdev(inst,
domain,
&domain->dev_vcpu[0],
- veillard(a)redhat.com ns);
+ ns,
+ p_error);
} else if (type == CIM_RES_TYPE_MEM) {
domain->dev_mem_ct = 1;
msg = rasd_to_vdevvirQEMUDriverCreateCapabilities(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 +1646,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 +1665,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 +1696,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 +1708,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 +2105,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 +2136,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);
Since error_msg could be NULL - it should be handled...
cu_statusf(_BROKER, s,
CMPI_RC_ERR_FAILED,
- "ResourceSettings Error: %s", msg);
+ "ResourceSettings Error: %s, %s", msg, error_msg);
Same here...
goto out;
}
@@ -2159,6 +2183,7 @@ static CMPIInstance *create_system(const CMPIContext *context,
out:
+ free(error_msg);
cleanup_dominfo(&domain);
free(xml);
inst_list_free(&list);
@@ -2638,6 +2663,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,7 +2703,7 @@ 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,
Why not add the "error_msg" output here too like create_system?
@@ -2702,6 +2728,8 @@ static CMPIStatus resource_add(struct domain
*dominfo,
(*count)++;
out:
+ free(error_msg);
+virQEMUDriverCreateCapabilities
return s;
}
@@ -2718,6 +2746,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 +2778,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,
Same comment
John
@@ -2793,6 +2822,8 @@ static CMPIStatus resource_mod(struct domain
*dominfo,
}
out:
+ free(error_msg);
+
return s;
}
5:38 p.m.
New subject: [PATCH 2/3] VSMS: tip error for invalid disk resource
Date: Thu, 25 Apr 2013 13:05:51 -0400
From: jferlan(a)redhat.com
To: libvirt-cim(a)redhat.com
Subject: Re: [Libvirt-cim] [PATCH 2/3] VSMS: tip error for invalid disk resource
On 04/23/2013 05:30 AM, cngesaint(a)outlook.com wrote:
> From: Xu Wang <cngesaint(a)outlook.com>
>
> Original code will report xml text missing when a disk is not accessable,
> make user confuse. This patch will report the real error to tip user check
s/accessable/accessible/
> its system health state on the server.
Can you provide an example test or command - so that it's "testable"?
Whether that's by adding a new cimtest or some other means. There seems
to be two errors serviced by DISK_UNKNOWN - the first one is a failure
on a 'stat64()' and the second is the st_mode not being a BLK device, a
REG (file), or a DIR (file system). How are they differentiated?
Seems to me earlier checks should determine that the path doesn't exist
while this check should be limited to invalid format. My other
experience with CIM enum's is that there's supposed to be an
"UNKNOWN"
and "OTHER" values, where UNKNOWN was always 0 and OTHER was always 1.
I may be the "OTHER" name space incorrect it's been a while...
Yes, I just found it is not suitable for just checking resource accessible by disk
type.
e.g.,under cdrom (blank or none disk) this patch will trigger another question. So
I will rewrite it.
>
> Signed-off-by: Xu Wang <cngesaint(a)outlook.com>
> ---
> src/Virt_VirtualSystemManagementService.c | 65 +++++++++++++++++++++--------
> 1 files changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/src/Virt_VirtualSystemManagementService.c
b/src/Virt_VirtualSystemManagementService.c
> index 4e93ef0..d252e12 100644
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -964,11 +964,13 @@ static const char *net_rasd_to_vdev(CMPIInstance *inst,
> }
>
> 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,17 @@ 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 (dev->dev.disk.disk_type == DISK_UNKNOWN) {
> + /* 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);
There's no error checking on whether the strdup()'s succeeded and thus
this could cause problems with %s and NULL strings. For that matter
there's very little error checking w/r/t strdup() failures so you're
following existing potential issues...
> + if (rc == -1) {
> + CU_DEBUG("error during recording exception!");
Since asprintf() says the parameter 1 is "undefined" if rc == -1, so be
safe and set p_error to NULL again...
Yes, I think so.
> + }
> + return "Can't get a valid disk type, ";
Looks like a cut-n-paste - just snip the ", ". Other error returns
don't use the ", " list marker...
It's caused by my input method, sorry...
> + }
> +
> if (cu_get_u16_prop(inst, "EmulatedType", &type) !=
CMPI_RC_OK)
> type = VIRT_DISK_TYPE_DISK;
>
> @@ -1452,10 +1465,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 +1508,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 +1531,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 +1575,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 +1629,15 @@ static const char *classify_resources(CMPIArray
*resources,
> msg = rasd_to_vdev(inst,
> domain,
> &domain->dev_vcpu[0],
> - veillard(a)redhat.com ns);
> + ns,
> + p_error);
> } else if (type == CIM_RES_TYPE_MEM) {
> domain->dev_mem_ct = 1;
> msg = rasd_to_vdevvirQEMUDriverCreateCapabilities(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 +1646,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 +1665,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 +1696,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 +1708,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 +2105,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 +2136,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);
Since error_msg could be NULL - it should be handled...
It would be OK. If error_msg is null, a blank will output and no error.
> cu_statusf(_BROKER, s,
> CMPI_RC_ERR_FAILED,
> - "ResourceSettings Error: %s", msg);
> + "ResourceSettings Error: %s, %s", msg,
error_msg);
Same here...
> goto out;
> }
>
> @@ -2159,6 +2183,7 @@ static CMPIInstance *create_system(const CMPIContext
*context,
>
>
> out:
> + free(error_msg);
> cleanup_dominfo(&domain);
> free(xml);
> inst_list_free(&list);
> @@ -2638,6 +2663,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,7 +2703,7 @@ 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,
Why not add the "error_msg" output here too like create_system?
Yes, error_msg also could be returned to server from here:-)
> @@ -2702,6 +2728,8 @@ static CMPIStatus resource_add(struct domain *dominfo,
> (*count)++;
>
> out:
> + free(error_msg);
> +virQEMUDriverCreateCapabilities
> return s;
> }
>
> @@ -2718,6 +2746,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 +2778,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,
Same comment
John
> @@ -2793,6 +2822,8 @@ static CMPIStatus resource_mod(struct domain *dominfo,
> }
>
> out:
> + free(error_msg);
> +
> return s;
> }
>
>
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
5:30 p.m.
New subject: [PATCH 3/3] make lldptool command and support output configurable
From: Xu Wang <cngesaint(a)outlook.com>
Signed-off-by: Xu Wang <cngesaint(a)outlook.com>
---
libvirt-cim.conf | 14 ++++++++++
libxkutil/misc_util.c | 18 +++++++++++++
libxkutil/misc_util.h | 2 +
src/Virt_SwitchService.c | 63 ++++++++++++++++++++++++++++++++++++++--------
4 files changed, 86 insertions(+), 11 deletions(-)
diff --git a/libvirt-cim.conf b/libvirt-cim.conf
index 3244ee3..396dac9 100644
--- a/libvirt-cim.conf
+++ b/libvirt-cim.conf
@@ -38,3 +38,17 @@
# 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 update the new output into the
+# output set. add comma between items.
+#
+# 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..f7bafbf 100644
--- a/src/Virt_SwitchService.c
+++ b/src/Virt_SwitchService.c
@@ -46,12 +46,26 @@ 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) {
+ 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_DEBUG("strdup vsi_support_key_string failed!");
+ goto out;
+ }
// Run lldptool command to find vsi support.
stream = popen(command, "r");
@@ -63,6 +77,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 +114,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 +249,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 +293,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
12:42 a.m.
On 04/23/2013 05:30 AM, cngesaint(a)outlook.com wrote:
From: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
Since in nested KVM, libvirt-cim doesn't handle it well now, add
this option to make it run well with qemu wchi help development
and test.
I think your commit message more simply stated is:
Allow libvirt-cim to be supported within a nested KVM environment in
order to more easily develop and test various configurations
What happens if someone sets this in a non nested environment? I would
think you'd want to test if the 'nested' property is set...
Signed-off-by: Xu Wang <cngesaint(a)outlook.com>
---
libvirt-cim.conf | 8 ++++++++
libxkutil/misc_util.c | 8 ++++++++
libxkutil/misc_util.h | 1 +
src/Virt_VirtualSystemManagementService.c | 7 +++++++
4 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/libvirt-cim.conf b/libvirt-cim.conf
index 37d7b0f..3244ee3 100644
--- a/libvirt-cim.conf
+++ b/libvirt-cim.conf
@@ -30,3 +30,11 @@
# 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 make it run well with qemu which help development and test.
+# Possible values: {true,false}
+# Default value: false
+#
+# force_use_qemu = false;
I suggest using the comments from above here too...
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..4e93ef0 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();
+
+ /* hack for nested KVM */
+ if (force_use_qemu) {
+ CU_DEBUG("Enter force use qemu mode!");
+ return false;
+ }
The above check is being done on the "local" system right? While the
following check is being done on the "BROKER" system, right? Which may
not be the same as the "local" system? So where should the check of
whether the system in which is executing libvirt-cim code be really
made? IOW, are you sure this is the right place to check? What's the
differentiation in libvirt-cim between QEMU & KVM being made for? Being
"new" I thought they were the same.
I'm not even convinced the routine is doing the right thing, but perhaps
I just don't have enough history. I see a virsh capabilities on my
system returns some kvm defs and some qemu defs, so a blind strstr is
returning true...
The prime differentiator is that the calling routine will set
domain->type to QEMU now rather than KVM and I'm not sure I understand
how by using that setting we'll be able support nested KVM.
John
conn = connect_by_classname(_BROKER, pfx, &s);
if ((conn == NULL) || (s.rc != CMPI_RC_OK)) {
11:24 a.m.
于 2013-4-26 0:42, John Ferlan 写道:
On 04/23/2013 05:30 AM, cngesaint(a)outlook.com wrote:
> From: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
>
> Since in nested KVM, libvirt-cim doesn't handle it well now, add
> this option to make it run well with qemu wchi help development
> and test.
I think your commit message more simply stated is:
Allow libvirt-cim to be supported within a nested KVM environment in
order to more easily develop and test various configurations
What happens if someone sets this in a non nested environment? I would
think you'd want to test if the 'nested' property is set...
Hi, John
The commit message is not clear, actually this patch provide a way to
manually disable KVM and fall back to qemu(without usage of hardware
acceleration module, kvm.ko), Since using KVM have some problem in
libvirt-cim in nested KVM env(which is a test env, most issues happens
in the xml gen in libvirt-cim and parsing in libvirt).
In non nested environment, it is same, kvm acceleration is disabled.
>
> Signed-off-by: Xu Wang <cngesaint(a)outlook.com>
> ---
> libvirt-cim.conf | 8 ++++++++
> libxkutil/misc_util.c | 8 ++++++++
> libxkutil/misc_util.h | 1 +
> src/Virt_VirtualSystemManagementService.c | 7 +++++++
> 4 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/libvirt-cim.conf b/libvirt-cim.conf
> index 37d7b0f..3244ee3 100644
> --- a/libvirt-cim.conf
> +++ b/libvirt-cim.conf
> @@ -30,3 +30,11 @@
> # 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 make it run well with qemu which help development and test.
> +# Possible values: {true,false}
> +# Default value: false
> +#
> +# force_use_qemu = false;
I suggest using the comments from above here too...
> 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..4e93ef0 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();
> +
> + /* hack for nested KVM */
> + if (force_use_qemu) {
> + CU_DEBUG("Enter force use qemu mode!");
> + return false;
> + }
The above check is being done on the "local" system right? While the
following check is being done on the "BROKER" system, right? Which may
They both happen on local system.
not be the same as the "local" system? So where should the
check of
whether the system in which is executing libvirt-cim code be really
made? IOW, are you sure this is the right place to check? What's the
differentiation in libvirt-cim between QEMU & KVM being made for? Being
"new" I thought they were the same.
I'm not even convinced the routine is doing the right thing, but perhaps
I just don't have enough history. I see a virsh capabilities on my
system returns some kvm defs and some qemu defs, so a blind strstr is
returning true...
The prime differentiator is that the calling routine will set
domain->type to QEMU now rather than KVM and I'm not sure I understand
how by using that setting we'll be able support nested KVM.
Support nested
KVM will need more efforts, I think you have found the
right place, there are some issues in following path, what I saw
is libvirt rejected some xml generated. To avoid the troubles, so
disable it first, to test libvirt-cim for qemu, for that most part
are the same with KVM case,
John
>
> conn = connect_by_classname(_BROKER, pfx, &s);
> if ((conn == NULL) || (s.rc != CMPI_RC_OK)) {
>
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
--
Best Regards
Wenchao Xia
4192
days inactive
4196
days old
6 comments
4 participants
participants (4)
-
cngesaint＠outlook.com -
John Ferlan -
WangXu
-
Wenchao Xia