On Tue, Jul 05, 2011 at 03:09:23PM +0200, Matthias Bolte wrote:
> 2011/7/4 Daniel P. Berrange <berrange@xxxxxxxxxx>:
> > 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list