Re: [Libvirt-cim] [PATCH] Fix UUID in migration job lifecycle indications

I am okay with virConnectClose trying to close a NULL connection. I suggest we change to &uuid. Regards, Sharad Mishra Open Virtualization Linux Technology Center IBM libvirt-cim-bounces(a)redhat.com wrote on 05/03/2011 12:21:13 PM: > Chip Vincent <cvincent(a)linux.vnet.ibm.com&gt; > Sent by: libvirt-cim-bounces(a)redhat.com > > 05/03/2011 12:21 PM > > Please respond to > cvincent(a)linux.vnet.ibm.com; Please respond to > List for discussion and development of libvirt CIM <libvirt-cim(a)redhat.com&gt; > > To > > libvirt-cim(a)redhat.com > > cc > > Subject > > Re: [Libvirt-cim] [PATCH] Fix UUID in migration job lifecycle indications > > > > > Subject > > > > > > [Libvirt-cim] [PATCH] Fix UUID in migration job lifecycle indications > > > > > > # HG changeset patch > > > # User Chip Vincent <cvincent(a)us.ibm.com&gt; > > > # Date 1304034351 14400 > > > # Node ID 454ce8f30a13881cc6f5206d8e8e6f42a2ff8621 > > > # Parent 8b428df21c360d1eaedba7157b0dfd429d2db121 > > > Fix UUID in migration job lifecycle indications. > > > > > > Fixed the logic that fetches a VM UUID and adds it to the migration > > > job's InstanceIdentifier property. > > > > > > Siged-off-by: Chip Vincent <cvincent(a)us.ibm.com&gt; > > > > > > diff --git a/src/Virt_VSMigrationService.c > > b/src/Virt_VSMigrationService.c > > > --- a/src/Virt_VSMigrationService.c > > > +++ b/src/Virt_VSMigrationService.c > > > @@ -812,15 +812,20 @@ > > > CMPIInstance *ind = NULL; > > > CMPIInstance *prev_inst = NULL; > > > const char *pfx = NULL; > > > + virConnectPtr conn = NULL; > > > virDomainPtr dom = NULL; > > > char uuid[VIR_UUID_STRING_BUFLEN]; > > > CMPIDateTime *timestamp = NULL; > > > > > > + conn = connect_by_classname(_BROKER, job->ref_cn, s); > > > + if(conn == NULL) > > > + goto out; > > > > "out" will try to close a connection that has not been established yet. > > This is the same pattern most of the providers use and virConnectClose() > properly handles a NULL connection, much like > free handles NULL. I prefer this approach as opposed to having > multiple goto's for each failure scenario. > > > > > > + > > > ind_name = ind_type_to_name(ind_type); > > > > > > CU_DEBUG("Creating indication."); > > > > > > - pfx = pfx_from_conn(job->conn); > > > + pfx = pfx_from_conn(conn); > > > > > > ind = get_typed_instance(broker, > > > pfx, > > > @@ -832,13 +837,15 @@ > > > goto out; > > > } > > > > > > - dom = virDomainLookupByName(job->conn, job->domain); > > > - if(dom == NULL) { > > > - CU_DEBUG("Failed to connect to domain %s", job->domain); > > > + timestamp = CMNewDateTime(broker, s); > > > + CMSetProperty(ind, "IndicationTime", > > > + (CMPIValue *)&timestamp, CMPI_dateTime); > > > + > > > + dom = virDomainLookupByName(conn, job->domain); > > > + if (dom == NULL) > > > goto out; > > > - } > > > > > > - if(virDomainGetUUIDString(dom, uuid) != 0) { > > > + if (virDomainGetUUIDString(dom, &uuid[0]) != 0) { > > > > Why are you doing "&uuid[0]" and why not just "&uuid" ? > > libvirt test case for this function used that notation, so I simply > followed their lead regarding this style guideline. > > > > > > CU_DEBUG("Failed to get UUID from domain name"); > > > goto out; > > > } > > > @@ -846,10 +853,6 @@ > > > CMSetProperty(ind, "IndicationIdentifier", > > > (CMPIValue *)uuid, CMPI_chars); > > > > > > - timestamp = CMNewDateTime(broker, s); > > > - CMSetProperty(ind, "IndicationTime", > > > - (CMPIValue *)&timestamp, CMPI_dateTime); > > > - > > > if (ind_type == MIG_MODIFIED) { > > > /* Need to copy job inst before attaching as > > > PreviousInstance > > > because otherwise the changes we are about to make to job > > > @@ -867,6 +870,7 @@ > > > > > > out: > > > virDomainFree(dom); > > > + virConnectClose(conn); > > > return ind; > > > } > > > > > > _______________________________________________ > > > Libvirt-cim mailing list > > > Libvirt-cim(a)redhat.com > > > https://www.redhat.com/mailman/listinfo/libvirt-cim > > > > > > > > _______________________________________________ > > Libvirt-cim mailing list > > Libvirt-cim(a)redhat.com > > https://www.redhat.com/mailman/listinfo/libvirt-cim > > -- > Chip Vincent > Open Virtualization > IBM Linux Technology Center > cvincent(a)linux.vnet.ibm.com > _______________________________________________ > Libvirt-cim mailing list > Libvirt-cim(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvirt-cim _______________________________________________ Libvirt-cim mailing list Libvirt-cim(a)redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim