On Sat, May 05, 2018 at 12:48:45 -0500, Chris Venteicher wrote: > Start and connect to QEMU so QMP commands can be performed. > > Isolates code for starting QEMU and establishing Monitor connections > from code for obtaining capabilities so that arbitrary QMP commands can > be exchanged with QEMU. > --- > src/qemu/qemu_capabilities.c | 59 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index afce3eb2b7..097985cbe7 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -4303,6 +4303,65 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd, > } > > > +/* Start and connect to QEMU so QMP commands can be performed. > + */ > +static virQEMUCapsInitQMPCommandPtr > +virQEMUCapsSpinUpQemu(const char *exec, > + const char *libDir, uid_t runUid, gid_t runGid, > + bool forceTCG) This wrapper should have a different signature (accepting qemuCaps), and probably a different name which would suggest it's for onetime QMP commands. I'm not sure what the right name would be, though :-) > +{ > + virQEMUCapsInitQMPCommandPtr cmd = NULL; > + virQEMUCapsInitQMPCommandPtr rtn_cmd = NULL; > + char *binary = NULL; > + > + if (exec) { > + if (VIR_STRDUP(binary, exec) < 0) > + goto cleanup; > + } else { > + /* Check for existence of base emulator, or alternate base > + * which can be used with magic cpu choice > + */ > + virArch arch = virArchFromHost(); > + binary = virQEMUCapsFindBinaryForArch(arch, arch); > + } > + > + VIR_DEBUG("binary=%s", binary); We have a cache of QEMU capabilities for all binaries we found so there's no need to try to look for them again. > + > + /* Make sure the binary we are about to try exec'ing exists. > + * Technically we could catch the exec() failure, but that's > + * in a sub-process so it's hard to feed back a useful error. > + */ > + if (!virFileIsExecutable(binary)) { > + virReportSystemError(errno, _("QEMU binary %s is not executable"), > + binary); > + goto cleanup; > + } Getting qemuCaps from the cache will also make sure the binary is executable. > + > + if (!(cmd = virQEMUCapsInitQMPCommandNew(binary, libDir, > + runUid, runGid, NULL))) > + goto cleanup; This will use capabilities.monitor.sock and capabilities.pidfile for all processes. We either need to serialize this code with caps probing and also serialize all threads running baseline/compare QMP commands, or we need to refactor virQEMUCapsInitQMPCommandNew a bit so that it can be used simultaneously by several threads. > + > + if ((virQEMUCapsInitQMPCommandRun(cmd, forceTCG)) < 0) { No need for extra () since there's no assignment here. > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Error starting QEMU")); This would override any useful error already set by virQEMUCapsInitQMPCommandRun. > + goto cleanup; > + } > + > + if (!(cmd->mon)) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Error connecting to QEMU")); > + goto cleanup; > + } We should just check virQEMUCapsInitQMPCommandRun returned 0 and fail otherwise. > + > + VIR_STEAL_PTR(rtn_cmd, cmd); > + > + cleanup: > + virQEMUCapsInitQMPCommandFree(cmd); > + VIR_FREE(binary); > + > + return rtn_cmd; > +} > + > + > static int > virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, > const char *libDir, > -- > 2.14.1 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list