Re: [libvirt] [PATCH v2] qemu: Prevent crash of libvirtd without guest agent configuration

On 07/16/13 11:30, Alex Jia wrote: > If users haven't configured guest agent then qemuAgentCommand() will > dereference a NULL 'mon' pointer, which causes crash of libvirtd. > > With the patch, when the qemu-ga service isn't running in the guest, > a expected error "error: Guest agent is not responding: Guest agent > not available for now" will be raised, and the error "error: argument > unsupported: QEMU guest agent is not configured" is raised when the > guest hasn't configured guest agent. > > > GDB backtrace: > > (gdb) bt > #0 virNetServerFatalSignal (sig=11, siginfo=<value optimized out>, > context=<value optimized out>) at rpc/virnetserver.c:326 > #1 <signal handler called> > #2 qemuAgentCommand (mon=0x0, cmd=0x7f39300017b0, > reply=0x7f394b090910, seconds=-2) at qemu/qemu_agent.c:975 > #3 0x00007f39429507f6 in qemuAgentGetVCPUs (mon=0x0, > info=0x7f394b0909b8) at qemu/qemu_agent.c:1475 > #4 0x00007f39429d9857 in qemuDomainGetVcpusFlags (dom=<value > optimized out>, flags=9) at qemu/qemu_driver.c:4849 > #5 0x00007f3957dffd8d in virDomainGetVcpusFlags > (domain=0x7f39300009c0, flags=8) at libvirt.c:9843 > > How to reproduce? > > # To start a guest without guest agent configuration > # then run the following cmdline > > # virsh vcpucount foobar --guest > error: End of file while reading data: Input/output error > error: One or more references were leaked after disconnect from the > hypervisor > error: Failed to reconnect to the hypervisor > > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=984821 > > Signed-off-by: Alex Jia <ajia(a)redhat.com&gt; > --- > src/qemu/qemu_driver.c | 26 ++++++++++++++++++++++++++ > 1 files changed, 26 insertions(+), 0 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 495867a..699388c 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -4122,6 +4122,19 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, > unsigned int nvcpus, > goto endjob; > } > > + if (priv->agentError) { > + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", > + _("QEMU guest agent is not " > + "available due to an error")); > + goto cleanup; this needs to be "goto endjob" > + } > + > + if (!priv->agent) { > + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > + _("QEMU guest agent is not configured")); > + goto cleanup; here too. > + } > + > qemuDomainObjEnterAgent(vm); > ncpuinfo = qemuAgentGetVCPUs(priv->agent, &cpuinfo); > qemuDomainObjExitAgent(vm); > @@ -4839,6 +4852,19 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, > unsigned int flags) > if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) > goto cleanup; > > + if (priv->agentError) { > + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", > + _("QEMU guest agent is not " > + "available due to an error")); > + goto cleanup; > + } > + > + if (!priv->agent) { > + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > + _("QEMU guest agent is not configured")); > + goto cleanup; > + } > + > if (!virDomainObjIsActive(vm)) { > virReportError(VIR_ERR_OPERATION_INVALID, "%s", > _("domain is not running")); > and also both here. ACK with that changed.

Peter