[libvirt] [PATCH 4/4] qemu: Switch code to use new agent job APIs

Michal Privoznik mprivozn at redhat.com
Thu Jun 14 14:22:24 UTC 2018


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 at 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




More information about the libvir-list mailing list