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

[Sharad Mishra]

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 at redhat.com wrote on 05/03/2011 12:21:13 PM:

> Chip Vincent <cvincent at linux.vnet.ibm.com>
> Sent by: libvirt-cim-bounces at redhat.com
>
> 05/03/2011 12:21 PM
>
> Please respond to
> cvincent at linux.vnet.ibm.com; Please respond to
> List for discussion and development of libvirt CIM
<libvirt-cim at redhat.com>
>
> To
>
> libvirt-cim at 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 at us.ibm.com>
> >  > # 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 at us.ibm.com>
> >  >
> >  > 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 at redhat.com
> >  > https://www.redhat.com/mailman/listinfo/libvirt-cim
> >
> >
> >
> > _______________________________________________
> > Libvirt-cim mailing list
> > Libvirt-cim at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvirt-cim
>
> --
> Chip Vincent
> Open Virtualization
> IBM Linux Technology Center
> cvincent at linux.vnet.ibm.com
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20110503/8e96a9bb/attachment.htm>