[Libvirt-cim] [PATCH V2 2/3] VSMS: tip error for invalid disk resource
John Ferlan
jferlan at redhat.com
Wed May 8 18:56:25 UTC 2013
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 at 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 at 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;
> }
>
>
More information about the Libvirt-cim
mailing list