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

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(a)linux.vnet.ibm.com&gt; > --- > 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); > _______________________________________________ Libvirt-cim mailing list Libvirt-cim(a)redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim