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