[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 1235493749 10800 # Node ID ec95377909381bf92beee417b5fce4f699950df5 # Parent 72dc446be12ca8ae1dc60a1aad97ff72eecd7b66 This patch exposes error messages from libvirt calls in VirtualSystemManagementService #2: Fixed code-style to match the file Removed un-needed custatusf() call Fixed a problem where status variable set by virt_set_status was being overriding by another call Added another libvirt error exposition point Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r 72dc446be12c -r ec9537790938 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Wed Feb 04 08:35:04 2009 -0800 +++ b/src/Virt_VirtualSystemManagementService.c Tue Feb 24 13:42:29 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; } @@ -1065,9 +1066,10 @@ dom = virDomainLookupByName(conn, name); if (dom == NULL) { - 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; } @@ -1274,13 +1276,16 @@ error: if (rc == IM_RC_SYS_NOT_FOUND) - cu_statusf(_BROKER, &status, - CMPI_RC_ERR_FAILED, - "Failed to find domain"); + virt_set_status(_BROKER, &status, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Referenced domain `%s' does not exist", + dom_name); else if (rc == IM_RC_FAILED) - cu_statusf(_BROKER, &status, - CMPI_RC_ERR_FAILED, - "Unable to retrieve domain name."); + virt_set_status(_BROKER, &status, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Unable to retrieve domain name"); else if (rc == IM_RC_OK) { status = (CMPIStatus){CMPI_RC_OK, NULL}; trigger_indication(context, @@ -1320,8 +1325,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, @@ -1459,9 +1469,10 @@ dom = virDomainLookupByName(conn, dominfo->name); if (dom == NULL) { CU_DEBUG("Failed to lookup VS `%s'", dominfo->name); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_NOT_FOUND, - "Virtual System `%s' not found", dominfo->name); + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Virtual System `%s' not found", dominfo->name); goto out; } @@ -1825,9 +1836,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; }
# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1235493767 10800 # Node ID 874a34b43248c303da834aa73333bfe26a53943c # Parent ec95377909381bf92beee417b5fce4f699950df5 This patch exposes error messages from libvirt calls in ComputerSystem #2: Added more points of error exposition Changed some error messages from "Could not..." to "Unable to..." to make them more readable Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r ec9537790938 -r 874a34b43248 src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Tue Feb 24 13:42:29 2009 -0300 +++ b/src/Virt_ComputerSystem.c Tue Feb 24 13:42:47 2009 -0300 @@ -273,7 +273,7 @@ struct infostore_ctx *infostore = NULL; ret = virDomainGetInfo(dom, &info); - if (ret != 0) + if (ret != 0) return 0; info.state = adjust_state_xen(dom, info.state); @@ -398,19 +398,27 @@ if (get_dominfo(dom, &domain) == 0) { CU_DEBUG("Unable to get domain information"); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get domain information"); + virt_set_status(broker, &s, + CMPI_RC_ERR_FAILED, + virDomainGetConnect(dom), + "Unable to get domain XML description"); goto out; } if (!set_name_from_dom(dom, instance)) { - /* Print trace error */ + virt_set_status(broker, &s, + CMPI_RC_ERR_FAILED, + virDomainGetConnect(dom), + "Unable to get domain name"); goto out; } if (!set_uuid_from_dom(dom, instance, &uuid)) { - /* Print trace error */ + virt_set_status(broker, &s, + CMPI_RC_ERR_FAILED, + virDomainGetConnect(dom), + "Unable to get domain UUID"); + goto out; } @@ -420,7 +428,11 @@ } if (!set_state_from_dom(broker, dom, instance)) { - /* Print trace error */ + virt_set_status(broker, &s, + CMPI_RC_ERR_FAILED, + virDomainGetConnect(dom), + "Unable to get domain info"); + goto out; } @@ -570,10 +582,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 +803,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 +1017,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; }
@@ -398,19 +398,27 @@
if (get_dominfo(dom, &domain) == 0) { CU_DEBUG("Unable to get domain information"); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get domain information"); + virt_set_status(broker, &s, + CMPI_RC_ERR_FAILED, + virDomainGetConnect(dom), + "Unable to get domain XML description");
Just a minor changer here.. We're doing 2 steps in the get_dominfo() call: 1) We're getting the XML from libvirt 2) We're parsing the XML and storing the domain info in a struct. So I would change this from "Unable to get domain XML description" to "Unable to get domain information" because the failure could be in either step. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1235493767 10800 # Node ID 1ff6cf0ca3a1dc8b1d507389cd7e1625c711ce50 # Parent 874a34b43248c303da834aa73333bfe26a53943c This patch exposes error messages from libvirt calls in VSMigrationService #2: Fixed compile error Changed code based on feedback from Kaitlin Rupert Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r 874a34b43248 -r 1ff6cf0ca3a1 src/Virt_VSMigrationService.c --- a/src/Virt_VSMigrationService.c Tue Feb 24 13:42:47 2009 -0300 +++ b/src/Virt_VSMigrationService.c Tue Feb 24 13:42:47 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, + virDomainGetConnect(dom), + "Error getting domain info"); goto out; } @@ -949,7 +970,7 @@ CU_DEBUG("Migration failed"); virt_set_status(_BROKER, &s, CMPI_RC_ERR_FAILED, - virDomainGetConnect(dom), + dconn, "Migration Failed"); } 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, + virDomainGetConnect(dom), + "Unable to shutdown guest"); goto out; } @@ -1005,9 +1027,14 @@ *xml = virDomainGetXMLDesc(dom, 0); if (*xml == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to retrieve domain XML."); + + virConnectPtr conn = virDomainGetConnect(dom); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to retrieve domain XML"); + goto out; } @@ -1025,6 +1052,7 @@ virDomainPtr dom; int i; int ret; + virConnectPtr lconn = virDomainGetConnect(ldom); for (i = 0; i < MIGRATE_SHUTDOWN_TIMEOUT; i++) { if ((i % 30) == 0) { @@ -1032,10 +1060,17 @@ virDomainGetName(ldom)); } - dom = virDomainLookupByName(virDomainGetConnect(ldom), + dom = virDomainLookupByName(lconn, virDomainGetName(ldom)); if (dom == NULL) { CU_DEBUG("Unable to re-lookup domain"); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + lconn, + "Domain `%s' not found", + virDomainGetName(ldom)); + ret = -1; break; } @@ -1055,9 +1090,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, + lconn, + "Failed to define domain"); goto out; } @@ -1067,9 +1103,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, + lconn, + "Failed to start domain on remote \ + host"); } } out: @@ -1086,9 +1124,13 @@ ret = virDomainGetInfo(dom, &info); if (ret == -1) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting domain info"); + virConnectPtr conn = virDomainGetConnect(dom); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Error getting domain info"); + goto out; } @@ -1117,9 +1159,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.
+ "Unable to get remote Hypervisor version"); goto out; }
@@ -1005,9 +1027,14 @@
*xml = virDomainGetXMLDesc(dom, 0); if (*xml == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to retrieve domain XML."); + + virConnectPtr conn = virDomainGetConnect(dom); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn,
This can just be virDomainGetConnect(dom) instead of conn, since conn isn't needed elsewhere in the function.
+ "Unable to retrieve domain XML"); + goto out; }
@@ -1025,6 +1052,7 @@ virDomainPtr dom; int i; int ret; + virConnectPtr lconn = virDomainGetConnect(ldom);
for (i = 0; i < MIGRATE_SHUTDOWN_TIMEOUT; i++) { if ((i % 30) == 0) { @@ -1032,10 +1060,17 @@ virDomainGetName(ldom)); }
- dom = virDomainLookupByName(virDomainGetConnect(ldom), + dom = virDomainLookupByName(lconn, virDomainGetName(ldom)); if (dom == NULL) { CU_DEBUG("Unable to re-lookup domain"); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + lconn, + "Domain `%s' not found", + virDomainGetName(ldom)); +
Setting an error here isn't necessary. We're polling to make sure the guest is undefined - we want the guest to disappear from the system. If we can't find the guest before the undefine call, it means the guest is already gone - which isn't an error.
ret = -1; break; } @@ -1055,9 +1090,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, + lconn,
You're defining the XML on the remote system - this needs to be rconn, not lconn.
+ "Failed to define domain"); goto out; }
@@ -1067,9 +1103,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, + lconn,
You're starting the guest on the remote system - this needs to be rconn, not lconn.
+ "Failed to start domain on remote \ + host"); } } out: @@ -1086,9 +1124,13 @@
ret = virDomainGetInfo(dom, &info); if (ret == -1) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting domain info"); + virConnectPtr conn = virDomainGetConnect(dom); + + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn,
Just use virDomainGetConnect(dom) here - conn isn't used elsewhere in the function.
+ "Error getting domain info"); + goto out; }
-- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (2)
-
Kaitlin Rupert -
Richard Maciel