From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53178 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Py3VU-0005g2-4T for qemu-devel@nongnu.org; Fri, 11 Mar 2011 09:40:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Py3VO-0000Cv-V8 for qemu-devel@nongnu.org; Fri, 11 Mar 2011 09:39:52 -0500 Received: from mail-yw0-f45.google.com ([209.85.213.45]:33937) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Py3VO-0000Cn-PG for qemu-devel@nongnu.org; Fri, 11 Mar 2011 09:39:50 -0500 Received: by ywl41 with SMTP id 41so1426129ywl.4 for ; Fri, 11 Mar 2011 06:39:50 -0800 (PST) Message-ID: <4D7A3433.2060006@codemonkey.ws> Date: Fri, 11 Mar 2011 08:39:47 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1299854234-22003-1-git-send-email-Jes.Sorensen@redhat.com> In-Reply-To: <1299854234-22003-1-git-send-email-Jes.Sorensen@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v2] Make VNC support optional List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes.Sorensen@redhat.com Cc: qemu-devel@nongnu.org On 03/11/2011 08:37 AM, Jes.Sorensen@redhat.com wrote: > From: Jes Sorensen > > Per default VNC is enabled. > > Signed-off-by: Jes Sorensen > --- > Makefile.objs | 19 ++++++++++--------- > configure | 37 +++++++++++++++++++++++++------------ > console.h | 26 ++++++++++++++++++++++++-- > monitor.c | 22 ++++++++++------------ > qerror.h | 3 +++ > ui/vnc.c | 14 ++++++++++---- > vl.c | 10 +++++++++- > 7 files changed, 91 insertions(+), 40 deletions(-) > > diff --git a/Makefile.objs b/Makefile.objs > index 9e98a66..58388e2 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -127,19 +127,20 @@ common-obj-y += $(addprefix audio/, $(audio-obj-y)) > ui-obj-y += keymaps.o > ui-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o > ui-obj-$(CONFIG_CURSES) += curses.o > -ui-obj-y += vnc.o d3des.o > -ui-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o > -ui-obj-y += vnc-enc-tight.o vnc-palette.o > -ui-obj-y += vnc-enc-zrle.o > -ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o > -ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o > -ui-obj-$(CONFIG_COCOA) += cocoa.o > +vnc-obj-y += vnc.o d3des.o > +vnc-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o > +vnc-obj-y += vnc-enc-tight.o vnc-palette.o > +vnc-obj-y += vnc-enc-zrle.o > +vnc-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o > +vnc-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o > +vnc-obj-$(CONFIG_COCOA) += cocoa.o > ifdef CONFIG_VNC_THREAD > -ui-obj-y += vnc-jobs-async.o > +vnc-obj-y += vnc-jobs-async.o > else > -ui-obj-y += vnc-jobs-sync.o > +vnc-obj-y += vnc-jobs-sync.o > endif > common-obj-y += $(addprefix ui/, $(ui-obj-y)) > +common-obj-$(CONFIG_VNC) += $(addprefix ui/, $(vnc-obj-y)) > > common-obj-y += iov.o acl.o > common-obj-$(CONFIG_THREAD) += qemu-thread.o > diff --git a/configure b/configure > index 39cdf2b..a64b750 100755 > --- a/configure > +++ b/configure > @@ -117,6 +117,7 @@ kvm="" > kvm_para="" > nptl="" > sdl="" > +vnc="yes" > sparse="no" > uuid="" > vde="" > @@ -539,6 +540,10 @@ for opt do > ;; > --enable-sdl) sdl="yes" > ;; > + --disable-vnc) vnc="no" > + ;; > + --enable-vnc) vnc="yes" > + ;; > --fmod-lib=*) fmod_lib="$optarg" > ;; > --fmod-inc=*) fmod_inc="$optarg" > @@ -836,6 +841,8 @@ echo " --disable-strip disable stripping binaries" > echo " --disable-werror disable compilation abort on warning" > echo " --disable-sdl disable SDL" > echo " --enable-sdl enable SDL" > +echo " --disable-vnc disable VNC" > +echo " --enable-vnc enable VNC" > echo " --enable-cocoa enable COCOA (Mac OS X only)" > echo " --audio-drv-list=LIST set audio drivers list:" > echo " Available drivers: $audio_possible_drivers" > @@ -1273,7 +1280,7 @@ fi > > ########################################## > # VNC TLS detection > -if test "$vnc_tls" != "no" ; then > +if test "$vnc" = "yes" -a "$vnc_tls" != "no" ; then > cat> $TMPC< #include > int main(void) { gnutls_session_t s; gnutls_init(&s, GNUTLS_SERVER); return 0; } > @@ -1293,7 +1300,7 @@ fi > > ########################################## > # VNC SASL detection > -if test "$vnc_sasl" != "no" ; then > +if test "$vnc" = "yes" -a "$vnc_sasl" != "no" ; then > cat> $TMPC< #include > #include > @@ -1315,7 +1322,7 @@ fi > > ########################################## > # VNC JPEG detection > -if test "$vnc_jpeg" != "no" ; then > +if test "$vnc" = "yes" -a "$vnc_jpeg" != "no" ; then > cat> $TMPC< #include > #include > @@ -1336,7 +1343,7 @@ fi > > ########################################## > # VNC PNG detection > -if test "$vnc_png" != "no" ; then > +if test "$vnc" = "yes" -a "$vnc_png" != "no" ; then > cat> $TMPC< //#include > #include > @@ -2495,11 +2502,14 @@ echo "Audio drivers $audio_drv_list" > echo "Extra audio cards $audio_card_list" > echo "Block whitelist $block_drv_whitelist" > echo "Mixer emulation $mixemu" > -echo "VNC TLS support $vnc_tls" > -echo "VNC SASL support $vnc_sasl" > -echo "VNC JPEG support $vnc_jpeg" > -echo "VNC PNG support $vnc_png" > -echo "VNC thread $vnc_thread" > +echo "VNC support $vnc" > +if test "$vnc" = "yes" ; then > + echo "VNC TLS support $vnc_tls" > + echo "VNC SASL support $vnc_sasl" > + echo "VNC JPEG support $vnc_jpeg" > + echo "VNC PNG support $vnc_png" > + echo "VNC thread $vnc_thread" > +fi > if test -n "$sparc_cpu"; then > echo "Target Sparc Arch $sparc_cpu" > fi > @@ -2649,6 +2659,9 @@ echo "CONFIG_BDRV_WHITELIST=$block_drv_whitelist">> $config_host_mak > if test "$mixemu" = "yes" ; then > echo "CONFIG_MIXEMU=y">> $config_host_mak > fi > +if test "$vnc" = "yes" ; then > + echo "CONFIG_VNC=y">> $config_host_mak > +fi > if test "$vnc_tls" = "yes" ; then > echo "CONFIG_VNC_TLS=y">> $config_host_mak > echo "VNC_TLS_CFLAGS=$vnc_tls_cflags">> $config_host_mak > @@ -2657,15 +2670,15 @@ if test "$vnc_sasl" = "yes" ; then > echo "CONFIG_VNC_SASL=y">> $config_host_mak > echo "VNC_SASL_CFLAGS=$vnc_sasl_cflags">> $config_host_mak > fi > -if test "$vnc_jpeg" != "no" ; then > +if test "$vnc_jpeg" = "yes" ; then > echo "CONFIG_VNC_JPEG=y">> $config_host_mak > echo "VNC_JPEG_CFLAGS=$vnc_jpeg_cflags">> $config_host_mak > fi > -if test "$vnc_png" != "no" ; then > +if test "$vnc_png" = "yes" ; then > echo "CONFIG_VNC_PNG=y">> $config_host_mak > echo "VNC_PNG_CFLAGS=$vnc_png_cflags">> $config_host_mak > fi > -if test "$vnc_thread" != "no" ; then > +if test "$vnc_thread" = "yes" ; then > echo "CONFIG_VNC_THREAD=y">> $config_host_mak > echo "CONFIG_THREAD=y">> $config_host_mak > fi > diff --git a/console.h b/console.h > index 87d3271..50942bb 100644 > --- a/console.h > +++ b/console.h > @@ -4,6 +4,8 @@ > #include "qemu-char.h" > #include "qdict.h" > #include "notify.h" > +#include "qerror.h" > +#include "monitor.h" > > /* keyboard/mouse support */ > > @@ -371,12 +373,32 @@ void cocoa_display_init(DisplayState *ds, int full_screen); > void vnc_display_init(DisplayState *ds); > void vnc_display_close(DisplayState *ds); > int vnc_display_open(DisplayState *ds, const char *display); > -int vnc_display_password(DisplayState *ds, const char *password); > int vnc_display_disable_login(DisplayState *ds); > +char *vnc_display_local_addr(DisplayState *ds); > +#ifdef CONFIG_VNC > +int vnc_display_password(DisplayState *ds, const char *password); > int vnc_display_pw_expire(DisplayState *ds, time_t expires); > void do_info_vnc_print(Monitor *mon, const QObject *data); > void do_info_vnc(Monitor *mon, QObject **ret_data); > -char *vnc_display_local_addr(DisplayState *ds); > +#else > +static inline int vnc_display_password(DisplayState *ds, const char *password) > +{ > + qerror_report(QERR_FEATURE_DISABLED, "vnc"); > + return -ENODEV; > +} > +static inline int vnc_display_pw_expire(DisplayState *ds, time_t expires) > +{ > + qerror_report(QERR_FEATURE_DISABLED, "vnc"); > + return -ENODEV; > +}; > +static inline void do_info_vnc(Monitor *mon, QObject **ret_data) > +{ > +}; > +static inline void do_info_vnc_print(Monitor *mon, const QObject *data) > +{ > + monitor_printf(mon, "VNC support disabled\n"); > +}; > +#endif > > /* curses.c */ > void curses_display_init(DisplayState *ds, int full_screen); > diff --git a/monitor.c b/monitor.c > index 22ae3bb..4a54c55 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1016,6 +1016,7 @@ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data) > return 0; > } > > +#ifdef CONFIG_VNC > static int change_vnc_password(const char *password) > { > if (!password || !password[0]) { > @@ -1062,6 +1063,13 @@ static int do_change_vnc(Monitor *mon, const char *target, const char *arg) > > return 0; > } > +#else > +static int do_change_vnc(Monitor *mon, const char *target, const char *arg) > +{ > + qerror_report(QERR_FEATURE_DISABLED, "vnc"); > + return -ENODEV; > +} > +#endif > > /** > * do_change(): Change a removable medium, or VNC configuration > @@ -1127,12 +1135,7 @@ static int set_password(Monitor *mon, const QDict *qdict, QObject **ret_data) > } > /* Note that setting an empty password will not disable login through > * this interface. */ > - rc = vnc_display_password(NULL, password); > - if (rc != 0) { > - qerror_report(QERR_SET_PASSWD_FAILED); > - return -1; > - } > - return 0; > + return vnc_display_password(NULL, password); > } > > qerror_report(QERR_INVALID_PARAMETER, "protocol"); > @@ -1171,12 +1174,7 @@ static int expire_password(Monitor *mon, const QDict *qdict, QObject **ret_data) > } > > if (strcmp(protocol, "vnc") == 0) { > - rc = vnc_display_pw_expire(NULL, when); > - if (rc != 0) { > - qerror_report(QERR_SET_PASSWD_FAILED); > - return -1; > - } > - return 0; > + return vnc_display_pw_expire(NULL, when); > } > > qerror_report(QERR_INVALID_PARAMETER, "protocol"); > diff --git a/qerror.h b/qerror.h > index f732d45..df61d2c 100644 > --- a/qerror.h > +++ b/qerror.h > @@ -171,4 +171,7 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_VNC_SERVER_FAILED \ > "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }" > > +#define QERR_FEATURE_DISABLED \ > + "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }" > + > #endif /* QERROR_H */ > diff --git a/ui/vnc.c b/ui/vnc.c > index 34dc0cd..dd7a44a 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -2638,16 +2638,19 @@ int vnc_display_disable_login(DisplayState *ds) > > int vnc_display_password(DisplayState *ds, const char *password) > { > + int ret = 0; > VncDisplay *vs = ds ? (VncDisplay *)ds->opaque : vnc_display; > > if (!vs) { > - return -1; > + ret = -EINVAL; > + goto out; > } > > if (!password) { > /* This is not the intention of this interface but err on the side > of being safe */ > - return vnc_display_disable_login(ds); > + ret = vnc_display_disable_login(ds); > + goto out; > } > > if (vs->password) { > @@ -2656,8 +2659,11 @@ int vnc_display_password(DisplayState *ds, const char *password) > } > vs->password = qemu_strdup(password); > vs->auth = VNC_AUTH_VNC; > - > - return 0; > +out: > + if (ret != 0) { > + qerror_report(QERR_SET_PASSWD_FAILED); > + } > + return ret; > } > > int vnc_display_pw_expire(DisplayState *ds, time_t expires) > diff --git a/vl.c b/vl.c > index a1faf65..08a1d72 100644 > --- a/vl.c > +++ b/vl.c > @@ -208,7 +208,9 @@ int smp_cpus = 1; > int max_cpus = 0; > int smp_cores = 1; > int smp_threads = 1; > +#ifdef CONFIG_VNC > const char *vnc_display; > +#endif > int shmem_video = 0; > int acpi_enabled = 1; > int no_hpet = 0; > @@ -1940,7 +1942,9 @@ int main(int argc, char **argv, char **envp) > int tb_size; > const char *pid_file = NULL; > const char *incoming = NULL; > +#ifdef CONFIG_VNC > int show_vnc_port = 0; > +#endif > int defconfig = 1; > > #ifdef CONFIG_SIMPLE_TRACE > @@ -2585,10 +2589,12 @@ int main(int argc, char **argv, char **envp) > exit(1); > } > break; > +#ifdef CONFIG_VNC > case QEMU_OPTION_vnc: > display_remote++; > vnc_display = optarg; > break; > +#endif Sorry, should have mentioned it in the last note, but same rules apply to command line options. Instead of #if 0's thing out, we should print a friendly message saying that this feature is disabled. > case QEMU_OPTION_shmem_video: > shmem_video = 1; > break; > @@ -3049,7 +3055,7 @@ int main(int argc, char **argv, char **envp) > if (display_type == DT_DEFAULT&& !display_remote) { > #if defined(CONFIG_SDL) || defined(CONFIG_COCOA) > display_type = DT_SDL; > -#else > +#elif defined(CONFIG_VNC) > vnc_display = "localhost:0,to=99"; > show_vnc_port = 1; > #endif > @@ -3078,6 +3084,7 @@ int main(int argc, char **argv, char **envp) > break; > } > > +#ifdef CONFIG_VNC > /* init remote displays */ > if (vnc_display) { > vnc_display_init(ds); > @@ -3088,6 +3095,7 @@ int main(int argc, char **argv, char **envp) > printf("VNC server running on `%s'\n", vnc_display_local_addr(ds)); > } > } > +#endif So what ends up being the default display if VNC and SDL are both disabled? Have you tested this? Regards, Anthony Liguori > #ifdef CONFIG_SPICE > if (using_spice&& !qxl_enabled) { > qemu_spice_display_init(ds);