[libvirt] PATCH: Improve error reporting for operations on inactive domains
Daniel P. Berrange
berrange at redhat.com
Fri Apr 17 11:39:42 UTC 2009
If you try todo an operation on an inactive QEMU guest which is not
applicable, eg ask to pause an inactive guest, then you currently get
a useless message
# virsh suspend demo
error: Failed to suspend domain demo
error: invalid domain pointer in no domain with matching id -1
There are two issues here
- The QEMU driver is mistakenly doing a lookup-by-id when locating the
guest in question. It should in fact do lookup-by-uuid for all APIs
since that's the best unique identifier.
- It was using VIR_ERR_INVALID_DOMAIN code instead of VIR_ERR_NO_DOMAIN
code, hence the 'invalid domain pointer' bogus message.
This patch changes all QEMU APIs to lookup based on UUID, and use the
correct VIR_ERR_NO_DOMAIN code when reporting failures.
You now get to see the real useful error message later in the API where
it validates whether the guest is running or not
# virsh suspend demo
error: Failed to suspend domain demo
error: operation failed: domain is not running
One thing I'm wondering, is whether we should introduce an explicit error
code for operations that are not applicable.
eg, instead of giving VIR_ERR_OPERATION_FAILED, we could give back a code
like VIR_ERR_OPERATION_INVALID. This would let callers distinguish
real failure of the operation, vs the fact that it simply isn't applicable
for inactive guests.
Daniel
diff -r 5b88ef324d90 src/qemu_driver.c
--- a/src/qemu_driver.c Fri Apr 17 12:06:21 2009 +0100
+++ b/src/qemu_driver.c Fri Apr 17 12:32:54 2009 +0100
@@ -1984,7 +1984,8 @@ static virDomainPtr qemudDomainLookupByI
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching id %d"), id);
goto cleanup;
}
@@ -2008,7 +2009,10 @@ static virDomainPtr qemudDomainLookupByU
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(uuid, uuidstr);
+ qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuid);
goto cleanup;
}
@@ -2032,7 +2036,8 @@ static virDomainPtr qemudDomainLookupByN
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching name '%s'"), name);
goto cleanup;
}
@@ -2182,11 +2187,14 @@ static int qemudDomainSuspend(virDomainP
virDomainEventPtr event = NULL;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- qemuDriverUnlock(driver);
-
- if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ qemuDriverUnlock(driver);
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
if (!virDomainIsActive(vm)) {
@@ -2232,12 +2240,14 @@ static int qemudDomainResume(virDomainPt
virDomainEventPtr event = NULL;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- qemuDriverUnlock(driver);
-
- if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ qemuDriverUnlock(driver);
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
if (!virDomainIsActive(vm)) {
@@ -2281,12 +2291,14 @@ static int qemudDomainShutdown(virDomain
int ret = -1;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- qemuDriverUnlock(driver);
-
- if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ qemuDriverUnlock(driver);
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2312,10 +2324,12 @@ static int qemudDomainDestroy(virDomainP
virDomainEventPtr event = NULL;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2349,8 +2363,10 @@ static char *qemudDomainGetOSType(virDom
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2375,9 +2391,8 @@ static unsigned long qemudDomainGetMaxMe
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
-
- virUUIDFormat(dom->uuid, uuidstr);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
_("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2401,9 +2416,8 @@ static int qemudDomainSetMaxMemory(virDo
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
-
- virUUIDFormat(dom->uuid, uuidstr);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
_("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2525,9 +2539,8 @@ static int qemudDomainSetMemory(virDomai
qemuDriverUnlock(driver);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
-
- virUUIDFormat(dom->uuid, uuidstr);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
_("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2566,8 +2579,10 @@ static int qemudDomainGetInfo(virDomainP
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2713,11 +2728,13 @@ static int qemudDomainSave(virDomainPtr
header.version = QEMUD_SAVE_VERSION;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
-
- if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -2850,9 +2867,8 @@ static int qemudDomainSetVcpus(virDomain
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
-
- virUUIDFormat(dom->uuid, uuidstr);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
_("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -3045,9 +3061,8 @@ static int qemudDomainGetMaxVcpus(virDom
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
-
- virUUIDFormat(dom->uuid, uuidstr);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
_("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -3080,9 +3095,8 @@ static int qemudDomainGetSecurityLabel(v
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
-
- virUUIDFormat(dom->uuid, uuidstr);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
_("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -3294,8 +3308,10 @@ static char *qemudDomainDumpXML(virDomai
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -3496,8 +3512,10 @@ static int qemudDomainStart(virDomainPtr
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -3586,8 +3604,10 @@ static int qemudDomainUndefine(virDomain
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -3979,9 +3999,11 @@ static int qemudDomainAttachDevice(virDo
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
if (!vm) {
- qemuDriverUnlock(driver);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ qemuDriverUnlock(driver);
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -4129,9 +4151,11 @@ static int qemudDomainDetachDevice(virDo
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
if (!vm) {
- qemuDriverUnlock(driver);
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ qemuDriverUnlock(driver);
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -4182,8 +4206,10 @@ static int qemudDomainGetAutostart(virDo
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -4208,8 +4234,10 @@ static int qemudDomainSetAutostart(virDo
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -4283,11 +4311,13 @@ qemudDomainBlockStats (virDomainPtr dom,
virDomainDiskDefPtr disk = NULL;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- qemuDriverUnlock(driver);
- if (!vm) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ qemuDriverUnlock(driver);
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
if (!virDomainIsActive (vm)) {
@@ -4418,12 +4448,14 @@ qemudDomainInterfaceStats (virDomainPtr
int ret = -1;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- qemuDriverUnlock(driver);
-
- if (!vm) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ qemuDriverUnlock(driver);
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -4486,8 +4518,10 @@ qemudDomainBlockPeek (virDomainPtr dom,
qemuDriverUnlock(driver);
if (!vm) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -4554,12 +4588,14 @@ qemudDomainMemoryPeek (virDomainPtr dom,
int fd = -1, ret = -1;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- qemuDriverUnlock(driver);
-
- if (!vm) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ qemuDriverUnlock(driver);
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -4887,10 +4923,12 @@ qemudDomainMigratePerform (virDomainPtr
int paused = 0;
qemuDriverLock(driver);
- vm = virDomainFindByID(&driver->domains, dom->id);
- if (!vm) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching id %d"), dom->id);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
goto cleanup;
}
@@ -5018,8 +5056,8 @@ qemudDomainMigrateFinish2 (virConnectPtr
qemuDriverLock(driver);
vm = virDomainFindByName(&driver->domains, dname);
if (!vm) {
- qemudReportError (dconn, NULL, NULL, VIR_ERR_INVALID_DOMAIN,
- _("no domain with matching name %s"), dname);
+ qemudReportError (dconn, NULL, NULL, VIR_ERR_NO_DOMAIN,
+ _("no domain with matching name '%s'"), dname);
goto cleanup;
}
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list