Re: [libvirt] [PATCH 8/8] Implement code to attach to external QEMU instances.
On Tue, Jul 05, 2011 at 03:09:23PM +0200, Matthias Bolte wrote:
> 2011/7/4 Daniel P. Berrange :
> > 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/8] Implement code to attach to external QEMU instances.
2011/7/4 Daniel P. Berrange :
> 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
[libvirt] [PATCH 8/8] Implement code to attach to external QEMU instances.
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(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 109a947..c5b6dd5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -55,7 +55,8 @@ VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
"custom-monitor",
"high-privileges",
"shell-scripts",
- "disk-probing");
+ "disk-probing",
+ "external-launch");
VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST,
"qemu",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 258289a..071bf50 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1249,6 +1249,7 @@ enum virDomainTaintFlags {
VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, /* Running with undesirably high
privileges */
VIR_DOMAIN_TAINT_SHELL_SCRIPTS,/* Network configuration using opaque
shell scripts */
VIR_DOMAIN_TAINT_DISK_PROBING, /* Relying on potentially unsafe disk
format probing */
+VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH, /* Externally launched guest domain */
VIR_DOMAIN_TAINT_LAST
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index dbc9e0c..aa66b55 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6182,6 +6182,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
if (!(def->name = strndup(val, process - val)))
goto no_memory;
}
+if (STREQ(def->name, ""))
+VIR_FREE(def->name);
} else if (STREQ(arg, "-M")) {
WANT_VALUE();
if (!(def->os.machine = strdup(val)))
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9486594..05643bc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4129,7 +4129,7 @@ qemudCanonicalizeMachineFromInfo(virDomainDefPtr def,
if (!machine->canonical)
continue;
-if (STRNEQ(def->os.machine, machine->name))
+if (def->os.machine && STRNEQ(def->os.machine, machine->name))
continue;
if (!(*canonical = strdup(machine->canonical))) {
@@ -4156,7 +4156,7 @@ qemudCanonicalizeMachineDirect(virDomainDefPtr def, char
**canonical)
if (!machines[i]->canonical)
continue;
-if (STRNEQ(def->os.machine, machines[i]->name))
+if (def->os.machine && STRNEQ(def->os.machine, machines[i]->name))
continue;
*canonical = machines[i]->canonical;
@@ -8347,6 +8347,92 @@ cleanup:
}
+static virDomainPtr qemuDomainAttach(virConnectPtr conn,
+ unsigned long long pid,
+ unsigned int flags)
+{
+struct qemud_driver *driver = conn->privateData;
+virDomainObjPtr vm = NULL;
+virDomainDefPtr def = NULL;
+virDomainPtr dom = NULL;
+virDomainChrSourceDefPtr monConfig = NULL;
+bool monJSON = false;
+char *pidfile;
+
+virCheckFlags(0, NULL);
+
+qemuDriverLock(driver);
+
+if (!(def = qemuParseCommandLinePid(driver->caps, pid,
+&pidfile, &monConfig, &monJSON)))
+goto cleanup;
+
+if (!monConfig) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("No monitor connection for pid %llu"),
+pid);
+goto cleanup;
+}
+if (monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("Cannot connect to monitor connection of type '%s'
for pid %llu"),
+virDomainChrTypeToString(monConfig->type), pid);
+goto cleanup;
+}
+
+if (!(def->name) &&
+virAsprintf(&def->name, "attach-pid-%llu", pid) < 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
+goto cleanup;
+
+if (qemudCanonicalizeMachine(driver, def) < 0)
+goto cleanup;
+
+
[libvirt] [PATCH 8/8] Implement code to attach to external QEMU instances.
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 | 220 ---
src/qemu/qemu_process.h |8 ++
6 files changed, 308 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5360863..8de6ce6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -55,7 +55,8 @@ VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
"custom-monitor",
"high-privileges",
"shell-scripts",
- "disk-probing");
+ "disk-probing",
+ "external-launch");
VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST,
"qemu",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ff5c28d..90a3229 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1248,6 +1248,7 @@ enum virDomainTaintFlags {
VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, /* Running with undesirably high
privileges */
VIR_DOMAIN_TAINT_SHELL_SCRIPTS,/* Network configuration using opaque
shell scripts */
VIR_DOMAIN_TAINT_DISK_PROBING, /* Relying on potentially unsafe disk
format probing */
+VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH, /* Externally launched guest domain */
VIR_DOMAIN_TAINT_LAST
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 41093c0..f5cbf6b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6156,6 +6156,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
if (!(def->name = strndup(val, process - val)))
goto no_memory;
}
+if (STREQ(def->name, ""))
+VIR_FREE(def->name);
} else if (STREQ(arg, "-M")) {
WANT_VALUE();
if (!(def->os.machine = strdup(val)))
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 20a3d59..282b69b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3924,7 +3924,7 @@ qemudCanonicalizeMachineFromInfo(virDomainDefPtr def,
if (!machine->canonical)
continue;
-if (STRNEQ(def->os.machine, machine->name))
+if (def->os.machine && STRNEQ(def->os.machine, machine->name))
continue;
if (!(*canonical = strdup(machine->canonical))) {
@@ -3951,7 +3951,7 @@ qemudCanonicalizeMachineDirect(virDomainDefPtr def, char
**canonical)
if (!machines[i]->canonical)
continue;
-if (STRNEQ(def->os.machine, machines[i]->name))
+if (def->os.machine && STRNEQ(def->os.machine, machines[i]->name))
continue;
*canonical = machines[i]->canonical;
@@ -8138,6 +8138,92 @@ cleanup:
}
+static virDomainPtr qemuDomainAttach(virConnectPtr conn,
+ unsigned long long pid,
+ unsigned int flags)
+{
+struct qemud_driver *driver = conn->privateData;
+virDomainObjPtr vm = NULL;
+virDomainDefPtr def = NULL;
+virDomainPtr dom = NULL;
+virDomainChrSourceDefPtr monConfig = NULL;
+bool monJSON = false;
+char *pidfile;
+
+virCheckFlags(0, NULL);
+
+qemuDriverLock(driver);
+
+if (!(def = qemuParseCommandLinePid(driver->caps, pid,
+&pidfile, &monConfig, &monJSON)))
+goto cleanup;
+
+if (!monConfig) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("No monitor connection for pid %llu"),
+pid);
+goto cleanup;
+}
+if (monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("Cannot connect to monitor connection of type '%s'
for pid %llu"),
+virDomainChrTypeToString(monConfig->type), pid);
+goto cleanup;
+}
+
+if (!(def->name) &&
+virAsprintf(&def->name, "attach-pid-%llu", pid) < 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
+goto cleanup;
+
+if (qemudCanonicalizeMachine(driver, def) < 0)
+goto cleanup;
+
+
