[PATCH 0 of 3] This series of patches exposes errors provided from executions of libvirt functions

# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1234981382 10800 # Node ID 9aeb111b89fb3b1428f8b21af5368a6b5c57f948 # Parent 72dc446be12ca8ae1dc60a1aad97ff72eecd7b66 This patch exposes error messages from libvirt calls in VirtualSystemManagementService Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r 72dc446be12c -r 9aeb111b89fb src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Wed Feb 04 08:35:04 2009 -0800 +++ b/src/Virt_VirtualSystemManagementService.c Wed Feb 18 15:23:02 2009 -0300 @@ -963,9 +963,10 @@ dom = virDomainLookupByName(conn, dominfo->name); if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_NOT_FOUND, - "Unable to lookup domain `%s'", dominfo->name); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Unable to lookup domain `%s'", dominfo->name); goto out; } @@ -1068,6 +1069,10 @@ cu_statusf(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, "Referenced domain `%s' does not exist", name); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' does not exist", name); goto out; } @@ -1252,6 +1257,10 @@ dom = virDomainLookupByName(conn, dom_name); if (dom == NULL) { CU_DEBUG("No such domain `%s'", dom_name); + virt_set_status(_BROKER, &status, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' does not exist", dom_name); rc = IM_RC_SYS_NOT_FOUND; goto error; } @@ -1320,8 +1329,13 @@ goto out; dom = virDomainLookupByName(conn, name); - if (dom == NULL) + if (dom == NULL) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' does not exist", name); goto out; + } if (!get_dominfo(dom, &dominfo)) { cu_statusf(_BROKER, &s, @@ -1825,9 +1839,11 @@ dom = virDomainLookupByName(conn, name); if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unknown system `%s'", name); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' \ + does not exist", name); goto end; }

