[libvirt] [PATCH 8/8] Implement code to attach to external QEMU instances.

[Matthias Bolte]

2011/7/4 Daniel P. Berrange <berrange at redhat.com>:
> Given a PID, the QEMU driver reads /proc/$PID/cmdline and
> /proc/$PID/environ to get the configuration. This is fed
> into the ARGV->XML convertor to build an XML configuration
> for the process.
>
> /proc/$PID/exe is resolved to identify the full command
> binary path
>
> After checking for name/uuid uniqueness, an attempt is
> made to connect to the monitor socket. If successful
> then 'info status' and 'info kvm' are issued to determine
> whether the CPUs are running and if KVM is enabled.
>
> * src/qemu/qemu_driver.c: Implement virDomainQemuAttach
> * src/qemu/qemu_process.h, src/qemu/qemu_process.c: Add
>  qemuProcessAttach to connect to the monitor of an
>  existing QEMU process
> ---
>  src/conf/domain_conf.c  |    3 +-
>  src/conf/domain_conf.h  |    1 +
>  src/qemu/qemu_command.c |    2 +
>  src/qemu/qemu_driver.c  |   91 +++++++++++++++++++-
>  src/qemu/qemu_process.c |  218 ++++++++++++++++++++++++++++++++++++++++++++---
>  src/qemu/qemu_process.h |    8 ++
>  6 files changed, 308 insertions(+), 15 deletions(-)

>  static int
>  qemuDomainOpenConsole(virDomainPtr dom,
>                       const char *devname,
> @@ -8539,6 +8625,7 @@ static virDriver qemuDriver = {
>     .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */
>     .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */
>     .qemuDomainMonitorCommand = qemuDomainMonitorCommand, /* 0.8.3 */
> +    .qemuDomainAttach = qemuDomainAttach, /* 0.9.3 */

s/0.9.3/0.9.4/

> +int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                      struct qemud_driver *driver,
> +                      virDomainObjPtr vm,
> +                      int pid,
> +                      const char *pidfile,
> +                      virDomainChrSourceDefPtr monConfig,
> +                      bool monJSON)
> +{
> +    char ebuf[1024];
> +    int logfile = -1;
> +    char *timestamp;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    bool running = true;
> +    virSecurityLabelPtr seclabel = NULL;
> +
> +    VIR_DEBUG("Beginning VM attach process");
> +
> +    if (virDomainObjIsActive(vm)) {
> +        qemuReportError(VIR_ERR_OPERATION_INVALID,
> +                        "%s", _("VM is already active"));
> +        return -1;
> +    }
> +
> +    /* Do this upfront, so any part of the startup process can add
> +     * runtime state to vm->def that won't be persisted. This let's us
> +     * report implicit runtime defaults in the XML, like vnc listen/socket
> +     */
> +    VIR_DEBUG("Setting current domain def as transient");
> +    if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0)
> +        goto cleanup;
> +
> +    vm->def->id = driver->nextvmid++;
> +
> +    if (virFileMakePath(driver->logDir) != 0) {
> +        virReportSystemError(errno,
> +                             _("cannot create log directory %s"),
> +                             driver->logDir);
> +        goto cleanup;
> +    }

This doesn't work as virFileMakePath doesn't set errno for all errors,
but returns an errno value. Needs to be something like this

    int err;
    if ((err = virFileMakePath(driver->logDir)) != 0) {
        virReportSystemError(err,
                             _("cannot create log directory %s"),
                             driver->logDir);
        goto cleanup;
    }

Also grep'ing for virFileMakePath shows that there are many instances
that use virFileMakePath in the wrong way.

> +    VIR_FREE(priv->pidfile);
> +    if (pidfile &&
> +        !(priv->pidfile = strdup(pidfile)))
> +        goto no_memory;
> +
> +    VIR_DEBUG("Detect security driver config");
> +    vm->def->seclabel.type = VIR_DOMAIN_SECLABEL_STATIC;
> +    if (VIR_ALLOC(seclabel) < 0)
> +        goto no_memory;
> +    if (virSecurityManagerGetProcessLabel(driver->securityManager,
> +                                          vm, seclabel) < 0)
> +        goto cleanup;
> +    if (!(vm->def->seclabel.model = strdup(driver->caps->host.secModel.model)))
> +        goto no_memory;
> +    if (!(vm->def->seclabel.label = strdup(seclabel->label)))
> +        goto no_memory;
> +
> +    VIR_DEBUG("Creating domain log file");
> +    if ((logfile = qemuDomainCreateLog(driver, vm, false)) < 0)
> +        goto cleanup;
> +
> +    VIR_DEBUG("Determining emulator version");
> +    qemuCapsFree(priv->qemuCaps);
> +    priv->qemuCaps = NULL;
> +    if (qemuCapsExtractVersionInfo(vm->def->emulator,
> +                                   vm->def->os.arch,
> +                                   NULL,
> +                                   &priv->qemuCaps) < 0)
> +        goto cleanup;
> +
> +    if (VIR_ALLOC(priv->monConfig) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    VIR_DEBUG("Preparing monitor state");
> +    priv->monConfig = monConfig;

Allocating and overwriting priv->monConfig leaks memory here.

ACK, with those problems fixed.

-- 
Matthias Bolte
http://photron.blogspot.com