[libvirt] [PATCH] qemu: move the guest status check before agent config and status check

[lhuang]

On 07/08/2015 04:57 PM, Michal Privoznik wrote:
> On 03.07.2015 08:58, Luyao Huang wrote:
>> When use setvcpus command with --guest option to a offline vm,
>> we will get error:
>>
>>   # virsh setvcpus test3 1 --guest
>>   error: Guest agent is not responding: QEMU guest agent is not connected
>>
>> However guest is not running, agent status could not be connected.
>> In this case, report domain is not running will be better than agent is
>> not connected. Move the guest status check more early to output error to
>> point out guest status is not right.
>>
>> Also from the logic, a running vm is a basic requirement to use
>> agent, we cannot use agent if vm is not running.
>>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>>   src/qemu/qemu_domain.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index f9bf32c..814fb2c 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -3084,6 +3084,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;
>> +    }
>>       if (!priv->agent) {
>>           if (qemuFindAgentConfig(vm->def)) {
>>               if (reportError) {
>> @@ -3099,13 +3106,6 @@ 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;
>>   }
>>   
>>
> I think the check could have been moved even one block up. I mean, it
> could be the very first check in the function.

Indeed

> I've moved the check, ACKed and pushed.

Thanks a lot for your help and review.

> Michal

Luyao