On 06/14/2018 12:14 AM, John Ferlan wrote:
On 06/08/2018 09:45 AM, Michal Privoznik wrote:
There are two sets of functions here: 1) some functions talk on both monitor and agent monitor, 2) some functions only talk on agent monitor.
For functions from set 1) we need to use qemuDomainObjBeginJobWithAgent() and for functions from set 2) we need to use qemuDomainObjBeginAgentJob() only.
It seems to me the category for (1) is more - some code makes the decision whether to use agent or normal code after the point at which we take the lock so we need to block not only agent, but also normal jobs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 91 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 28769878cc..bc1368386b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1956,6 +1956,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) bool useAgent = false, agentRequested, acpiRequested; bool isReboot = false; bool agentForced; + bool agentJob = false; int agentFlag = QEMU_AGENT_SHUTDOWN_POWERDOWN;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | @@ -1982,9 +1983,14 @@ 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) + if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) || + (useAgent && qemuDomainObjBeginJobWithAgent(driver, vm, + QEMU_JOB_MODIFY, + QEMU_AGENT_JOB_MODIFY) < 0))
I think if you change qemuDomainObjBeginAgentJob to use MODIFY_BLOCK, then this just becomes a single call...
As ugly as my proposal looks like I think it is still better than hiding taking of both jobs behind one value of qemuDomainJob while the rest of them don't do that. Michal