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

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(a)redhat.com&gt; > --- > 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.

I've moved the check, ACKed and pushed.

Michal