9:30 a.m.
Previously, only virDomainObjIsActive() was used to check domain liveness.
This includes states like 'VIR_DOMAIN_PMSUSPENDED', where QEMU is running,
but the guest is not.
Explicitly check the state against VIR_DOMAIN_RUNNING and refuse to
communicate with the agent if it does not match.
https://bugzilla.redhat.com/show_bug.cgi?id=872424
Ján Tomko (3):
Check for qemu guest agent availability after getting the job
Pass virDomainObjPtr to qemuDomainAgentAvailable
Check if domain is running in qemuDomainAgentIsAvailable
src/qemu/qemu_domain.c | 11 ++++++++++-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++------------------------
3 files changed, 36 insertions(+), 26 deletions(-)
--
2.0.5
9:30 a.m.
New subject: [libvirt] [PATCH 1/3] Check for qemu guest agent availability after getting the job
This way checks requiring the job can be done in qemuDomainAgentAvailable.
---
src/qemu/qemu_driver.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e282464..d5b9d0d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1939,16 +1939,16 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags)
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
agentForced = agentRequested && !acpiRequested;
if (!qemuDomainAgentAvailable(priv, agentForced)) {
if (agentForced)
- goto cleanup;
+ goto endjob;
useAgent = false;
}
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
- goto cleanup;
-
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
@@ -2037,9 +2037,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
priv->agent))
useAgent = true;
- if (useAgent && !qemuDomainAgentAvailable(priv, true)) {
- goto cleanup;
- } else {
+ if (!useAgent) {
#if WITH_YAJL
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
@@ -2060,6 +2058,9 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
+ if (useAgent && !qemuDomainAgentAvailable(priv, true))
+ goto endjob;
+
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
@@ -17610,12 +17611,12 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
}
}
- if (!qemuDomainAgentAvailable(priv, true))
- goto cleanup;
-
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
+ if (!qemuDomainAgentAvailable(priv, true))
+ goto endjob;
+
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
@@ -17730,12 +17731,12 @@ qemuDomainQemuAgentCommand(virDomainPtr domain,
goto cleanup;
}
- if (!qemuDomainAgentAvailable(priv, true))
- goto cleanup;
-
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
+ if (!qemuDomainAgentAvailable(priv, true))
+ goto endjob;
+
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
@@ -17839,12 +17840,12 @@ qemuDomainFSTrim(virDomainPtr dom,
goto cleanup;
}
- if (!qemuDomainAgentAvailable(priv, true))
- goto cleanup;
-
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
+ if (!qemuDomainAgentAvailable(priv, true))
+ goto endjob;
+
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
--
2.0.5
9:30 a.m.
New subject: [libvirt] [PATCH 2/3] Pass virDomainObjPtr to qemuDomainAgentAvailable
Not just the DomainObj's private data.
---
src/qemu/qemu_domain.c | 4 +++-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 24 ++++++++++++------------
3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ac5ca74..f3f3910 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2798,9 +2798,11 @@ qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
}
bool
-qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv,
+qemuDomainAgentAvailable(virDomainObjPtr vm,
bool reportError)
{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
if (priv->agentError) {
if (reportError) {
virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index b2c3881..fe3e2b1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -395,7 +395,7 @@ bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
virDomainDefPtr src,
virDomainDefPtr dst);
-bool qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv,
+bool qemuDomainAgentAvailable(virDomainObjPtr vm,
bool reportError);
int qemuDomainJobInfoUpdateTime(qemuDomainJobInfoPtr jobInfo)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d5b9d0d..519705c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1943,7 +1943,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int
flags)
goto cleanup;
agentForced = agentRequested && !acpiRequested;
- if (!qemuDomainAgentAvailable(priv, agentForced)) {
+ if (!qemuDomainAgentAvailable(vm, agentForced)) {
if (agentForced)
goto endjob;
useAgent = false;
@@ -2058,7 +2058,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
- if (useAgent && !qemuDomainAgentAvailable(priv, true))
+ if (useAgent && !qemuDomainAgentAvailable(vm, true))
goto endjob;
if (!virDomainObjIsActive(vm)) {
@@ -4766,7 +4766,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
goto endjob;
}
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
if (nvcpus > vm->def->vcpus) {
@@ -5490,7 +5490,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
goto cleanup;
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
if (!virDomainObjIsActive(vm)) {
@@ -12892,7 +12892,7 @@ qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver
ATTRIBUTE_UNUSED,
qemuDomainObjPrivatePtr priv = vm->privateData;
int frozen;
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
return -1;
qemuDomainObjEnterAgent(vm);
@@ -12912,7 +12912,7 @@ qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver
ATTRIBUTE_UNUSED,
int thawed;
virErrorPtr err = NULL;
- if (!qemuDomainAgentAvailable(priv, report))
+ if (!qemuDomainAgentAvailable(vm, report))
return -1;
qemuDomainObjEnterAgent(vm);
@@ -17614,7 +17614,7 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
if (!virDomainObjIsActive(vm)) {
@@ -17734,7 +17734,7 @@ qemuDomainQemuAgentCommand(virDomainPtr domain,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
if (!virDomainObjIsActive(vm)) {
@@ -17843,7 +17843,7 @@ qemuDomainFSTrim(virDomainPtr dom,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
if (!virDomainObjIsActive(vm)) {
@@ -18027,7 +18027,7 @@ qemuDomainGetTime(virDomainPtr dom,
goto endjob;
}
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
qemuDomainObjEnterAgent(vm);
@@ -18086,7 +18086,7 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
qemuDomainObjEnterAgent(vm);
@@ -19063,7 +19063,7 @@ qemuDomainGetFSInfo(virDomainPtr dom,
goto endjob;
}
- if (!qemuDomainAgentAvailable(priv, true))
+ if (!qemuDomainAgentAvailable(vm, true))
goto endjob;
qemuDomainObjEnterAgent(vm);
--
2.0.5
9:30 a.m.
New subject: [libvirt] [PATCH 3/3] Check if domain is running in qemuDomainAgentIsAvailable
If the domain is not running, the agent will not respond.
Do not even try.
https://bugzilla.redhat.com/show_bug.cgi?id=872424
---
src/qemu/qemu_domain.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f3f3910..28bfbee 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2818,6 +2818,13 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
}
return false;
}
+ if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
+ if (reportError) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ }
+ return false;
+ }
return true;
}
--
2.0.5
5:01 p.m.
On 02/27/2015 08:30 AM, Ján Tomko wrote:
Previously, only virDomainObjIsActive() was used to check domain
liveness.
This includes states like 'VIR_DOMAIN_PMSUSPENDED', where QEMU is running,
but the guest is not.
Explicitly check the state against VIR_DOMAIN_RUNNING and refuse to
communicate with the agent if it does not match.
https://bugzilla.redhat.com/show_bug.cgi?id=872424
Ján Tomko (3):
Check for qemu guest agent availability after getting the job
Pass virDomainObjPtr to qemuDomainAgentAvailable
Check if domain is running in qemuDomainAgentIsAvailable
ACK series; since it fixes a bug it is probably safe for freeze.
src/qemu/qemu_domain.c | 11 ++++++++++-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++------------------------
3 files changed, 36 insertions(+), 26 deletions(-)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3559
days inactive
3559
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Ján Tomko