[libvirt] [PATCH 1/2] Introduce new VIR_ERR_AGENT_UNRESPONSIVE error code
Currently, when guest agent is configure but not responsive (e.g. due to appropriate service not running in the guest) we return VIR_ERR_INTERNAL_ERROR or VIR_ERR_ARGUMENT_UNSUPPORTED. Both are wrong. Therefore we need to introduce new error code to reflect this case. --- include/libvirt/virterror.h | 2 ++ src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_driver.c | 24 ++++++++++++++---------- src/util/virterror.c | 7 +++++++ 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 69c64aa..5140c38 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -283,6 +283,8 @@ typedef enum { VIR_ERR_OPERATION_UNSUPPORTED = 84, /* The requested operation is not supported */ VIR_ERR_SSH = 85, /* error in ssh transport driver */ + VIR_ERR_AGENT_UNRESPONSIVE = 86, /* guest agent is unresponsive, + not running or not usable */ } virErrorNumber; /** diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index c658bf8..ba29783 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -880,7 +880,7 @@ static int qemuAgentSend(qemuAgentPtr mon, if ((timeout && virCondWaitUntil(&mon->notify, &mon->lock, then) < 0) || (!timeout && virCondWait(&mon->notify, &mon->lock) < 0)) { if (errno == ETIMEDOUT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", _("Guest agent not available for now")); ret = -2; } else { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3fcca0e..f64d9ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1734,8 +1734,9 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { if (useAgent) { if (priv->agentError) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("QEMU guest agent is not available due to an error")); + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); goto cleanup; } if (!priv->agent) { @@ -1815,8 +1816,9 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) if (useAgent) { if (priv->agentError) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("QEMU guest agent is not available due to an error")); + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); goto cleanup; } if (!priv->agent) { @@ -10391,7 +10393,7 @@ qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, int freezed; if (priv->agentError) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", _("QEMU guest agent is not " "available due to an error")); return -1; @@ -10419,7 +10421,7 @@ qemuDomainSnapshotFSThaw(struct qemud_driver *driver, if (priv->agentError) { if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", _("QEMU guest agent is not " "available due to an error")); return -1; @@ -13708,8 +13710,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, } if (priv->agentError) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("QEMU guest agent is not available due to an error")); + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); goto cleanup; } @@ -13849,8 +13852,9 @@ qemuDomainAgentCommand(virDomainPtr domain, } if (priv->agentError) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("QEMU guest agent is not available due to an error")); + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); goto cleanup; } diff --git a/src/util/virterror.c b/src/util/virterror.c index 3ee2ae0..7caa69e 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1199,6 +1199,13 @@ virErrorMsg(virErrorNumber error, const char *info) errmsg = _("SSH transport error"); else errmsg = _("SSH transport error: %s"); + break; + case VIR_ERR_AGENT_UNRESPONSIVE: + if (info == NULL) + errmsg = _("Guest agent is not responding"); + else + errmsg = _("Guest agent is not responding: %s"); + break; } return errmsg; } -- 1.7.8.6
Currently, if a syscall in qemu_agent.c fails we report an internal error even though we should be reporting the system error. --- src/qemu/qemu_agent.c | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index ba29783..aaff9fc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -191,14 +191,16 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) } if (virSetNonBlock(monfd) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to put monitor into non-blocking mode")); + virReportSystemError(errno, "%s", + _("Unable to put monitor " + "into non-blocking mode")); goto error; } if (virSetCloseExec(monfd) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to set monitor close-on-exec flag")); + virReportSystemError(errno, "%s", + _("Unable to set monitor " + "close-on-exec flag")); goto error; } @@ -256,14 +258,14 @@ qemuAgentOpenPty(const char *monitor) int monfd; if ((monfd = open(monitor, O_RDWR | O_NONBLOCK)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to open monitor path %s"), monitor); + virReportSystemError(errno, + _("Unable to open monitor path %s"), monitor); return -1; } if (virSetCloseExec(monfd) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to set monitor close-on-exec flag")); + virReportSystemError(errno, "%s", + _("Unable to set monitor close-on-exec flag")); goto error; } @@ -733,14 +735,14 @@ qemuAgentOpen(virDomainObjPtr vm, return NULL; if (virMutexInit(&mon->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor mutex")); + virReportSystemError(errno, "%s", + _("cannot initialize monitor mutex")); VIR_FREE(mon); return NULL; } if (virCondInit(&mon->notify) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor condition")); + virReportSystemError(errno, "%s", + _("cannot initialize monitor condition")); virMutexDestroy(&mon->lock); VIR_FREE(mon); return NULL; @@ -884,8 +886,8 @@ static int qemuAgentSend(qemuAgentPtr mon, _("Guest agent not available for now")); ret = -2; } else { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to wait on monitor condition")); + virReportSystemError(errno, "%s", + _("Unable to wait on monitor condition")); } goto cleanup; } -- 1.7.8.6
On 08/27/2012 01:57 PM, Michal Privoznik wrote:
Currently, if a syscall in qemu_agent.c fails we report an internal error even though we should be reporting the system error.
s/the/a/ ACK, Martin
On 08/27/2012 01:57 PM, Michal Privoznik wrote:
Currently, when guest agent is configure but not responsive
s/configure/configured/
(e.g. due to appropriate service not running in the guest) we return VIR_ERR_INTERNAL_ERROR or VIR_ERR_ARGUMENT_UNSUPPORTED.
There is no VIR_ERR_ARGUMENT_UNSUPPORTED in the patch.
Both are wrong. Therefore we need to introduce new error code to reflect this case. ---
ACK with the commit message fixed, Martin P.S.: Looks like a nice start of the 'INTERNAL_ERROR' usage abusement ;)
On 27.08.2012 14:11, Martin Kletzander wrote:
On 08/27/2012 01:57 PM, Michal Privoznik wrote:
Currently, when guest agent is configure but not responsive
s/configure/configured/
(e.g. due to appropriate service not running in the guest) we return VIR_ERR_INTERNAL_ERROR or VIR_ERR_ARGUMENT_UNSUPPORTED.
There is no VIR_ERR_ARGUMENT_UNSUPPORTED in the patch.
Both are wrong. Therefore we need to introduce new error code to reflect this case. ---
ACK with the commit message fixed,
Thanks. I think this is a bug fix rather than enhancement so I've pushed the whole series. Thanks. Michal
participants (2)
-
Martin Kletzander -
Michal Privoznik