5:08 p.m.
This patch adds support for using the monitor interface to set the VNC
password
(qemu) change vnc password
Password: ********
A minor tricky thing is that we can't just send the command and password
all in one go, we must wait for the 'Password' prompt before sending the
password.
When doing this I noticed that virsh dumpxml has no way to request a
secure XML dump (required to see the password element), nor did the
virsh edit command set the SECURE or INACTIVE flags when changing
the XML.
qemu_conf.c | 45 ++++++++++++-----------
qemu_driver.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--------------
virsh.c | 30 ++++++++++-----
3 files changed, 131 insertions(+), 56 deletions(-)
Daniel
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1138,37 +1138,42 @@ int qemudBuildCommandLine(virConnectPtr
if (vm->def->graphics &&
vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
- char vncdisplay[PATH_MAX];
- int ret;
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ char *optstr;
if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) {
- char options[PATH_MAX] = "";
+ if (vm->def->graphics->data.vnc.listenAddr)
+ virBufferAdd(&opt, vm->def->graphics->data.vnc.listenAddr,
-1);
+ else if (driver->vncListen)
+ virBufferAdd(&opt, driver->vncListen, -1);
+
+ virBufferVSprintf(&opt, ":%d",
+ vm->def->graphics->data.vnc.port - 5900);
+
+ if (vm->def->graphics->data.vnc.passwd)
+ virBufferAddLit(&opt, ",password");
+
if (driver->vncTLS) {
- strcat(options, ",tls");
+ virBufferAddLit(&opt, ",tls");
if (driver->vncTLSx509verify) {
- strcat(options, ",x509verify=");
+ virBufferVSprintf(&opt, ",x509verify=%s",
+ driver->vncTLSx509certdir);
} else {
- strcat(options, ",x509=");
+ virBufferVSprintf(&opt, ",x509=%s",
+ driver->vncTLSx509certdir);
}
- strncat(options, driver->vncTLSx509certdir,
- sizeof(options) - (strlen(driver->vncTLSx509certdir)-1));
- options[sizeof(options)-1] = '\0';
}
- ret = snprintf(vncdisplay, sizeof(vncdisplay), "%s:%d%s",
- (vm->def->graphics->data.vnc.listenAddr ?
- vm->def->graphics->data.vnc.listenAddr :
- (driver->vncListen ? driver->vncListen :
"")),
- vm->def->graphics->data.vnc.port - 5900,
- options);
} else {
- ret = snprintf(vncdisplay, sizeof(vncdisplay), "%d",
- vm->def->graphics->data.vnc.port - 5900);
+ virBufferVSprintf(&opt, "%d",
+ vm->def->graphics->data.vnc.port - 5900);
}
- if (ret < 0 || ret >= (int)sizeof(vncdisplay))
- goto error;
+ if (virBufferError(&opt))
+ goto no_memory;
+
+ optstr = virBufferContentAndReset(&opt);
ADD_ARG_LIT("-vnc");
- ADD_ARG_LIT(vncdisplay);
+ ADD_ARG(optstr);
if (vm->def->graphics->data.vnc.keymap) {
ADD_ARG_LIT("-k");
ADD_ARG_LIT(vm->def->graphics->data.vnc.keymap);
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -74,6 +74,10 @@
/* For storing short-lived temporary files. */
#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt"
+#define QEMU_CMD_PROMPT "\n(qemu) "
+#define QEMU_PASSWD_PROMPT "Password: "
+
+
static int qemudShutdown(void);
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -138,9 +142,14 @@ static void qemudShutdownVMDaemon(virCon
static int qemudDomainGetMaxVcpus(virDomainPtr dom);
-static int qemudMonitorCommand (const virDomainObjPtr vm,
- const char *cmd,
- char **reply);
+static int qemudMonitorCommand(const virDomainObjPtr vm,
+ const char *cmd,
+ char **reply);
+static int qemudMonitorCommandExtra(const virDomainObjPtr vm,
+ const char *cmd,
+ const char *extra,
+ const char *extraPrompt,
+ char **reply);
static struct qemud_driver *qemu_driver = NULL;
@@ -1012,6 +1021,36 @@ qemudInitCpus(virConnectPtr conn,
}
+static int
+qemudInitPasswords(virConnectPtr conn,
+ virDomainObjPtr vm) {
+ char *info = NULL;
+
+ /*
+ * NB: Might have more passwords to set in the future. eg a qcow
+ * disk decryption password, but there's no monitor command
+ * for that yet...
+ */
+
+ if (vm->def->graphics &&
+ vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
+ vm->def->graphics->data.vnc.passwd) {
+
+ if (qemudMonitorCommandExtra(vm, "change vnc password",
+ vm->def->graphics->data.vnc.passwd,
+ QEMU_PASSWD_PROMPT,
+ &info) < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("resume operation failed"));
+ return -1;
+ }
+ VIR_FREE(info);
+ }
+
+ return 0;
+}
+
+
static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
int i;
@@ -1204,7 +1243,8 @@ static int qemudStartVMDaemon(virConnect
if (ret == 0) {
if ((qemudWaitForMonitor(conn, driver, vm, pos) < 0) ||
(qemudDetectVcpuPIDs(conn, vm) < 0) ||
- (qemudInitCpus(conn, vm, migrateFrom) < 0)) {
+ (qemudInitCpus(conn, vm, migrateFrom) < 0) ||
+ (qemudInitPasswords(conn, vm) < 0)) {
qemudShutdownVMDaemon(conn, driver, vm);
return -1;
}
@@ -1314,12 +1354,15 @@ cleanup:
}
static int
-qemudMonitorCommand (const virDomainObjPtr vm,
- const char *cmd,
- char **reply) {
+qemudMonitorCommandExtra(const virDomainObjPtr vm,
+ const char *cmd,
+ const char *extra,
+ const char *extraPrompt,
+ char **reply) {
int size = 0;
char *buf = NULL;
size_t cmdlen = strlen(cmd);
+ size_t extralen = extra ? strlen(extra) : 0;
if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen)
return -1;
@@ -1355,25 +1398,34 @@ qemudMonitorCommand (const virDomainObjP
}
/* Look for QEMU prompt to indicate completion */
- if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
- char *commptr = NULL, *nlptr = NULL;
-
- /* Preserve the newline */
- tmp[1] = '\0';
-
- /* The monitor doesn't dump clean output after we have written to
- * it. Every character we write dumps a bunch of useless stuff,
- * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
- * Try to throw away everything before the first full command
- * occurence, and inbetween the command and the newline starting
- * the response
- */
- if ((commptr = strstr(buf, cmd)))
- memmove(buf, commptr, strlen(commptr)+1);
- if ((nlptr = strchr(buf, '\n')))
- memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
-
- break;
+ if (buf) {
+ if (extra) {
+ if (strstr(buf, extraPrompt) != NULL) {
+ if (safewrite(vm->monitor, extra, extralen) != extralen)
+ return -1;
+ if (safewrite(vm->monitor, "\r", 1) != 1)
+ return -1;
+ extra = NULL;
+ }
+ } else if ((tmp = strstr(buf, QEMU_CMD_PROMPT)) != NULL) {
+ char *commptr = NULL, *nlptr = NULL;
+ /* Preserve the newline */
+ tmp[1] = '\0';
+
+ /* The monitor doesn't dump clean output after we have written to
+ * it. Every character we write dumps a bunch of useless stuff,
+ * so the result looks like
"cXcoXcomXcommXcommaXcommanXcommand"
+ * Try to throw away everything before the first full command
+ * occurence, and inbetween the command and the newline starting
+ * the response
+ */
+ if ((commptr = strstr(buf, cmd)))
+ memmove(buf, commptr, strlen(commptr)+1);
+ if ((nlptr = strchr(buf, '\n')))
+ memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
+
+ break;
+ }
}
pollagain:
/* Need to wait for more data */
@@ -1403,6 +1455,14 @@ qemudMonitorCommand (const virDomainObjP
return -1;
}
+static int
+qemudMonitorCommand(const virDomainObjPtr vm,
+ const char *cmd,
+ char **reply) {
+ return qemudMonitorCommandExtra(vm, cmd, NULL, NULL, reply);
+}
+
+
/**
* qemudProbe:
*
diff --git a/src/virsh.c b/src/virsh.c
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -2079,6 +2079,8 @@ static const vshCmdInfo info_dumpxml[] =
static const vshCmdOptDef opts_dumpxml[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id
or uuid")},
+ {"inactive", VSH_OT_BOOL, 0, gettext_noop("show inactive defined
XML")},
+ {"secure", VSH_OT_BOOL, 0, gettext_noop("include security sensitive
data")},
{NULL, 0, 0, NULL}
};
@@ -2088,14 +2090,22 @@ cmdDumpXML(vshControl *ctl, const vshCmd
virDomainPtr dom;
int ret = TRUE;
char *dump;
-
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
- return FALSE;
-
- if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- return FALSE;
-
- dump = virDomainGetXMLDesc(dom, 0);
+ int flags = 0;
+ int inactive = vshCommandOptBool(cmd, "inactive");
+ int secure = vshCommandOptBool(cmd, "secure");
+
+ if (inactive)
+ flags |= VIR_DOMAIN_XML_INACTIVE;
+ if(secure)
+ flags |= VIR_DOMAIN_XML_SECURE;
+
+ if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ return FALSE;
+
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ return FALSE;
+
+ dump = virDomainGetXMLDesc(dom, flags);
if (dump != NULL) {
printf("%s", dump);
free(dump);
@@ -5374,7 +5384,7 @@ cmdEdit (vshControl *ctl, const vshCmd *
goto cleanup;
/* Get the XML configuration of the domain. */
- doc = virDomainGetXMLDesc (dom, 0);
+ doc = virDomainGetXMLDesc (dom, VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE);
if (!doc)
goto cleanup;
@@ -5404,7 +5414,7 @@ cmdEdit (vshControl *ctl, const vshCmd *
* it was being edited? This also catches problems such as us
* losing a connection or the domain going away.
*/
- doc_reread = virDomainGetXMLDesc (dom, 0);
+ doc_reread = virDomainGetXMLDesc (dom, VIR_DOMAIN_XML_SECURE |
VIR_DOMAIN_XML_INACTIVE);
if (!doc_reread)
goto cleanup;
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:04 p.m.
On Tue, Jan 20, 2009 at 11:08:56PM +0000, Daniel P. Berrange wrote:
[..snip..]
+static int
+qemudInitPasswords(virConnectPtr conn,
+ virDomainObjPtr vm) {
+ char *info = NULL;
+
+ /*
+ * NB: Might have more passwords to set in the future. eg a qcow
+ * disk decryption password, but there's no monitor command
+ * for that yet...
+ */
+
+ if (vm->def->graphics &&
+ vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
+ vm->def->graphics->data.vnc.passwd) {
+
+ if (qemudMonitorCommandExtra(vm, "change vnc password",
+ vm->def->graphics->data.vnc.passwd,
+ QEMU_PASSWD_PROMPT,
+ &info) < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("resume operation failed"));
\--- cut'n'paste?
+ return -1;
+ }
+ VIR_FREE(info);
+ }
+
+ return 0;
+}
-- Guido
6:48 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
This patch adds support for using the monitor interface to set the
VNC
password
(qemu) change vnc password
Password: ********
A minor tricky thing is that we can't just send the command and password
all in one go, we must wait for the 'Password' prompt before sending the
password.
When doing this I noticed that virsh dumpxml has no way to request a
secure XML dump (required to see the password element), nor did the
virsh edit command set the SECURE or INACTIVE flags when changing
the XML.
...
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1138,37 +1138,42 @@ int qemudBuildCommandLine(virConnectPtr
if (vm->def->graphics &&
vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
- char vncdisplay[PATH_MAX];
- int ret;
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ char *optstr;
...
+ optstr = virBufferContentAndReset(&opt);
ADD_ARG_LIT("-vnc");
- ADD_ARG_LIT(vncdisplay);
+ ADD_ARG(optstr);
Looks fine.
Just be sure to add this:
VIR_FREE(optstr);
Also, there are a few new lines longer than 80.
BTW, can you outline what you do to test this?
and I'll see about adding something to exercise the new code.
7:05 a.m.
On Wed, Jan 21, 2009 at 01:48:25PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> This patch adds support for using the monitor interface to set the VNC
> password
>
> (qemu) change vnc password
> Password: ********
>
> A minor tricky thing is that we can't just send the command and password
> all in one go, we must wait for the 'Password' prompt before sending the
> password.
>
> When doing this I noticed that virsh dumpxml has no way to request a
> secure XML dump (required to see the password element), nor did the
> virsh edit command set the SECURE or INACTIVE flags when changing
> the XML.
...
> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> --- a/src/qemu_conf.c
> +++ b/src/qemu_conf.c
> @@ -1138,37 +1138,42 @@ int qemudBuildCommandLine(virConnectPtr
>
> if (vm->def->graphics &&
> vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> - char vncdisplay[PATH_MAX];
> - int ret;
> + virBuffer opt = VIR_BUFFER_INITIALIZER;
> + char *optstr;
...
> + optstr = virBufferContentAndReset(&opt);
>
> ADD_ARG_LIT("-vnc");
> - ADD_ARG_LIT(vncdisplay);
> + ADD_ARG(optstr);
Looks fine.
Just be sure to add this:
VIR_FREE(optstr);
No need - ADD_ARG() owns the pointer - ADD_ARG_LIT() strdups, hence why
I changed it.
BTW, can you outline what you do to test this?
and I'll see about adding something to exercise the new code.
There's two parts to testing
- The ARGV for QEMU - trivially to add more datafiles to qemuxml2argvtest.c
- The monitor interaction
We have no way to testing monitor interaction, and definitely do not want
to be in the business of running real QEMU instances to test this. What
I think we should do is to create a fake / mock QEMU binary in the tests
directory, which simulates the monitor console. It would output pre-defined
responses for each of the monitor commands we care about. This would let
us unit test each of the individual functions that process monitor commands
in isolation. It actally might be best to just use a self-pipe in the test
case and have a thread to provide the server end of the monitor - that
would let us trivially tell it what response we want to get back to each
command we're about to test.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:59 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
No need - ADD_ARG() owns the pointer - ADD_ARG_LIT() strdups, hence
why
I changed it.
Ah. glad you didn't add it ;-)
> BTW, can you outline what you do to test this?
> and I'll see about adding something to exercise the new code.
There's two parts to testing
- The ARGV for QEMU - trivially to add more datafiles to qemuxml2argvtest.c
I've just done that.
- The monitor interaction
We have no way to testing monitor interaction, and definitely do not want
to be in the business of running real QEMU instances to test this. What
I think we should do is to create a fake / mock QEMU binary in the tests
directory, which simulates the monitor console. It would output pre-defined
responses for each of the monitor commands we care about. This would let
us unit test each of the individual functions that process monitor commands
in isolation. It actally might be best to just use a self-pipe in the test
case and have a thread to provide the server end of the monitor - that
would let us trivially tell it what response we want to get back to each
command we're about to test.
Sounds reasonable.
11:01 a.m.
New subject: [libvirt] [PATCH] tests: add qemu/argv test for vnc-passwd feature [was: Support VNC password for QEMU guests
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
This patch adds support for using the monitor interface to set the
VNC
password
(qemu) change vnc password
Password: ********
A minor tricky thing is that we can't just send the command and password
all in one go, we must wait for the 'Password' prompt before sending the
password.
When doing this I noticed that virsh dumpxml has no way to request a
secure XML dump (required to see the password element), nor did the
virsh edit command set the SECURE or INACTIVE flags when changing
the XML.
qemu_conf.c | 45 ++++++++++++-----------
qemu_driver.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--------------
As you suggested, here's a test of the command-line/xml-processing part:
A couple minor things I'm going to change eventually:
1) The name of every file in the qemuxml2argvdata/ directory
starts with qemuxml2argv. I'm going to remove those suffixes
and adjust the few callers.
2) Currently, each test case (pair of .args, .xml files) is checked
in separately, giving no clue as to how similar the inputs of one
test may be to another. Seeing such relationships would make
it easier to add new tests.
For example in this case, I've just added `, passwd="123456"'
to the <graphics.../> line in the existing vnc-passwd.xml file:
$ sed '/<graphics/s,/, passwd="123456"/,'
qemuxml2argv-graphics-vnc.xml \
| diff -u - qemuxml2argv-vnc-passwd.xml
- <graphics type='vnc' ... listen='127.0.0.1'/>
+ <graphics type='vnc' ... listen='127.0.0.1'
passwd="123456"/>
and appended ",password" to its .args file:
$ sed 's/:3$/:3,password/' qemuxml2argv-graphics-vnc.args \
| diff -u - qemuxml2argv-vnc-passwd.args
-LC_ALL=C ... -vnc 127.0.0.1:3
+LC_ALL=C ... -vnc 127.0.0.1:3,password
From ab770e0becc5b13cbd59f0f1219bb719f29e20d5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Wed, 21 Jan 2009 17:22:59 +0100
Subject: [PATCH] tests: add an qemu/argv test for the new vnc-passwd feature
* tests/qemuxml2argvtest.c (mymain): Run the new test.
* tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.xml: New file.
* tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.args: Likewise.
---
.../qemuxml2argvdata/qemuxml2argv-vnc-passwd.args | 1 +
tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.xml | 24 ++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 +
3 files changed, 27 insertions(+), 0 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.args
b/tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.args
new file mode 100644
index 0000000..30cc17b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214
-smp 1 -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda
/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -vnc
127.0.0.1:3,password
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.xml
b/tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.xml
new file mode 100644
index 0000000..da6ecf3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-vnc-passwd.xml
@@ -0,0 +1,24 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ </disk>
+ <input type='mouse' bus='ps2'/>
+ <graphics type='vnc' port='5903' autoport='no'
listen='127.0.0.1' passwd="123456"/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 17684fd..5607ea0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -226,6 +226,8 @@ mymain(int argc, char **argv)
DO_TEST("hostdev-pci-address", 0);
+ DO_TEST("vnc-passwd", 0);
+
virCapabilitiesFree(driver.caps);
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
--
1.6.1.347.g0238
2:22 p.m.
Daniel P. Berrange napsal(a):
This patch adds support for using the monitor interface to set the
VNC
password
(qemu) change vnc password
Password: ********
A minor tricky thing is that we can't just send the command and password
all in one go, we must wait for the 'Password' prompt before sending the
password.
When doing this I noticed that virsh dumpxml has no way to request a
secure XML dump (required to see the password element), nor did the
virsh edit command set the SECURE or INACTIVE flags when changing
the XML.
qemu_conf.c | 45 ++++++++++++-----------
qemu_driver.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--------------
virsh.c | 30 ++++++++++-----
3 files changed, 131 insertions(+), 56 deletions(-)
Daniel
I tried the patch and I've found that define command do not set the
password as the SECURE flag is not set when defining the domain from XML
file. This can be very unpleasant as it effectively disables the
password when redefining the domain. I've tried this simple dirty hotfix
diff -ur b/domain_conf.c a/domain_conf.c
--- b/domain_conf.c 2008-12-23 14:20:02.000000000 +0100
+++ a/domain_conf.c 2009-01-25 20:12:47.000000000 +0100
@@ -2297,7 +2297,7 @@
}
def = virDomainDefParseNode(conn, caps, xml, root,
- VIR_DOMAIN_XML_INACTIVE);
+ VIR_DOMAIN_XML_INACTIVE |
VIR_DOMAIN_XML_SECURE );
cleanup:
xmlFreeParserCtxt (pctxt);
and it works. However there must be some better way how to do it but I
do not know the libvirt source code good enough to find it.
Radek
P.S. I've also found that virt-manager is missing handler for login
button when asking for password but I will send email to virt-manager
mailing list about it.
5:15 a.m.
On Sun, Jan 25, 2009 at 09:22:53PM +0100, Radek Hladik wrote:
Daniel P. Berrange napsal(a):
>This patch adds support for using the monitor interface to set the VNC
>password
>
> (qemu) change vnc password
> Password: ********
>
>A minor tricky thing is that we can't just send the command and password
>all in one go, we must wait for the 'Password' prompt before sending the
>password.
>
>When doing this I noticed that virsh dumpxml has no way to request a
>secure XML dump (required to see the password element), nor did the
>virsh edit command set the SECURE or INACTIVE flags when changing
>the XML.
>
> qemu_conf.c | 45 ++++++++++++-----------
> qemu_driver.c | 112
> ++++++++++++++++++++++++++++++++++++++++++++--------------
> virsh.c | 30 ++++++++++-----
> 3 files changed, 131 insertions(+), 56 deletions(-)
>
>Daniel
>
I tried the patch and I've found that define command do not set the
password as the SECURE flag is not set when defining the domain from XML
file. This can be very unpleasant as it effectively disables the
password when redefining the domain. I've tried this simple dirty hotfix
diff -ur b/domain_conf.c a/domain_conf.c
--- b/domain_conf.c 2008-12-23 14:20:02.000000000 +0100
+++ a/domain_conf.c 2009-01-25 20:12:47.000000000 +0100
@@ -2297,7 +2297,7 @@
}
def = virDomainDefParseNode(conn, caps, xml, root,
- VIR_DOMAIN_XML_INACTIVE);
+ VIR_DOMAIN_XML_INACTIVE |
VIR_DOMAIN_XML_SECURE );
cleanup:
xmlFreeParserCtxt (pctxt);
and it works. However there must be some better way how to do it but I
do not know the libvirt source code good enough to find it.
That is the correct place to put the flag - I'll include this in my
patches. We need to tell it to parse the passwd when loading the files
from disk upon daemon startup.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:48 a.m.
On Mon, Jan 26, 2009 at 11:15:01AM +0000, Daniel P. Berrange wrote:
On Sun, Jan 25, 2009 at 09:22:53PM +0100, Radek Hladik wrote:
> Daniel P. Berrange napsal(a):
> >This patch adds support for using the monitor interface to set the VNC
> >password
> >
> > (qemu) change vnc password
> > Password: ********
> >
> >A minor tricky thing is that we can't just send the command and password
> >all in one go, we must wait for the 'Password' prompt before sending
the
> >password.
> >
> >When doing this I noticed that virsh dumpxml has no way to request a
> >secure XML dump (required to see the password element), nor did the
> >virsh edit command set the SECURE or INACTIVE flags when changing
> >the XML.
> >
> > qemu_conf.c | 45 ++++++++++++-----------
> > qemu_driver.c | 112
> > ++++++++++++++++++++++++++++++++++++++++++++--------------
> > virsh.c | 30 ++++++++++-----
> > 3 files changed, 131 insertions(+), 56 deletions(-)
> >
> >Daniel
> >
>
> I tried the patch and I've found that define command do not set the
> password as the SECURE flag is not set when defining the domain from XML
> file. This can be very unpleasant as it effectively disables the
> password when redefining the domain. I've tried this simple dirty hotfix
>
> diff -ur b/domain_conf.c a/domain_conf.c
> --- b/domain_conf.c 2008-12-23 14:20:02.000000000 +0100
> +++ a/domain_conf.c 2009-01-25 20:12:47.000000000 +0100
> @@ -2297,7 +2297,7 @@
> }
>
> def = virDomainDefParseNode(conn, caps, xml, root,
> - VIR_DOMAIN_XML_INACTIVE);
> + VIR_DOMAIN_XML_INACTIVE |
> VIR_DOMAIN_XML_SECURE );
>
> cleanup:
> xmlFreeParserCtxt (pctxt);
>
>
> and it works. However there must be some better way how to do it but I
> do not know the libvirt source code good enough to find it.
That is the correct place to put the flag - I'll include this in my
patches. We need to tell it to parse the passwd when loading the files
from disk upon daemon startup.
Actually this is not required. The password is always parsed from the
XML. The secure flag is only required when generating XML for output.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
7:12 a.m.
Daniel P. Berrange napsal(a):
On Mon, Jan 26, 2009 at 11:15:01AM +0000, Daniel P. Berrange wrote:
> On Sun, Jan 25, 2009 at 09:22:53PM +0100, Radek Hladik wrote:
>
>> Daniel P. Berrange napsal(a):
>>
>>> This patch adds support for using the monitor interface to set the VNC
>>> password
>>>
>>> (qemu) change vnc password
>>> Password: ********
>>>
>>> A minor tricky thing is that we can't just send the command and password
>>> all in one go, we must wait for the 'Password' prompt before sending
the
>>> password.
>>>
>>> When doing this I noticed that virsh dumpxml has no way to request a
>>> secure XML dump (required to see the password element), nor did the
>>> virsh edit command set the SECURE or INACTIVE flags when changing
>>> the XML.
>>>
>>> qemu_conf.c | 45 ++++++++++++-----------
>>> qemu_driver.c | 112
>>> ++++++++++++++++++++++++++++++++++++++++++++--------------
>>> virsh.c | 30 ++++++++++-----
>>> 3 files changed, 131 insertions(+), 56 deletions(-)
>>>
>>> Daniel
>>>
>>>
>> I tried the patch and I've found that define command do not set the
>> password as the SECURE flag is not set when defining the domain from XML
>> file. This can be very unpleasant as it effectively disables the
>> password when redefining the domain. I've tried this simple dirty hotfix
>>
>> diff -ur b/domain_conf.c a/domain_conf.c
>> --- b/domain_conf.c 2008-12-23 14:20:02.000000000 +0100
>> +++ a/domain_conf.c 2009-01-25 20:12:47.000000000 +0100
>> @@ -2297,7 +2297,7 @@
>> }
>>
>> def = virDomainDefParseNode(conn, caps, xml, root,
>> - VIR_DOMAIN_XML_INACTIVE);
>> + VIR_DOMAIN_XML_INACTIVE |
>> VIR_DOMAIN_XML_SECURE );
>>
>> cleanup:
>> xmlFreeParserCtxt (pctxt);
>>
>>
>> and it works. However there must be some better way how to do it but I
>> do not know the libvirt source code good enough to find it.
>>
> That is the correct place to put the flag - I'll include this in my
> patches. We need to tell it to parse the passwd when loading the files
> from disk upon daemon startup.
>
Actually this is not required. The password is always parsed from the
XML. The secure flag is only required when generating XML for output.
Daniel
I am not sure about this. I've downloaded latest hourly snapshot
yesterday, patched it with the VNC password patch and specified password
via passwd attribute in xml. I've started the daemon and VNC required
password - so far so good. However I've changed something other in xml
file, reloaded the xml with virsh 'define test.xml' command and the
machine didn't require VNC password any more. The supplied patch solved
the issue...
Radek
9:27 a.m.
On Tue, Jan 20, 2009 at 11:08:56PM +0000, Daniel P. Berrange wrote:
This patch adds support for using the monitor interface to set the
VNC
password
(qemu) change vnc password
Password: ********
A minor tricky thing is that we can't just send the command and password
all in one go, we must wait for the 'Password' prompt before sending the
password.
When doing this I noticed that virsh dumpxml has no way to request a
secure XML dump (required to see the password element), nor did the
virsh edit command set the SECURE or INACTIVE flags when changing
the XML.
qemu_conf.c | 45 ++++++++++++-----------
qemu_driver.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--------------
virsh.c | 30 ++++++++++-----
3 files changed, 131 insertions(+), 56 deletions(-)
This update also makes it possible to set a global default password
for all QEMU vms in /etc/libvirt/qemu.conf. This mirrors a similar
option XenD has. A per-VM specified password always takes priority
qemud/Makefile.am | 2
qemud/libvirtd_qemu.aug | 1
qemud/test_libvirtd.aug | 18 ++++++
qemud/test_libvirtd_qemu.aug | 33 +++++++-----
src/qemu.conf | 11 ++++
src/qemu_conf.c | 58 ++++++++++++++-------
src/qemu_conf.h | 1
src/qemu_driver.c | 116 +++++++++++++++++++++++++++++++++----------
src/uml_driver.c | 4 +
src/virsh.c | 30 +++++++----
10 files changed, 204 insertions(+), 70 deletions(-)
Daniel
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -246,6 +246,8 @@ libvirtd.init: libvirtd.init.in
check-local:
test -x '$(AUGPARSE)' \
&& '$(AUGPARSE)' -I $(srcdir) $(srcdir)/test_libvirtd.aug || :
+ test -x '$(AUGPARSE)' \
+ && '$(AUGPARSE)' -I $(srcdir) $(srcdir)/test_libvirtd_qemu.aug || :
else
diff --git a/qemud/libvirtd_qemu.aug b/qemud/libvirtd_qemu.aug
--- a/qemud/libvirtd_qemu.aug
+++ b/qemud/libvirtd_qemu.aug
@@ -26,6 +26,7 @@ module Libvirtd_qemu =
| bool_entry "vnc_tls"
| str_entry "vnc_tls_x509_cert_dir"
| bool_entry "vnc_tls_x509_verify"
+ | str_entry "vnc_password"
(* Each enty in the config is one of the following three ... *)
let entry = vnc_entry
diff --git a/qemud/test_libvirtd.aug b/qemud/test_libvirtd.aug
--- a/qemud/test_libvirtd.aug
+++ b/qemud/test_libvirtd.aug
@@ -259,6 +259,15 @@ max_requests = 20
# this should be a small fraction of the global max_requests
# and max_workers parameter
max_client_requests = 5
+
+# Logging level:
+log_level = 4
+
+# Logging outputs:
+log_outputs=\"4:stderr\"
+
+# Logging filters:
+log_filters=\"a\"
"
test Libvirtd.lns get conf =
@@ -525,3 +534,12 @@ max_client_requests = 5
{ "#comment" = "this should be a small fraction of the global
max_requests" }
{ "#comment" = "and max_workers parameter" }
{ "max_client_requests" = "5" }
+ { "#empty" }
+ { "#comment" = "Logging level:" }
+ { "log_level" = "4" }
+ { "#empty" }
+ { "#comment" = "Logging outputs:" }
+ { "log_outputs" = "4:stderr" }
+ { "#empty" }
+ { "#comment" = "Logging filters:" }
+ { "log_filters" = "a" }
diff --git a/qemud/test_libvirtd_qemu.aug b/qemud/test_libvirtd_qemu.aug
--- a/qemud/test_libvirtd_qemu.aug
+++ b/qemud/test_libvirtd_qemu.aug
@@ -50,14 +50,16 @@ vnc_tls_x509_cert_dir = \"/etc/pki/libvi
#
vnc_tls_x509_verify = 1
-# Logging level:
-log_level = 4
-# Logging outputs:
-log_outputs="4:stderr"
-
-# Logging filters:
-log_filters=""
+# The default VNC password. Only 8 letters are significant for
+# VNC passwords. This parameter is only used if the per-domain
+# XML config does not already provide a password. To allow
+# access without passwords, leave this commented out. An empty
+# string will still enable passwords, but be rejected by QEMU
+# effectively preventing any use of VNC. Obviously change this
+# example here before you set this
+#
+vnc_password = \"XYZ12345\"
"
test Libvirtd_qemu.lns get conf =
@@ -110,9 +112,14 @@ log_filters=""
{ "#comment" = "certificate signed by the CA in
/etc/pki/libvirt-vnc/ca-cert.pem" }
{ "#comment" = "" }
{ "vnc_tls_x509_verify" = "1" }
-{ "#comment" = "Logging level:" }
-{ "log_level" = "4" }
-{ "#comment" = "Logging outputs:" }
-{ "log_outputs" = "4:stderr" }
-{ "#comment" = "Logging filters" }
-{ "log_filters" = "" }
+{ "#empty" }
+{ "#empty" }
+{ "#comment" = "The default VNC password. Only 8 letters are significant
for" }
+{ "#comment" = "VNC passwords. This parameter is only used if the
per-domain" }
+{ "#comment" = "XML config does not already provide a password. To
allow" }
+{ "#comment" = "access without passwords, leave this commented out. An
empty" }
+{ "#comment" = "string will still enable passwords, but be rejected by
QEMU" }
+{ "#comment" = "effectively preventing any use of VNC. Obviously change
this" }
+{ "#comment" = "example here before you set this" }
+{ "#comment" = "" }
+{ "vnc_password" = "XYZ12345" }
diff --git a/src/qemu.conf b/src/qemu.conf
--- a/src/qemu.conf
+++ b/src/qemu.conf
@@ -47,3 +47,14 @@
# certificate signed by the CA in /etc/pki/libvirt-vnc/ca-cert.pem
#
# vnc_tls_x509_verify = 1
+
+
+# The default VNC password. Only 8 letters are significant for
+# VNC passwords. This parameter is only used if the per-domain
+# XML config does not already provide a password. To allow
+# access without passwords, leave this commented out. An empty
+# string will still enable passwords, but be rejected by QEMU
+# effectively preventing any use of VNC. Obviously change this
+# example here before you set this
+#
+# vnc_password = "XYZ12345"
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -129,6 +129,18 @@ int qemudLoadDriverConfig(struct qemud_d
}
}
+ p = virConfGetValue (conf, "vnc_password");
+ CHECK_TYPE ("vnc_password", VIR_CONF_STRING);
+ if (p && p->str) {
+ VIR_FREE(driver->vncPassword);
+ if (!(driver->vncPassword = strdup(p->str))) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY,
+ "%s", _("failed to allocate
vnc_listen"));
+ virConfFree(conf);
+ return -1;
+ }
+ }
+
virConfFree (conf);
return 0;
}
@@ -1138,37 +1150,43 @@ int qemudBuildCommandLine(virConnectPtr
if (vm->def->graphics &&
vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
- char vncdisplay[PATH_MAX];
- int ret;
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ char *optstr;
if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) {
- char options[PATH_MAX] = "";
+ if (vm->def->graphics->data.vnc.listenAddr)
+ virBufferAdd(&opt, vm->def->graphics->data.vnc.listenAddr,
-1);
+ else if (driver->vncListen)
+ virBufferAdd(&opt, driver->vncListen, -1);
+
+ virBufferVSprintf(&opt, ":%d",
+ vm->def->graphics->data.vnc.port - 5900);
+
+ if (vm->def->graphics->data.vnc.passwd ||
+ driver->vncPassword)
+ virBufferAddLit(&opt, ",password");
+
if (driver->vncTLS) {
- strcat(options, ",tls");
+ virBufferAddLit(&opt, ",tls");
if (driver->vncTLSx509verify) {
- strcat(options, ",x509verify=");
+ virBufferVSprintf(&opt, ",x509verify=%s",
+ driver->vncTLSx509certdir);
} else {
- strcat(options, ",x509=");
+ virBufferVSprintf(&opt, ",x509=%s",
+ driver->vncTLSx509certdir);
}
- strncat(options, driver->vncTLSx509certdir,
- sizeof(options) - (strlen(driver->vncTLSx509certdir)-1));
- options[sizeof(options)-1] = '\0';
}
- ret = snprintf(vncdisplay, sizeof(vncdisplay), "%s:%d%s",
- (vm->def->graphics->data.vnc.listenAddr ?
- vm->def->graphics->data.vnc.listenAddr :
- (driver->vncListen ? driver->vncListen :
"")),
- vm->def->graphics->data.vnc.port - 5900,
- options);
} else {
- ret = snprintf(vncdisplay, sizeof(vncdisplay), "%d",
- vm->def->graphics->data.vnc.port - 5900);
+ virBufferVSprintf(&opt, "%d",
+ vm->def->graphics->data.vnc.port - 5900);
}
- if (ret < 0 || ret >= (int)sizeof(vncdisplay))
- goto error;
+ if (virBufferError(&opt))
+ goto no_memory;
+
+ optstr = virBufferContentAndReset(&opt);
ADD_ARG_LIT("-vnc");
- ADD_ARG_LIT(vncdisplay);
+ ADD_ARG(optstr);
if (vm->def->graphics->data.vnc.keymap) {
ADD_ARG_LIT("-k");
ADD_ARG_LIT(vm->def->graphics->data.vnc.keymap);
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -68,6 +68,7 @@ struct qemud_driver {
unsigned int vncTLSx509verify : 1;
char *vncTLSx509certdir;
char *vncListen;
+ char *vncPassword;
virCapsPtr caps;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -74,6 +74,10 @@
/* For storing short-lived temporary files. */
#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt"
+#define QEMU_CMD_PROMPT "\n(qemu) "
+#define QEMU_PASSWD_PROMPT "Password: "
+
+
static int qemudShutdown(void);
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -138,9 +142,14 @@ static void qemudShutdownVMDaemon(virCon
static int qemudDomainGetMaxVcpus(virDomainPtr dom);
-static int qemudMonitorCommand (const virDomainObjPtr vm,
- const char *cmd,
- char **reply);
+static int qemudMonitorCommand(const virDomainObjPtr vm,
+ const char *cmd,
+ char **reply);
+static int qemudMonitorCommandExtra(const virDomainObjPtr vm,
+ const char *cmd,
+ const char *extra,
+ const char *extraPrompt,
+ char **reply);
static struct qemud_driver *qemu_driver = NULL;
@@ -583,6 +592,7 @@ qemudShutdown(void) {
VIR_FREE(qemu_driver->stateDir);
VIR_FREE(qemu_driver->vncTLSx509certdir);
VIR_FREE(qemu_driver->vncListen);
+ VIR_FREE(qemu_driver->vncPassword);
/* Free domain callback list */
virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks);
@@ -1012,6 +1022,39 @@ qemudInitCpus(virConnectPtr conn,
}
+static int
+qemudInitPasswords(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm) {
+ char *info = NULL;
+
+ /*
+ * NB: Might have more passwords to set in the future. eg a qcow
+ * disk decryption password, but there's no monitor command
+ * for that yet...
+ */
+
+ if (vm->def->graphics &&
+ vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
+ vm->def->graphics->data.vnc.passwd) {
+
+ if (qemudMonitorCommandExtra(vm, "change vnc password",
+ vm->def->graphics->data.vnc.passwd ?
+ vm->def->graphics->data.vnc.passwd :
+ driver->vncPassword,
+ QEMU_PASSWD_PROMPT,
+ &info) < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("setting VNC password
failed"));
+ return -1;
+ }
+ VIR_FREE(info);
+ }
+
+ return 0;
+}
+
+
static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
int i;
@@ -1204,7 +1247,8 @@ static int qemudStartVMDaemon(virConnect
if (ret == 0) {
if ((qemudWaitForMonitor(conn, driver, vm, pos) < 0) ||
(qemudDetectVcpuPIDs(conn, vm) < 0) ||
- (qemudInitCpus(conn, vm, migrateFrom) < 0)) {
+ (qemudInitCpus(conn, vm, migrateFrom) < 0) ||
+ (qemudInitPasswords(conn, driver, vm) < 0)) {
qemudShutdownVMDaemon(conn, driver, vm);
return -1;
}
@@ -1314,12 +1358,15 @@ cleanup:
}
static int
-qemudMonitorCommand (const virDomainObjPtr vm,
- const char *cmd,
- char **reply) {
+qemudMonitorCommandExtra(const virDomainObjPtr vm,
+ const char *cmd,
+ const char *extra,
+ const char *extraPrompt,
+ char **reply) {
int size = 0;
char *buf = NULL;
size_t cmdlen = strlen(cmd);
+ size_t extralen = extra ? strlen(extra) : 0;
if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen)
return -1;
@@ -1355,25 +1402,34 @@ qemudMonitorCommand (const virDomainObjP
}
/* Look for QEMU prompt to indicate completion */
- if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
- char *commptr = NULL, *nlptr = NULL;
-
- /* Preserve the newline */
- tmp[1] = '\0';
-
- /* The monitor doesn't dump clean output after we have written to
- * it. Every character we write dumps a bunch of useless stuff,
- * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
- * Try to throw away everything before the first full command
- * occurence, and inbetween the command and the newline starting
- * the response
- */
- if ((commptr = strstr(buf, cmd)))
- memmove(buf, commptr, strlen(commptr)+1);
- if ((nlptr = strchr(buf, '\n')))
- memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
-
- break;
+ if (buf) {
+ if (extra) {
+ if (strstr(buf, extraPrompt) != NULL) {
+ if (safewrite(vm->monitor, extra, extralen) != extralen)
+ return -1;
+ if (safewrite(vm->monitor, "\r", 1) != 1)
+ return -1;
+ extra = NULL;
+ }
+ } else if ((tmp = strstr(buf, QEMU_CMD_PROMPT)) != NULL) {
+ char *commptr = NULL, *nlptr = NULL;
+ /* Preserve the newline */
+ tmp[1] = '\0';
+
+ /* The monitor doesn't dump clean output after we have written to
+ * it. Every character we write dumps a bunch of useless stuff,
+ * so the result looks like
"cXcoXcomXcommXcommaXcommanXcommand"
+ * Try to throw away everything before the first full command
+ * occurence, and inbetween the command and the newline starting
+ * the response
+ */
+ if ((commptr = strstr(buf, cmd)))
+ memmove(buf, commptr, strlen(commptr)+1);
+ if ((nlptr = strchr(buf, '\n')))
+ memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
+
+ break;
+ }
}
pollagain:
/* Need to wait for more data */
@@ -1403,6 +1459,14 @@ qemudMonitorCommand (const virDomainObjP
return -1;
}
+static int
+qemudMonitorCommand(const virDomainObjPtr vm,
+ const char *cmd,
+ char **reply) {
+ return qemudMonitorCommandExtra(vm, cmd, NULL, NULL, reply);
+}
+
+
/**
* qemudProbe:
*
diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -324,6 +324,7 @@ umlStartup(void) {
/* Don't have a dom0 so start from 1 */
uml_driver->nextvmid = 1;
+ uml_driver->inotifyWatch = -1;
userdir = virGetUserDirectory(NULL, uid);
if (!userdir)
@@ -484,7 +485,8 @@ umlShutdown(void) {
return -1;
umlDriverLock(uml_driver);
- virEventRemoveHandle(uml_driver->inotifyWatch);
+ if (uml_driver->inotifyWatch != -1)
+ virEventRemoveHandle(uml_driver->inotifyWatch);
close(uml_driver->inotifyFD);
virCapabilitiesFree(uml_driver->caps);
diff --git a/src/virsh.c b/src/virsh.c
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -2079,6 +2079,8 @@ static const vshCmdInfo info_dumpxml[] =
static const vshCmdOptDef opts_dumpxml[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id
or uuid")},
+ {"inactive", VSH_OT_BOOL, 0, gettext_noop("show inactive defined
XML")},
+ {"secure", VSH_OT_BOOL, 0, gettext_noop("include security sensitive
data")},
{NULL, 0, 0, NULL}
};
@@ -2088,14 +2090,22 @@ cmdDumpXML(vshControl *ctl, const vshCmd
virDomainPtr dom;
int ret = TRUE;
char *dump;
-
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
- return FALSE;
-
- if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- return FALSE;
-
- dump = virDomainGetXMLDesc(dom, 0);
+ int flags = 0;
+ int inactive = vshCommandOptBool(cmd, "inactive");
+ int secure = vshCommandOptBool(cmd, "secure");
+
+ if (inactive)
+ flags |= VIR_DOMAIN_XML_INACTIVE;
+ if(secure)
+ flags |= VIR_DOMAIN_XML_SECURE;
+
+ if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ return FALSE;
+
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ return FALSE;
+
+ dump = virDomainGetXMLDesc(dom, flags);
if (dump != NULL) {
printf("%s", dump);
free(dump);
@@ -5374,7 +5384,7 @@ cmdEdit (vshControl *ctl, const vshCmd *
goto cleanup;
/* Get the XML configuration of the domain. */
- doc = virDomainGetXMLDesc (dom, 0);
+ doc = virDomainGetXMLDesc (dom, VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE);
if (!doc)
goto cleanup;
@@ -5404,7 +5414,7 @@ cmdEdit (vshControl *ctl, const vshCmd *
* it was being edited? This also catches problems such as us
* losing a connection or the domain going away.
*/
- doc_reread = virDomainGetXMLDesc (dom, 0);
+ doc_reread = virDomainGetXMLDesc (dom, VIR_DOMAIN_XML_SECURE |
VIR_DOMAIN_XML_INACTIVE);
if (!doc_reread)
goto cleanup;
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
12:46 p.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Jan 20, 2009 at 11:08:56PM +0000, Daniel P. Berrange wrote:
> This patch adds support for using the monitor interface to set the VNC
> password
>
> (qemu) change vnc password
> Password: ********
>
> A minor tricky thing is that we can't just send the command and password
> all in one go, we must wait for the 'Password' prompt before sending the
> password.
>
> When doing this I noticed that virsh dumpxml has no way to request a
> secure XML dump (required to see the password element), nor did the
> virsh edit command set the SECURE or INACTIVE flags when changing
> the XML.
>
> qemu_conf.c | 45 ++++++++++++-----------
> qemu_driver.c | 112 ++++++++++++++++++++++++++++++++++++++++++++--------------
> virsh.c | 30 ++++++++++-----
> 3 files changed, 131 insertions(+), 56 deletions(-)
...
+ int flags = 0;
+ int inactive = vshCommandOptBool(cmd, "inactive");
+ int secure = vshCommandOptBool(cmd, "secure");
+
+ if (inactive)
+ flags |= VIR_DOMAIN_XML_INACTIVE;
+ if(secure)
+ flags |= VIR_DOMAIN_XML_SECURE;
ACK.
My only reservation is that this new --secure option currently means
"also dump sensitive info" (passwords), which is sometimes
_in_secure. So how about naming it --all instead?
4:03 p.m.
On Wed, Jan 28, 2009 at 07:46:34PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Tue, Jan 20, 2009 at 11:08:56PM +0000, Daniel P. Berrange wrote:
>> This patch adds support for using the monitor interface to set the VNC
>> password
>>
>> (qemu) change vnc password
>> Password: ********
>>
>> A minor tricky thing is that we can't just send the command and password
>> all in one go, we must wait for the 'Password' prompt before sending
the
>> password.
>>
>> When doing this I noticed that virsh dumpxml has no way to request a
>> secure XML dump (required to see the password element), nor did the
>> virsh edit command set the SECURE or INACTIVE flags when changing
>> the XML.
>>
>> qemu_conf.c | 45 ++++++++++++-----------
>> qemu_driver.c | 112
++++++++++++++++++++++++++++++++++++++++++++--------------
>> virsh.c | 30 ++++++++++-----
>> 3 files changed, 131 insertions(+), 56 deletions(-)
...
> + int flags = 0;
> + int inactive = vshCommandOptBool(cmd, "inactive");
> + int secure = vshCommandOptBool(cmd, "secure");
> +
> + if (inactive)
> + flags |= VIR_DOMAIN_XML_INACTIVE;
> + if(secure)
> + flags |= VIR_DOMAIN_XML_SECURE;
ACK.
My only reservation is that this new --secure option currently means
"also dump sensitive info" (passwords), which is sometimes
_in_secure. So how about naming it --all instead?
How about --security-info ? I think --all is probably a little too
generic a term
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:22 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Wed, Jan 28, 2009 at 07:46:34PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> > On Tue, Jan 20, 2009 at 11:08:56PM +0000, Daniel P. Berrange wrote:
> >> This patch adds support for using the monitor interface to set the VNC
> >> password
> >>
> >> (qemu) change vnc password
> >> Password: ********
> >>
> >> A minor tricky thing is that we can't just send the command and
password
> >> all in one go, we must wait for the 'Password' prompt before sending
the
> >> password.
> >>
> >> When doing this I noticed that virsh dumpxml has no way to request a
> >> secure XML dump (required to see the password element), nor did the
> >> virsh edit command set the SECURE or INACTIVE flags when changing
> >> the XML.
> >>
> >> qemu_conf.c | 45 ++++++++++++-----------
> >> qemu_driver.c | 112
++++++++++++++++++++++++++++++++++++++++++++--------------
> >> virsh.c | 30 ++++++++++-----
> >> 3 files changed, 131 insertions(+), 56 deletions(-)
> ...
> > + int flags = 0;
> > + int inactive = vshCommandOptBool(cmd, "inactive");
> > + int secure = vshCommandOptBool(cmd, "secure");
> > +
> > + if (inactive)
> > + flags |= VIR_DOMAIN_XML_INACTIVE;
> > + if(secure)
> > + flags |= VIR_DOMAIN_XML_SECURE;
>
> ACK.
> My only reservation is that this new --secure option currently means
> "also dump sensitive info" (passwords), which is sometimes
> _in_secure. So how about naming it --all instead?
How about --security-info ? I think --all is probably a little too
generic a term
Sounds good.
5811
days inactive
5820
days old
13 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Guido Günther -
Jim Meyering -
Radek Hladik