[PATCH] Fix UUID in migration job lifecycle indications
# HG changeset patch # User Chip Vincent <cvincent@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@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; + 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) { 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; }
Sharad Mishra Open Virtualization Linux Technology Center IBM libvirt-cim-bounces@redhat.com wrote on 04/28/2011 04:51:13 PM:
Chip Vincent <cvincent@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
04/28/2011 04:51 PM
Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
Subject
[Libvirt-cim] [PATCH] Fix UUID in migration job lifecycle indications
# HG changeset patch # User Chip Vincent <cvincent@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@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.
+ 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" ?
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@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
Subject
[Libvirt-cim] [PATCH] Fix UUID in migration job lifecycle indications
# HG changeset patch # User Chip Vincent <cvincent@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
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@redhat.com wrote on 05/03/2011 12:21:13 PM:
Chip Vincent <cvincent@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
05/03/2011 12:21 PM
Please respond to cvincent@linux.vnet.ibm.com; Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@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@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com _______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
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@redhat.com wrote on 05/03/2011 12:21:13 PM:
Chip Vincent <cvincent@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
05/03/2011 12:21 PM
Please respond to cvincent@linux.vnet.ibm.com; Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@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@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com _______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
Please disregard. You made a typo and I'm sleepy... Using just 'uuid' is sufficient. Will post updated patch shortly.
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.
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
participants (2)
-
Chip Vincent -
Sharad Mishra