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@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to