[Libvirt-cim] [PATCH V5 13/15] VSSD: report success if not all VS fail in enum

[John Ferlan]

On 03/20/2013 11:39 PM, Wenchao Xia wrote:
>   Original code 'continue' in the 'for' in return_enum_vssd() when
> one VSDS fail in retrieving, but forgot to set s to normal. This patch
> fix this case.
>   Also many debug message is added to log the error if met.
> 
> Signed-off-by: Wenchao Xia <xiawenc at linux.vnet.ibm.com>
> ---
>  src/Virt_VSSD.c |   43 +++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/src/Virt_VSSD.c b/src/Virt_VSSD.c
> index 975623b..3d2de26 100644
> --- a/src/Virt_VSSD.c
> +++ b/src/Virt_VSSD.c
> @@ -181,8 +181,10 @@ static int instance_from_dom(const CMPIBroker *broker,
>          struct domain *dominfo = NULL;
>  
>          ret = get_dominfo(dom, &dominfo);
> -        if (!ret)
> +        if (!ret) {
> +                CU_DEBUG("Failed in get_dominfo().");
>                  goto out;
> +        }
>  
>          op = CMGetObjectPath(inst, NULL);
>          pfx = class_prefix_name(CLASSNAME(op));
> @@ -245,6 +247,7 @@ static int instance_from_dom(const CMPIBroker *broker,
>              (dominfo->type == DOMAIN_KVM) || (dominfo->type == DOMAIN_QEMU)) {
>                  s = _set_fv_prop(broker, dominfo, inst);
>                  if (s.rc != CMPI_RC_OK) {
> +                        CU_DEBUG("Failed to set full virtual props.");
>                          ret = 0;
>                          goto out;
>                  }
> @@ -259,6 +262,7 @@ static int instance_from_dom(const CMPIBroker *broker,
>                           dominfo->type);
>  
>          if (asprintf(&vsid, "%s:%s", pfx, dominfo->name) == -1) {
> +                CU_DEBUG("Failed in asprintf().");
>                  ret = 0;
>                  goto out;
>          }
> @@ -328,14 +332,34 @@ static CMPIStatus return_enum_vssd(const CMPIObjectPath *reference,
>          } else if (count == 0)
>                  goto out;
>  
> +        int fail_count = 0;

Put this with the other decls.

>          for (i = 0; i < count; i++) {
>                  CMPIInstance *inst = NULL;
>                  
>                  inst = _get_vssd(_BROKER, reference, conn, list[i], &s);
>                  
>                  virDomainFree(list[i]);
> -                if (inst == NULL)
> +                if (inst == NULL) {
> +                        /* log the error */
> +                        const char *dom_name = virDomainGetName(list[i]);

You'll need to do this before you virDomainFree(list[i]) or reformat the
code a bit to do the virDomainFree() twice.  Once before the continue
below and once if inst != NULL

Or do the virDomainFree() below in a for loop before free(list).

The rest seems fine.

John

> +                        if (s.msg) {
> +                                CU_DEBUG("Failed to get VSSD instance from "
> +                                         "domain [%s], status msg [%s].",
> +                                         dom_name, CMGetCharPtr(s.msg));
> +                        } else {
> +                                CU_DEBUG("Failed to get VSSD instance from "
> +                                         "domain [%s].",
> +                                         dom_name);
> +                        }
> +                        /* restore s until last one */
> +                        if (i < count - 1) {
> +                                cu_statusf(_BROKER, &s,
> +                                            CMPI_RC_OK,
> +                                            "NULL");
> +                        }
> +                        fail_count++;
>                          continue;
> +                }
>  
>                  if (names_only)
>                          cu_return_instance_name(results, inst);
> @@ -343,6 +367,21 @@ static CMPIStatus return_enum_vssd(const CMPIObjectPath *reference,
>                          CMReturnInstance(results, inst);
>          }
>  
> +        /* check if some VS fail */
> +        if (fail_count > 0) {
> +                CU_DEBUG("Failed to get %d VSSD in enum, total is %d.",
> +                         fail_count, count);
> +               if (fail_count < count) {
> +                        /* consider it succeed, some VSSD will be returned */
> +                        cu_statusf(_BROKER, &s,
> +                                   CMPI_RC_OK,
> +                                   "Got %d/%d VSSD, "
> +                                   "some VS may changed during enum",
> +                                   count - fail_count, count);
> +                }
> +        }
> +
> +
>   out:
>          free(list);
>          virConnectClose(conn);
>