Re: [Qemu-devel] [PATCH] xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC

Re: [Qemu-devel] [PATCH] xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC

[Stefano Stabellini]

On Wed, 20 Apr 2011, ZhouPeng wrote:
> This pacth allows you to use vncpasswd for xen-upstream-qemu
> 
> Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>
> 
> xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC
> 

This patch is for upstream qemu, right?
In that case you always need to CC qemu-devel@nongnu.org.
Also when submitting patches for upstream qemu we need to try to reuse
the existing infrastructure to do things.
In this particular example, we cannot use xenstore to communicate the
password to qemu, we have to use QMP that is the RPC mechanism exported
by Qemu. Unfortunately libxl doesn't speak QMP yet, but adding QMP
support to libxl is one of the next things that have to be done anyway.

Re: [Qemu-devel] [PATCH] xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC

[ZhouPeng]

[-- Attachment #1: Type: text/plain, Size: 7537 bytes --]

Thank you for your review.

Does this means the xen-upsteam-qemu will give up xenstore?

Many features of xen except for vncpasswd
may need that xl send||receive msg to||from xen-qemu.
I think some other part of xen may need to read xenstore which may be
updated by qemu.
xenstore's sharing characteristic give much convenience for many-way
msg communication,
but the RPC may only give single-way.

So I am somewhat in doubt xen-upstream-qemu will give up xenstore.

IMO, I don't think qemu may accept xenstore's code only used by xen,

except
qemu need to add a new general shared many-way msg exchange mechanism,
like xenstore database.

Thank you Stefano Stabellini.

send the patch again to CC to qemu-devel@nongnu.org
=================================================================
This pacth allows you to use vncpasswd for xen-upstream-qemu

Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>

xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC

diff --git a/Makefile.objs b/Makefile.objs
index f8cf199..2a012bf 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -157,6 +157,8 @@ common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/,
$(slirp-obj-y))
 common-obj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o
 common-obj-$(CONFIG_XEN) += xen_console.o xenfb.o xen_disk.o xen_nic.o

+common-obj-$(CONFIG_XEN) += xenstore.o
+
 ######################################################################
 # libuser

diff --git a/vl.c b/vl.c
index 60ec6de..b519690 100644
--- a/vl.c
+++ b/vl.c
@@ -163,6 +163,8 @@ int main(int argc, char **argv)

 #include "ui/qemu-spice.h"

+#include "xenstore.h"
+
 //#define DEBUG_NET
 //#define DEBUG_SLIRP

