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

2011/7/4 Daniel P. Berrange <berrange(a)redhat.com&gt;: > 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.

> +    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.