[libvirt] [PATCHv1 3/7] qemu_capabilities: Start and connect to QEMU

Chris Venteicher posted 7 patches 7 years ago
[libvirt] [PATCHv1 3/7] qemu_capabilities: Start and connect to QEMU
Posted by Chris Venteicher 7 years ago
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)
+{
+    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);
+
+    /* 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;
+    }
+
+    if (!(cmd = virQEMUCapsInitQMPCommandNew(binary, libDir,
+                                             runUid, runGid, NULL)))
+        goto cleanup;
+
+    if ((virQEMUCapsInitQMPCommandRun(cmd, forceTCG)) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Error starting QEMU"));
+        goto cleanup;
+    }
+
+    if (!(cmd->mon)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Error connecting to QEMU"));
+        goto cleanup;
+    }
+
+    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
Re: [libvirt] [PATCHv1 3/7] qemu_capabilities: Start and connect to QEMU
Posted by Jiri Denemark 6 years, 12 months ago
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