[Libvirt-cim] [PATCH] Fix UUID in migration job lifecycle indications
Chip Vincent
cvincent at linux.vnet.ibm.com
Fri May 6 00:48:05 UTC 2011
Sharad,
I finally got a chance to try your suggestion and it did not work correctly.
Virt_VSMigrationService.c: In function ‘prepare_indication’:
Virt_VSMigrationService.c:848: error: passing argument 2 of
‘virDomainGetUUIDString’ from incompatible pointer type
/usr/include/libvirt/libvirt.h:678: note: expected ‘char *’ but argument
is of type ‘char (*)[37]’
This is because '&uuid[0]' is equivalent to &(*uuid), not &uuid.
On 05/03/2011 03:44 PM, Sharad Mishra wrote:
> 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 *)×tamp, 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 *)×tamp, 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
>
>
>
> _______________________________________________
> 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
More information about the Libvirt-cim
mailing list