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

[Daniel P. Berrange]

On Tue, Jul 05, 2011 at 03:09:23PM +0200, Matthias Bolte wrote:
> 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.

THis has since been fixed.

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

Fixed that too

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|