> Subject
>
> [Libvirt-cim] [PATCH] Fix UUID in migration job lifecycle indications
>
> # HG changeset patch
> # User Chip Vincent <cvincent(a)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(a)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(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