@@ -1068,6 +1069,10 @@ cu_statusf(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, "Referenced domain `%s' does not exist", name);
Remove the call to cu_statusf().
+ virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' does not exist", name); goto out; }
@@ -1252,6 +1257,10 @@ dom = virDomainLookupByName(conn, dom_name); if (dom == NULL) { CU_DEBUG("No such domain `%s'", dom_name); + virt_set_status(_BROKER, &status, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' does not exist", dom_name);
When you set the status here, it will get overwritten by the calls to cu_statusf() on lines 1285 - 1292. Instead, replace the cu_statusf() calls on lines 1285 - 1292 with calls to virt_set_status().
@@ -1825,9 +1839,11 @@
dom = virDomainLookupByName(conn, name); if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unknown system `%s'", name); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' \ + does not exist", name);
Instead of splitting the string up, format like the following: virt_set_status(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, conn, "Referenced domain `%s' does not exist", name); -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

Also, what about changing line 1476? -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1234981406 10800 # Node ID 9ffb5f88de2b5abc58d80bb647c352e5ddf94de4 # Parent 9aeb111b89fb3b1428f8b21af5368a6b5c57f948 This patch exposes error messages from libvirt calls in ComputerSystem Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r 9aeb111b89fb -r 9ffb5f88de2b src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Wed Feb 18 15:23:02 2009 -0300 +++ b/src/Virt_ComputerSystem.c Wed Feb 18 15:23:26 2009 -0300 @@ -570,10 +570,11 @@ dom = virDomainLookupByName(conn, name); if (dom == NULL) { - cu_statusf(broker, &s, - CMPI_RC_ERR_NOT_FOUND, - "No such instance (%s).", - name); + virt_set_status(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' does not exist", + name); goto out; } @@ -790,9 +791,10 @@ xml = virDomainGetXMLDesc(dom, 0); if (xml == NULL) { CU_DEBUG("Unable to retrieve domain XML"); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get domain definition"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to get domain definition"); return s; } @@ -1003,16 +1005,18 @@ dom = virDomainLookupByName(conn, name); if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Domain not found"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Domain not found"); goto out; } if (virDomainGetInfo(dom, &info) != 0) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get current state"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to get current state"); goto out; }

Richard Maciel wrote:
# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1234981406 10800 # Node ID 9ffb5f88de2b5abc58d80bb647c352e5ddf94de4 # Parent 9aeb111b89fb3b1428f8b21af5368a6b5c57f948 This patch exposes error messages from libvirt calls in ComputerSystem
Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com>
diff -r 9aeb111b89fb -r 9ffb5f88de2b src/Virt_ComputerSystem.c
What about calls to: virDomainGetUUIDString() - line 101 (the error can be printed on line 413) virDomainGetInfo - line 275 -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1234981421 10800 # Node ID 2d7444518de4a50e511a2a854aa80634f3b559bf # Parent 9ffb5f88de2b5abc58d80bb647c352e5ddf94de4 This patch exposes error messages from libvirt calls in VSMigrationService Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r 9ffb5f88de2b -r 2d7444518de4 src/Virt_VSMigrationService.c --- a/src/Virt_VSMigrationService.c Wed Feb 18 15:23:26 2009 -0300 +++ b/src/Virt_VSMigrationService.c Wed Feb 18 15:23:41 2009 -0300 @@ -259,16 +259,18 @@ unsigned long remote; if (virConnectGetVersion(conn, &local)) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get local Hypervisor version"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to get local Hypervisor version"); goto out; } if (virConnectGetVersion(dconn, &remote)) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get remote hypervisor version"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to get remote Hypervisor version"); goto out; } @@ -396,12 +398,28 @@ pid_t pid; int i; int rc = -1; + virConnectPtr conn = virDomainGetConnect(dom); + const char *name = virDomainGetName(dom); + const char *uri = virConnectGetURI(conn); + + if (name == NULL) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Failed to get domain name"); + goto out; + } + + if (uri == NULL) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Failed to get URI of connection"); + goto out; + } pid = fork(); if (pid == 0) { - virConnectPtr conn = virDomainGetConnect(dom); - const char *name = virDomainGetName(dom); - const char *uri = virConnectGetURI(conn); if (setpgrp() == -1) perror("setpgrp"); @@ -436,6 +454,7 @@ free(name); } + out: return s; } @@ -589,9 +608,10 @@ dom = virDomainLookupByName(conn, domain); if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_NOT_FOUND, - "No such domain"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "No such domain"); goto out; } @@ -930,9 +950,10 @@ ret = virDomainGetInfo(dom, &info); if (ret == -1) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting domain info"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Error getting domain info"); goto out; } @@ -969,9 +990,10 @@ CU_DEBUG("Shutting down domain for migration"); ret = virDomainShutdown(dom); if (ret != 0) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to shutdown guest"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + dconn, + "Unable to shutdown guest"); goto out; } @@ -1005,10 +1027,21 @@ *xml = virDomainGetXMLDesc(dom, 0); if (*xml == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to retrieve domain XML."); - goto out; + + virConnectPtr conn = virDomainGetConnect(dom); + if (conn == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to retrieve domain XML."); + goto out; + } + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to retrieve domain XML"); + + goto out; } out: @@ -1036,6 +1069,13 @@ virDomainGetName(ldom)); if (dom == NULL) { CU_DEBUG("Unable to re-lookup domain"); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + rconn, + "Domain `%s' not found", + virDomainGetName(ldom)); + ret = -1; break; } @@ -1055,9 +1095,10 @@ newdom = virDomainDefineXML(rconn, xml); if (newdom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Failed to define domain"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + rconn, + "Failed to define domain"); goto out; } @@ -1067,9 +1108,11 @@ CU_DEBUG("Restarting domain on remote host"); if (virDomainCreate(newdom) != 0) { CU_DEBUG("Failed to start domain on remote host"); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Failed to start domain on remote host"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + rconn, + "Failed to start domain on remote \ + host"); } } out: @@ -1086,9 +1129,20 @@ ret = virDomainGetInfo(dom, &info); if (ret == -1) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting domain info"); + virConnectPtr conn = virDomainGetConnect(dom); + if (conn == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Error getting domain info"); + + goto out; + } + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Error getting domain info"); + goto out; } @@ -1117,9 +1171,11 @@ dom = virDomainLookupByName(conn, job->domain); if (dom == NULL) { CU_DEBUG("Failed to lookup `%s'", job->domain); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Failed to lookup domain `%s'", job->domain); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Failed to lookup domain `%s'", job->domain); + goto out; }

if (virConnectGetVersion(dconn, &remote)) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get remote hypervisor version"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn,
This needs to be dconn, not conn.
@@ -930,9 +950,10 @@
ret = virDomainGetInfo(dom, &info); if (ret == -1) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting domain info"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn,
There isn't a conn variable in this function. You need to use: virDomainGetConnect(dom);
+ "Error getting domain info"); goto out; }
Can you fix the bug on line 973? That should be using dconn, not virDomainGetConnect(dom);
@@ -969,9 +990,10 @@ CU_DEBUG("Shutting down domain for migration"); ret = virDomainShutdown(dom); if (ret != 0) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to shutdown guest"); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + dconn,
This should be virDomainGetConnect(dom), not dconn. You're shutting down the guest on the local system, but the remote system. Actually.. dconn isn't used in the function. Should be removed from the function at some point, but that's a different patch altogether....
+ "Unable to shutdown guest"); goto out; }
@@ -1005,10 +1027,21 @@
*xml = virDomainGetXMLDesc(dom, 0); if (*xml == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to retrieve domain XML."); - goto out; + + virConnectPtr conn = virDomainGetConnect(dom); + if (conn == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to retrieve domain XML."); + goto out; + }
No need for this check here. virt_set_status() handles NULL connections.
+ + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to retrieve domain XML"); + + goto out;
The goto out isn't aligned properly.
}
out: @@ -1036,6 +1069,13 @@ virDomainGetName(ldom)); if (dom == NULL) { CU_DEBUG("Unable to re-lookup domain"); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + rconn,
Should use virDomainGetConnect(ldom) instead of rconn.
+ "Domain `%s' not found", + virDomainGetName(ldom)); +
@@ -1086,9 +1129,20 @@
ret = virDomainGetInfo(dom, &info); if (ret == -1) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting domain info"); + virConnectPtr conn = virDomainGetConnect(dom); + if (conn == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Error getting domain info"); + + goto out; + }
No need to do this check - virt_set_status() handles NULL connections. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (2)
-
Kaitlin Rupert
-
Richard Maciel