Support VNC password setting in QEMU driver

ChangeLog

@@ -1,3 +1,22 @@
+Thu Jan 29 17:40:22 GMT 2009 Daniel P. Berrange <berrange@redhat.com>
+
+	Support VNC password setting for QEMU driver
+	* qemud/Makefile.am: Add missing test of libvirt_qemud.aug file
+	* qemud/libvirtd_qemu.aug: Add suport for VNC password config
+	* qemud/test_libvirtd.aug: Add logging params test
+	* qemud/test_libvirtd_qemu.aug: Remove bogus logging params,
+	and add VNC password test
+	* src/qemu.conf: Include example VNC password config
+	* src/qemu_conf.c, src/qemu_conf.h, src/qemu_driver.c: Support
+	setting a VNC password on a per-VM basis, or from QEMU driver
+	global config file.
+	* src/uml_driver.c: Fix initialization of inotifyWatch param
+	to avoid bogus watch unregister later
+	* src/virsh.c: Add --security-info and --inative flags to
+	dumpxml command. Ensure edit command uses SECURE & INACTIVE
+	flags when changing config
+
+
 Thu Jan 29 17:24:22 GMT 2009 Daniel P. Berrange <berrange@redhat.com>
 
 	Fix save/restore for new KVM releases

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
 

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

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" }

qemud/test_libvirtd_qemu.aug

@@ -50,14 +50,16 @@ vnc_tls_x509_cert_dir = \"/etc/pki/libvirt-vnc\"
 #
 vnc_tls_x509_verify = 1
 
-# Logging level:
-log_level = 4
 
-# Logging outputs:
+# The default VNC password. Only 8 letters are significant for
-log_outputs="4:stderr"
+# VNC passwords. This parameter is only used if the per-domain
-
+# XML config does not already provide a password. To allow
-# Logging filters:
+# access without passwords, leave this commented out. An empty
-log_filters=""
+# 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:" }
+{ "#empty" }
-{ "log_level" = "4" }
+{ "#empty" }
-{ "#comment" = "Logging outputs:" }
+{ "#comment" = "The default VNC password. Only 8 letters are significant for" }
-{ "log_outputs" = "4:stderr" }
+{ "#comment" = "VNC passwords. This parameter is only used if the per-domain" }
-{ "#comment" = "Logging filters" }
+{ "#comment" = "XML config does not already provide a password. To allow" }
-{ "log_filters" = "" }
+{ "#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" }

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"

src/qemu_conf.c

@@ -125,6 +125,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
         }
     }
 
+    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))) {
+            virReportOOMError(NULL);
+            virConfFree(conf);
+            return -1;
+        }
+    }
+
     virConfFree (conf);
     return 0;
 }
@@ -1196,37 +1207,43 @@ int qemudBuildCommandLine(virConnectPtr conn,
 
     if (vm->def->graphics &&
         vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
-        char vncdisplay[PATH_MAX];
+        virBuffer opt = VIR_BUFFER_INITIALIZER;
-        int ret;
+        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",
+            virBufferVSprintf(&opt, "%d",
-                           vm->def->graphics->data.vnc.port - 5900);
+                              vm->def->graphics->data.vnc.port - 5900);
         }
-        if (ret < 0 || ret >= (int)sizeof(vncdisplay))
+        if (virBufferError(&opt))
-            goto error;
+            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);

src/qemu_conf.h

@@ -73,6 +73,7 @@ struct qemud_driver {
     unsigned int vncTLSx509verify : 1;
     char *vncTLSx509certdir;
     char *vncListen;
+    char *vncPassword;
 
     virCapsPtr caps;
 

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)
@@ -139,9 +143,14 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
 
 static int qemudDomainGetMaxVcpus(virDomainPtr dom);
 
-static int qemudMonitorCommand (const virDomainObjPtr vm,
+static int qemudMonitorCommand(const virDomainObjPtr vm,
-                                const char *cmd,
+                               const char *cmd,
-                                char **reply);
+                               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);
@@ -1009,6 +1019,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;
 
@@ -1202,7 +1245,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     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;
         }
@@ -1312,12 +1356,15 @@ cleanup:
 }
 
 static int
-qemudMonitorCommand (const virDomainObjPtr vm,
+qemudMonitorCommandExtra(const virDomainObjPtr vm,
-                     const char *cmd,
+                         const char *cmd,
-                     char **reply) {
+                         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;
@@ -1353,25 +1400,34 @@ qemudMonitorCommand (const virDomainObjPtr vm,
         }
 
         /* Look for QEMU prompt to indicate completion */
-        if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
+        if (buf) {
-            char *commptr = NULL, *nlptr = NULL;
+            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';
 
-            /* Preserve the newline */
+                /* The monitor doesn't dump clean output after we have written to
-            tmp[1] = '\0';
+                 * 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);
 
-            /* The monitor doesn't dump clean output after we have written to
+                break;
-             * 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 */
@@ -1401,6 +1457,14 @@ qemudMonitorCommand (const virDomainObjPtr vm,
     return -1;
 }
 
+static int
+qemudMonitorCommand(const virDomainObjPtr vm,
+                    const char *cmd,
+                    char **reply) {
+    return qemudMonitorCommandExtra(vm, cmd, NULL, NULL, reply);
+}
+
+
 /**
  * qemudProbe:
  *

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);
 

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")},
+    {"security-info", VSH_OT_BOOL, 0, gettext_noop("include security sensitive information in XML dump")},
     {NULL, 0, 0, NULL}
 };
 
@@ -2088,6 +2090,14 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
     virDomainPtr dom;
     int ret = TRUE;
     char *dump;
+    int flags = 0;
+    int inactive = vshCommandOptBool(cmd, "inactive");
+    int secure = vshCommandOptBool(cmd, "security-info");
+
+    if (inactive)
+        flags |= VIR_DOMAIN_XML_INACTIVE;
+    if (secure)
+        flags |= VIR_DOMAIN_XML_SECURE;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         return FALSE;
@@ -2095,7 +2105,7 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return FALSE;
 
-    dump = virDomainGetXMLDesc(dom, 0);
+    dump = virDomainGetXMLDesc(dom, flags);
     if (dump != NULL) {
         printf("%s", dump);
         free(dump);
@@ -5374,7 +5384,7 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
         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 *cmd)
      * 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;