@@ -3321,7 +3323,11 @@ int main(int argc, char **argv, char **envp)
 #ifdef CONFIG_VNC
     /* init remote displays */
     if (vnc_display) {
+        char password[20];
         vnc_display_init(ds);
+        xenstore_read_vncpasswd(xen_domid, password, sizeof(password));
+        vnc_display_password(ds, password);
+
         if (vnc_display_open(ds, vnc_display) < 0)
             exit(1);

diff --git a/xenstore.c b/xenstore.c
new file mode 100644
index 0000000..0bb2a55
--- /dev/null
+++ b/xenstore.c
@@ -0,0 +1,154 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License.  See the file "COPYING" in the main directory of
+ * this archive for more details.
+ *
+ * Copyright (C) 2006 Christian Limpach
+ * Copyright (C) 2006 XenSource Ltd.
+ * Copyright (C) 2011 Zhou Peng <zhoupeng@nfs.iscas.ac.cn>.
+ *
+ */
+
+#include "xenstore.h"
+#include <xs.h>
+
+struct xs_handle *xsh = NULL;
+static FILE *logfile = NULL;
+extern uint32_t xen_domid;
+
+static int pasprintf(char **buf, const char *fmt, ...)
+{
+    va_list ap;
+    int ret = 0;
+
+    if (*buf)
+        free(*buf);
+    va_start(ap, fmt);
+    if (vasprintf(buf, fmt, ap) == -1) {
+        buf = NULL;
+        ret = -1;
+    }
+    va_end(ap);
+    return ret;
+}
+
+static const char *xenstore_get_guest_uuid(void)
+{
+    static char *already_computed = NULL;
+
+    char *domain_path = NULL, *vm_path = NULL, *vm_value = NULL, *p = NULL;
+    unsigned int len;
+
+    if (already_computed)
+        return already_computed;
+
+    if (xsh == NULL)
+        return NULL;
+
+    domain_path = xs_get_domain_path(xsh, xen_domid);
+    if (domain_path == NULL) {
+        fprintf(logfile, "xs_get_domain_path() error. xen_domid
%d.\n", xen_domid);
+        goto out;
+    }
+
+    if (pasprintf(&vm_path, "%s/vm", domain_path) == -1) {
+        fprintf(logfile, "xenstore_get_guest_uuid(): out of memory.\n");
+        goto out;
+    }
+    vm_value = xs_read(xsh, XBT_NULL, vm_path, &len);
+    if (vm_value == NULL) {
+        fprintf(logfile, "xs_read(): uuid get error. %s.\n", vm_path);
+        goto out;
+    }
+
+    if (strtok(vm_value, "/") == NULL) {
+        fprintf(logfile, "failed to parse guest uuid\n");
+        goto out;
+    }
+    p = strtok(NULL, "/");
+    if (p == NULL) {
+        fprintf(logfile, "failed to parse guest uuid\n");
+        goto out;
+    }
+
+    if (pasprintf(&already_computed, "%s", p) == -1) {
+        fprintf(logfile, "xenstore_get_guest_uuid(): out of memory.\n");
+        goto out;
+    }
+
+    fprintf(logfile, "Guest uuid = %s\n", already_computed);
+
+ out:
+    free(domain_path);
+    free(vm_path);
+    free(vm_value);
+
+    return already_computed;
+}
+
+void xenstore_read_vncpasswd(int domid, char *pwbuf, size_t pwbuflen)
+{
+    char *buf = NULL, *path, *uuid = NULL, *passwd = NULL;
+    unsigned int i, len;
+
+    pwbuf[0] = '\0';
+
+    if (xsh == NULL)
+        return;
+
+    path = xs_get_domain_path(xsh, domid);
+    if (path == NULL) {
+        fprintf(logfile, "xs_get_domain_path() error. domid %d.\n", domid);
+        return;
+    }
+
+    pasprintf(&buf, "%s/vm", path);
+    free(path);
+    uuid = xs_read(xsh, XBT_NULL, buf, &len);
+    if (uuid == NULL) {
+        fprintf(logfile, "xs_read(): uuid get error. %s.\n", buf);
+        free(buf);
+        return;
+    }
+
+    pasprintf(&buf, "%s/vncpasswd", uuid);
+    free(uuid);
+    passwd = xs_read(xsh, XBT_NULL, buf, &len);
+    if (passwd == NULL) {
+        fprintf(logfile, "xs_read(): vncpasswd get error. %s.\n", buf);
+        free(buf);
+        return;
+    }
+
+    if (len >= pwbuflen)
+    {
+        fprintf(logfile,
+                "xenstore_read_vncpasswd(): truncated password to
avoid buffer overflow\n");
+        len = pwbuflen - 1;
+    }
+
+    for (i=0; i<len; i++)
+        pwbuf[i] = passwd[i];
+    pwbuf[len] = '\0';
+    passwd[0] = '\0';
+    if (xs_write(xsh, XBT_NULL, buf, passwd, 1) == 0)
+        fprintf(logfile, "xs_write() vncpasswd failed.\n");
+
+    free(passwd);
+    free(buf);
+}
+
+static void xenstore_init(void)
+{
+    logfile = stderr;
+
+    xenstore_get_guest_uuid();
+
+    xsh = xs_daemon_open();
+    if (xsh == NULL) {
+        fprintf(logfile, "Could not contact xenstore for domain config\n");
+        return;
+    }
+}
+
+device_init(xenstore_init);
diff --git a/xenstore.h b/xenstore.h
new file mode 100644
index 0000000..5242f2c
--- /dev/null
+++ b/xenstore.h
@@ -0,0 +1,15 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License.  See the file "COPYING" in the main directory of
+ * this archive for more details.
+ *
+ */
+
+#ifndef XEN_STORE_H
+#define XEN_STORE_H
+
+#include "qemu-common.h"
+
+void xenstore_read_vncpasswd(int domid, char *pwbuf, size_t pwbuflen);
+
+#endif /* XEN_STORE_H */

2011/4/20 Stefano Stabellini <stefano.stabellini@eu.citrix.com>:
> On Wed, 20 Apr 2011, ZhouPeng wrote:
>> This pacth allows you to use vncpasswd for xen-upstream-qemu
>>
>> Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>
>>
>> xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC
>>
>
> This patch is for upstream qemu, right?

Yes, it's for xen-qemu-0.14.50 from Anthony's tree, very close to the
upstream qemu.

> In that case you always need to CC qemu-devel@nongnu.org.
> Also when submitting patches for upstream qemu we need to try to reuse
> the existing infrastructure to do things.
> In this particular example, we cannot use xenstore to communicate the
> password to qemu, we have to use QMP that is the RPC mechanism exported
> by Qemu. Unfortunately libxl doesn't speak QMP yet, but adding QMP
> support to libxl is one of the next things that have to be done anyway.
>
>

-- 
Zhou Peng
Operating System Technology Group
Institute of Software, the Chinese Academy of Sciences (ISCAS)

[-- Attachment #2: xen-upstream-qemu-vncpassword-by-xenstore.diff --]
[-- Type: text/x-patch, Size: 5733 bytes --]

This pacth allows you to use vncpasswd for xen-upstream-qemu

Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>

xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC

diff --git a/Makefile.objs b/Makefile.objs
index f8cf199..2a012bf 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -157,6 +157,8 @@ common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
 common-obj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o
 common-obj-$(CONFIG_XEN) += xen_console.o xenfb.o xen_disk.o xen_nic.o
 
+common-obj-$(CONFIG_XEN) += xenstore.o
+
 ######################################################################
 # libuser
 
diff --git a/vl.c b/vl.c
index 60ec6de..b519690 100644
--- a/vl.c
+++ b/vl.c
@@ -163,6 +163,8 @@ int main(int argc, char **argv)
 
 #include "ui/qemu-spice.h"
 
+#include "xenstore.h"
+
 //#define DEBUG_NET
 //#define DEBUG_SLIRP
 
@@ -3321,7 +3323,11 @@ int main(int argc, char **argv, char **envp)
 #ifdef CONFIG_VNC
     /* init remote displays */
     if (vnc_display) {
+        char password[20];
         vnc_display_init(ds);
+        xenstore_read_vncpasswd(xen_domid, password, sizeof(password));
+        vnc_display_password(ds, password);
+
         if (vnc_display_open(ds, vnc_display) < 0)
             exit(1);
 
diff --git a/xenstore.c b/xenstore.c
new file mode 100644
index 0000000..0bb2a55
--- /dev/null
+++ b/xenstore.c
@@ -0,0 +1,154 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License.  See the file "COPYING" in the main directory of
+ * this archive for more details.
+ *
+ * Copyright (C) 2006 Christian Limpach
+ * Copyright (C) 2006 XenSource Ltd.
+ * Copyright (C) 2011 Zhou Peng <zhoupeng@nfs.iscas.ac.cn>.
+ *
+ */
+
+#include "xenstore.h"
+#include <xs.h>
+
+struct xs_handle *xsh = NULL;
+static FILE *logfile = NULL;
+extern uint32_t xen_domid;
+
+static int pasprintf(char **buf, const char *fmt, ...)
+{
+    va_list ap;
+    int ret = 0;
+
+    if (*buf)
+        free(*buf);
+    va_start(ap, fmt);
+    if (vasprintf(buf, fmt, ap) == -1) {
+        buf = NULL;
+        ret = -1;
+    }
+    va_end(ap);
+    return ret;
+}
+
+static const char *xenstore_get_guest_uuid(void)
+{
+    static char *already_computed = NULL;
+
+    char *domain_path = NULL, *vm_path = NULL, *vm_value = NULL, *p = NULL;
+    unsigned int len;
+
+    if (already_computed)
+        return already_computed;
+
+    if (xsh == NULL)
+        return NULL;
+
+    domain_path = xs_get_domain_path(xsh, xen_domid);
+    if (domain_path == NULL) {
+        fprintf(logfile, "xs_get_domain_path() error. xen_domid %d.\n", xen_domid);
+        goto out;
+    }
+
+    if (pasprintf(&vm_path, "%s/vm", domain_path) == -1) {
+        fprintf(logfile, "xenstore_get_guest_uuid(): out of memory.\n");
+        goto out;
+    }
+    vm_value = xs_read(xsh, XBT_NULL, vm_path, &len);
+    if (vm_value == NULL) {
+        fprintf(logfile, "xs_read(): uuid get error. %s.\n", vm_path);
+        goto out;
+    }
+
+    if (strtok(vm_value, "/") == NULL) {
+        fprintf(logfile, "failed to parse guest uuid\n");
+        goto out;
+    }
+    p = strtok(NULL, "/");
+    if (p == NULL) {
+        fprintf(logfile, "failed to parse guest uuid\n");
+        goto out;
+    }
+
+    if (pasprintf(&already_computed, "%s", p) == -1) {
+        fprintf(logfile, "xenstore_get_guest_uuid(): out of memory.\n");
+        goto out;
+    }
+
+    fprintf(logfile, "Guest uuid = %s\n", already_computed);
+
+ out:
+    free(domain_path);
+    free(vm_path);
+    free(vm_value);
+
+    return already_computed;
+}
+
+void xenstore_read_vncpasswd(int domid, char *pwbuf, size_t pwbuflen)
+{
+    char *buf = NULL, *path, *uuid = NULL, *passwd = NULL;
+    unsigned int i, len;
+
+    pwbuf[0] = '\0';
+
+    if (xsh == NULL)
+        return;
+
+    path = xs_get_domain_path(xsh, domid);
+    if (path == NULL) {
+        fprintf(logfile, "xs_get_domain_path() error. domid %d.\n", domid);
+        return;
+    }
+
+    pasprintf(&buf, "%s/vm", path);
+    free(path);
+    uuid = xs_read(xsh, XBT_NULL, buf, &len);
+    if (uuid == NULL) {
+        fprintf(logfile, "xs_read(): uuid get error. %s.\n", buf);
+        free(buf);
+        return;
+    }
+
+    pasprintf(&buf, "%s/vncpasswd", uuid);
+    free(uuid);
+    passwd = xs_read(xsh, XBT_NULL, buf, &len);
+    if (passwd == NULL) {
+        fprintf(logfile, "xs_read(): vncpasswd get error. %s.\n", buf);
+        free(buf);
+        return;
+    }
+
+    if (len >= pwbuflen)
+    {
+        fprintf(logfile, 
+                "xenstore_read_vncpasswd(): truncated password to avoid buffer overflow\n");
+        len = pwbuflen - 1;
+    }
+
+    for (i=0; i<len; i++)
+        pwbuf[i] = passwd[i];
+    pwbuf[len] = '\0';
+    passwd[0] = '\0';
+    if (xs_write(xsh, XBT_NULL, buf, passwd, 1) == 0)
+        fprintf(logfile, "xs_write() vncpasswd failed.\n");
+
+    free(passwd);
+    free(buf);
+}
+
+static void xenstore_init(void)
+{
+    logfile = stderr;
+
+    xenstore_get_guest_uuid();
+
+    xsh = xs_daemon_open();
+    if (xsh == NULL) {
+        fprintf(logfile, "Could not contact xenstore for domain config\n");
+        return;
+    }
+}
+
+device_init(xenstore_init);
diff --git a/xenstore.h b/xenstore.h
new file mode 100644
index 0000000..5242f2c
--- /dev/null
+++ b/xenstore.h
@@ -0,0 +1,15 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License.  See the file "COPYING" in the main directory of
+ * this archive for more details.
+ *
+ */
+
+#ifndef XEN_STORE_H
+#define XEN_STORE_H
+
+#include "qemu-common.h"
+
+void xenstore_read_vncpasswd(int domid, char *pwbuf, size_t pwbuflen);
+
+#endif /* XEN_STORE_H */

Re: [Qemu-devel] [PATCH] xen-upstream-qemu: get vncpassword through xenstore, enable VNC_AUTH_VNC

[Stefano Stabellini]

On Thu, 21 Apr 2011, ZhouPeng wrote:
> Thank you for your review.
> 
> Does this means the xen-upsteam-qemu will give up xenstore?
> 
> Many features of xen except for vncpasswd
> may need that xl send||receive msg to||from xen-qemu.
> I think some other part of xen may need to read xenstore which may be
> updated by qemu.
> xenstore's sharing characteristic give much convenience for many-way
> msg communication,
> but the RPC may only give single-way.
> 
> So I am somewhat in doubt xen-upstream-qemu will give up xenstore.
> 
> IMO, I don't think qemu may accept xenstore's code only used by xen,
> 
> except
> qemu need to add a new general shared many-way msg exchange mechanism,
> like xenstore database.
> 

Even though a "QMP bus" could be useful, I don't think there is anything
like that at the moment.
Also considering that each qemu instance takes care of a single guest,
the usefulness of such a mechanism is reduced.

Anyhow in this particular case using QMP seems appropriate: I would
welcome patches to add QMP support to libxenlight, even though I realize
that it is a bigger effort than this single patch.