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@xxxxxxxxxx> >> --- >> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
