Re: [PATCH 6/6] libxl: vncviewer: make autopass work properly

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 6/6] libxl: vncviewer: make autopass work properly"):
> Even though it might be tolerable to have this function exits, I still
> don't see any benefits as opposed to failing with an error, considering
> that this function returns an integer and the long term plan would be
> to return an error anyway.

Very well, how about this.

> It is just a matter of checking the return value in
> tools/libxl/xl_cmdimpl.c:vncviewer.

It already does.  (Nice to see, for a change.)

Ian.


libxl: vncviewer: make autopass work properly

The file we write the vnc password to must be rewound back to the
beginning, or the vnc viewer will simply get EOF.

When the syscalls for communicating the password to the vnc client
fail, bomb out with an error messsage rather than blundering on (and
probably producing a spurious password prompt).

Following this patch, xl vncviewer --autopass works, provided the qemu
patch for writing the password to xenstore has also been applied.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b386a2a..3a351e2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -827,25 +827,38 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass)
     if ( vnc_pass ) {
         char tmpname[] = "/tmp/vncautopass.XXXXXX";
         autopass_fd = mkstemp(tmpname);
-        if ( autopass_fd < 0 )
-            goto skip_autopass;
+        if ( autopass_fd < 0 ) {
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+                             "mkstemp %s failed", tmpname);
+            goto x_fail;
+        }
 
-        if ( unlink(tmpname) )
+        if ( unlink(tmpname) ) {
             /* should never happen */
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unlink %s failed", tmpname);
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+                             "unlink %s failed", tmpname);
+            goto x_fail;
+        }
 
         if ( libxl_write_exactly(ctx, autopass_fd, vnc_pass, strlen(vnc_pass),
-                                    tmpname, "vnc password") ) {
-            do { close(autopass_fd); } while(errno == EINTR);
-            goto skip_autopass;
+                                    tmpname, "vnc password") )
+            goto x_fail;
+
+        if ( lseek(autopass_fd, SEEK_SET, 0) ) {
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+                             "rewind %s (autopass) failed", tmpname);
+            goto x_fail; 
         }
 
         args[2] = "-autopass";
     }
 
-skip_autopass:
     libxl__exec(autopass_fd, -1, -1, args[0], args);
     abort();
+
+ x_fail:
+    libxl__free_all(&gc);
+    return ERROR_FAIL;
 }
 
 /******************************************************************************/