[Spice-devel,qxl] spiceqxl: Improve the Xspice and Xorg configuration option descriptions

Submitted by Francois Gouget on Oct. 31, 2016, 8:50 p.m.

Details

Message ID E1c1JXi-0001Xy-91@amboise
State New
Headers show
Series "spiceqxl: Improve the Xspice and Xorg configuration option descriptions" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Francois Gouget Oct. 31, 2016, 8:50 p.m.
Group the options more logically and improve their descriptions.
In the Xorg configuration, always show the default in the
commented-out sample.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
---
 examples/spiceqxl.xorg.conf.example | 166 +++++++++++++++++++-----------------
 scripts/Xspice                      |  57 ++++++-------
 2 files changed, 117 insertions(+), 106 deletions(-)

Patch hide | download patch | download mbox

diff --git a/examples/spiceqxl.xorg.conf.example b/examples/spiceqxl.xorg.conf.example
index b6f4840..aeae93a 100644
--- a/examples/spiceqxl.xorg.conf.example
+++ b/examples/spiceqxl.xorg.conf.example
@@ -2,111 +2,123 @@  Section "Device"
     Identifier "XSPICE"
     Driver "spiceqxl"
 
-    # Enable regular port. Either this or SpiceTlsPort, or one of XSPICE_PORT or
-    # XSPICE_TLS_PORT environment variables must be specified
+    # ---- Network and security options
+
+    # Use Spice's regular unencrypted port. One of SpicePort or SpiceTlsPort
+    # must be specified. SpicePort can also be set through the XSPICE_PORT
+    # environment variable or the --port Xspice option.
     # Specify 0 to disable the use of the regular port
     # default: 5900
     #Option "SpicePort" "5900"
 
-    # Enable a TLS (encrypted) port. Either this or SpicePort must be specified,
-    # either here or via environment varialbes or via xspice --port or --tls-port
-    #Option "SpiceTlsPort" "5901"
+    # Use a TLS (encrypted) port. One of SpicePort or SpiceTlsPort must be
+    # specified. SpiceTlsPort can also be set through the XSPICE_TLS_PORT
+    # environment variable or the --tls-port Xspice option.
+    #Option "SpiceTlsPort" "5900"
     
-    # Listen to a specific interface. Default is to listen to all (0.0.0.0)
+    # Listen to a specific interface.
+    # default: Listen to all (0.0.0.0).
     #Option "SpiceAddr" ""
 
-    # Enable usage of SASL supported by spice-gtk client. Not required,
+    # Enable usage of SASL supported by the spice-gtk client. Not required,
+    # default: False
+    #Option "SpiceSasl" "False"
+
+    # Do not require a client password.
     # default: False
-    #Option "SpiceSasl" "True"
+    #Option "SpiceDisableTicketing" "False"
 
-    # Do not request any password from client
-    #Option "SpiceDisableTicketing" "0"
+    # Set the password required to connect to the Spice server.
+    #Option "SpicePassword" ""
 
-    # Set directory where cacert, server key and server cert are searched
-    # using the same predefined names qemu uses:
+    # Set the directory where the CA certificate, server key and server
+    # certificate are searched for TLS, using the same predefined names QEMU
+    # uses:
     #   cacert.pem, server-key.pem, server-cert.pem
     #Option "SpiceX509Dir" ""
 
-    # Set password client will be required to produce.
-    #Option "SpicePassword" ""
+    # Set the CA certificate file location for TLS.
+    #Option "SpiceCacertFile" ""
 
-    # Set spice server key file.
+    # Set the server key file location for TLS.
     #Option "SpiceX509KeyFile" ""
 
-    # Set cert file location.
+    # Set the server key's password for TLS.
+    #Option "SpiceX509KeyPassword" ""
+
+    # Set the server certificate file location for TLS.
     #Option "SpiceX509CertFile" ""
 
-    # Set key file password.
-    #Option "SpiceX509KeyPassword" ""
+    # Set the server DH file location for TLS.
+    #Option "SpiceDhFile" ""
 
-    # Set tls ciphers used.
+    # Set the TLS ciphers preference order.
     #Option "SpiceTlsCiphers" ""
 
-    # Set cacert file.
-    #Option "SpiceCacertFile" ""
+    # Set to True to only listen on IPv4 interfaces.
+    # default: False
+    #Option "SpiceIPV4Only" "False"
 
-    # Set dh file used.
-    #Option "SpiceDhFile" ""
+    # Set to True to only listen on IPv6 interfaces.
+    # default: False
+    #Option "SpiceIPV6Only" "False"
 
-    # Set streaming video method. Options are filter, off, all
+    # If set, the Spice server will exit when the first client disconnects.
+    # default: False
+    #Option "SpiceExitOnDisconnect" "True"
+
+
+    # ---- Compression options
+
+    # Set the streaming video method. Options are filter, off, all.
     # default: filter
     #Option "SpiceStreamingVideo" ""
 
-    # Set video codecs to use.  Provide a semicolon list of
-    # codecs, in preference order.  Each codec requires an encoder
-    # which can be one of spice or gstreamer, and then a codec type,
-    # for instance mjpeg or vp8. The default is spice:mjpeg,
-    # which uses the builtin mjpeg encoder.
+    # Sets a semicolon-separated list of preferred video codecs.
+    # Each takes the form encoder:codec, with spice:mjpeg being the default,
+    # and other options being provided by gstreamer for the mjpeg, vp8 and h264
+    # codecs.
     #Option "SpiceVideoCodecs" ""
 
     # Set zlib glz wan compression. Options are auto, never, always.
     # default: auto
     #Option "SpiceZlibGlzWanCompression" ""
 
-    # Set jpeg wan compression. Options are auto, never, always
+    # Set jpeg wan compression. Options are auto, never, always.
     # default: auto
     #Option "SpiceJpegWanCompression" ""
 
-    # Set image compression. Options are off,auto_glz,auto_lz,quic,glz,lz.
+    # Set image compression. Options are off, auto_glz, auto_lz, quic, glz, lz.
     # default: auto_glz
     #Option "SpiceImageCompression" ""
 
-    # Set to true to only listen on ipv4 interfaces.
-    # default: False
-    #Option "SpiceIPV4Only" ""
-
-    # Set to true to only listen on ipv6 interfaces.
-    # default: False
-    #Option "SpiceIPV6Only" ""
-
     # If non zero, the driver will render all operations to the frame buffer,
-    #  and keep track of a changed rectangle list.  The changed rectangles
-    #  will be transmitted at the rate requested (e.g. 10 Frames Per Second)
+    # and keep track of a changed rectangle list. The changed rectangles
+    # will be transmitted at the rate requested (e.g. 10 frames per second).
     # This can dramatically reduce network bandwidth for some use cases.
     #Option "SpiceDeferredFPS" "10"
 
-    # If set, the Spice Server will exit when the first client disconnects
-    #Option "SpiceExitOnDisconnect" "1"
-
-    # Enable caching of images directly written with uxa->put_image
+    # Enable caching of images directly written with uxa->put_image.
     # default: True
     #Option "EnableImageCache" "True"
 
-    # Enable caching of images created by uxa->prepare_access
+    # Enable caching of images created by uxa->prepare_access.
     # default: True
     #Option "EnableFallbackCache" "True"
 
-    # Enable the use of off screen srufaces
+    # Enable the use of off-screen surfaces.
     # default: True
     #Option "EnableSurfaces" "True"
 
-    # The number of heads to allocate by default
+    # The number of heads to allocate by default.
     # default: 4
     #Option "NumHeads" "4"
 
-    #--------------------------------------------------------------
+
+    # ---- Xspice-specific buffer options
+
     # Buffer Size notes:
-    #  The following buffer sizes are used for Xspice only
+    #  The following buffer sizes are used for Xspice only.
     #  If you are using the DFPS mode, surface ram is not used,
     #  and you can set it to 1.
     #  Otherwise, the surface buffer should be at least as large
@@ -117,13 +129,12 @@  Section "Device"
     #   is a command buffer 7x the size of the frame buffer.
     #  If you see 'Out of memory' errors in your xorg.log, you probably need
     #   to increase the surface or command buffer sizes.
-    #--------------------------------------------------------------
 
-    # The amount of surface buffer ram, in megabytes, to allocate
+    # The amount of surface buffer ram, in megabytes, to allocate.
     # default: 128
     #Option "SurfaceBufferSize" "128"
 
-    # The amount of command buffer ram, in megabytes, to allocate
+    # The amount of command buffer ram, in megabytes, to allocate.
     # default: 128
     #Option "CommandBufferSize" "128"
 
@@ -134,51 +145,52 @@  Section "Device"
     # default: 16
     #Option "FrameBufferSize" "16"
 
-    # Whether or not to accept Vdagent connections
+
+    # ---- VDAgent options
+
+    # Whether or not to accept Vdagent connections.
     # default: False
     #Option "SpiceVdagentEnabled" "False"
 
-    # Default path to listen for agent virtual io connections
-    #  default: /tmp/xspice-virtio"
+    # Default path to listen for agent virtual io connections.
+    # default: /tmp/xspice-virtio"
     #Option "SpiceVdagentVirtioPath" "/tmp/xspice-virtio"
 
-    # Default path to listen for agent uinput connections
-    #  default: /tmp/xspice-uinput
+    # Default path to listen for agent uinput connections.
+    # default: /tmp/xspice-uinput
     #Option "SpiceVdagentUinputPath" "/tmp/xspice-uinput"
 
-    # Default user id to set for the Virtio and Uinput sockets
-    #  default: 0, or none
-    #  Note: both uid and gid must be given for either to take
-    #  effect
+    # Default user id to set for the Virtio and Uinput sockets.
+    # Note: both uid and gid must be given for either to take effect.
+    # default: 0, or none
     #Option "SpiceVdagentUid" "0"
 
-    # Default group id to set for the Virtio and Uinput sockets
-    #  default: 0, or none
-    #  Note: both uid and gid must be given for either to take
-    #  effect
+    # Default group id to set for the Virtio and Uinput sockets.
+    # Note: both uid and gid must be given for either to take effect.
+    # default: 0, or none
     #Option "SpiceVdagentGid" "0"
 
-    # Set Spice Agent Mouse - Use Agent mouse if an agent connects
+    # Set Spice Agent Mouse - Use Agent mouse if an agent connects.
     # default: True
     #Option "SpiceAgentMouse" "True"
 
-    # Set Spice Playback compression
+    # Set Spice Playback compression.
     # default: True
     #Option "SpicePlaybackCompression" "True"
 
-    # Disable copy and paste
+    # Disable copy and paste.
     # default: False
     #Option "SpiceDisableCopyPaste" "False"
 
-    # If a directory is given, any file in that
-    #  directory will be read for audio data to be sent
-    #  to the client.   Default is no mixing.
-    #Option "SpicePlaybackFIFODir"  "/tmp/"
+    # If a directory is given, any file in that directory will be read
+    # for audio data to be sent to the client.
+    # default: Not set.
+    #Option "SpicePlaybackFIFODir" "/tmp/"
 
-    # A unix domain name for a unix domain socket
-    #  to communicate with a spiceccid smartcard driver
-    #  Default is not to use a smartcard driver.
-    #Option "SpiceSmartCardFile"  "/tmp/spice.pcsc.comm"
+    # A unix domain name for a unix domain socket to communicate with
+    # a spiceccid smartcard driver.
+    # default: Not set.
+    #Option "SpiceSmartCardFile" "/tmp/spice.pcsc.comm"
 EndSection
 
 Section "InputDevice"
diff --git a/scripts/Xspice b/scripts/Xspice
index bf8112f..6d1afaf 100755
--- a/scripts/Xspice
+++ b/scripts/Xspice
@@ -59,37 +59,37 @@  parser.add_argument('--xsession', help='If given, will run after Xorg launch.  S
 parser.add_argument('--config', default='spiceqxl.xorg.conf')
 # Don't use any options that are already used by Xorg (unless we must)
 # specifically, don't use -p and -s.
-parser.add_argument('--port', type=int, help='standard spice port')
+parser.add_argument('--port', type=int, help="Use the specified port as Spice's regular unencrypted port.")
+parser.add_argument('--tls-port', type=int, help='Use the specified port as a TLS (encrypted) port.', default=0)
+add_boolean('--disable-ticketing', help="Do not require a client password.")
+parser.add_argument('--password', help="Set the password required to connect to the server.")
+add_boolean('--sasl', help="Use SASL to authenticate to the server.")
+parser.add_argument('--x509-dir', help="Set the directory where the CA certificate, server key and server certificate are searched for TLS, using the same predefined names QEMU uses.")
+parser.add_argument('--cacert-file', help="Set the CA certificate file location for TLS.")
+parser.add_argument('--x509-key-file', help="Set the server key file location for TLS.")
+parser.add_argument('--x509-key-password', help="Set the server key's password for TLS.")
+parser.add_argument('--x509-cert-file', help="Set the server certificate file location for TLS.")
+parser.add_argument('--dh-file', help="Set the server DH file location for TLS.")
+parser.add_argument('--tls-ciphers', help="Set the TLS ciphers preference order.")
+add_boolean('--ipv4-only')
+add_boolean('--ipv6-only')
 parser.add_argument('--exit-on-disconnect', action='store_true', help='Exit the X server when any client disconnects')
 parser.add_argument('--numheads', type=int, help='Number of virtual heads to create.')
-parser.add_argument('--deferred-fps', type=int, help='If given, render to a buffer and send updates only this many times per second')
-parser.add_argument('--tls-port', type=int, help='spice tls port', default=0)
-add_boolean('--disable-ticketing', help="do not require a client password")
-add_boolean('--sasl', help="enable sasl")
-parser.add_argument('--x509-dir', help="x509 directory for tls")
-parser.add_argument('--cacert-file', help="ca certificate file for tls")
-parser.add_argument('--x509-cert-file', help="server certificate file for tls")
-parser.add_argument('--x509-key-file', help="server key file for tls")
-parser.add_argument('--x509-key-password', help="key file password for tls")
-parser.add_argument('--tls-ciphers')
-parser.add_argument('--dh-file')
-parser.add_argument('--password', help="set password required to connect to server")
 parser.add_argument('--image-compression',
                     choices = ['off', 'auto_glz', 'auto_lz', 'quic',
                                'glz', 'lz'],
-                    help='auto_glz by default')
+                    help="Set image compression.")
 parser.add_argument('--jpeg-wan-compression',
                     choices=wan_compression_options,
-                    help='auto by default')
+                    help="Set jpeg wan compression.")
 parser.add_argument('--zlib-glz-wan-compression',
                     choices=wan_compression_options,
-                    help='auto by default')
+                    help="Set zlib glz wan compression.")
+parser.add_argument('--deferred-fps', type=int, help='If non zero, the driver will render all operations to the frame buffer, and keep track of a changed rectangle list. The changed rectangles will be transmitted at the rate requested (e.g. 10 frames per second). This can dramatically reduce network bandwidth for some use cases.')
 # TODO - sound support
 parser.add_argument('--streaming-video', choices=['off', 'all', 'filter'],
-                    help='filter by default')
-parser.add_argument('--video-codecs', help="Sets a semicolon-separated list of preferred video codecs to use. Each takes the form encoder:codec, with spice:mjpeg being the default and other options being provided by gstreamer for the mjpeg, vp8 and h264 codecs.")
-add_boolean('--ipv4-only')
-add_boolean('--ipv6-only')
+                    help='Set the streaming video method.')
+parser.add_argument('--video-codecs', help='Sets a semicolon-separated list of preferred video codecs to use. Each takes the form encoder:codec, with spice:mjpeg being the default and other options being provided by gstreamer for the mjpeg, vp8 and h264 codecs.')
 parser.add_argument('--vdagent', action='store_true', dest='vdagent_enabled', default=False, help='launch vdagent & vdagentd. They provide clipboard & resolution automation')
 parser.add_argument('--vdagent-virtio-path', help='virtio socket path used by vdagentd')
 parser.add_argument('--vdagent-uinput-path', help='uinput socket path used by vdagent')
@@ -99,7 +99,7 @@  parser.add_argument('--vdagent-exec', help='path to spice-vdagent (used with --v
 parser.add_argument('--vdagent-no-launch', default=True, action='store_false', dest='vdagent_launch', help='do not launch vdagent & vdagentd, used for debugging or if some external script wants to take care of that')
 parser.add_argument('--vdagent-uid', default=str(os.getuid()), help='set vdagent user id. changing it makes sense only in conjunction with --vdagent-no-launch')
 parser.add_argument('--vdagent-gid', default=str(os.getgid()), help='set vdagent group id. changing it makes sense only in conjunction with --vdagent-no-launch')
-parser.add_argument('--audio-fifo-dir', help="set fifo directory for playback audio. designed to work with PulseAudio's module-pipe-sink")
+parser.add_argument('--audio-fifo-dir', help="If a directory is given, any file in that directory will be read for audio data to be sent to the client. This is designed to work with PulseAudio's module-pipe-sink.")
 
 #TODO
 #Option "SpiceAddr" ""
@@ -278,14 +278,13 @@  if args.vdagent_enabled:
 
     cleanup_files.extend([args.vdagent_udcs_path, args.vdagent_virtio_path, args.vdagent_uinput_path])
 
-var_args = ['port', 'tls_port', 'disable_ticketing',
-    'x509_dir', 'sasl', 'cacert_file', 'x509_cert_file',
-    'x509_key_file', 'x509_key_password',
-    'tls_ciphers', 'dh_file', 'password', 'image_compression',
-    'jpeg_wan_compression', 'zlib_glz_wan_compression',
-    'streaming_video', 'video_codecs', 'deferred_fps', 'exit_on_disconnect',
-    'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path',
-    'vdagent_uid', 'vdagent_gid']
+var_args = ['port', 'tls_port', 'disable_ticketing', 'password',
+            'sasl', 'x509_dir', 'cacert_file', 'x509_key_file',
+            'x509_key_password', 'x509_cert_file', 'dh_file', 'tls_ciphers',
+            'exit_on_disconnect', 'image_compression', 'jpeg_wan_compression',
+            'zlib_glz_wan_compression', 'deferred_fps', 'streaming_video',
+            'video_codecs', 'vdagent_enabled', 'vdagent_virtio_path',
+            'vdagent_uinput_path', 'vdagent_uid', 'vdagent_gid']
 
 for arg in var_args:
     if getattr(args, arg) != None:

Comments

On Mon, Oct 31, 2016 at 09:50:54PM +0100, Francois Gouget wrote:
> Group the options more logically and improve their descriptions.
> In the Xorg configuration, always show the default in the
> commented-out sample.
> 
> Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> ---
>  examples/spiceqxl.xorg.conf.example | 166 +++++++++++++++++++-----------------
>  scripts/Xspice                      |  57 ++++++-------
>  2 files changed, 117 insertions(+), 106 deletions(-)
> 
> diff --git a/scripts/Xspice b/scripts/Xspice
> index bf8112f..6d1afaf 100755
> --- a/scripts/Xspice
> +++ b/scripts/Xspice
> @@ -59,37 +59,37 @@ parser.add_argument('--xsession', help='If given, will run after Xorg launch.  S
>  parser.add_argument('--config', default='spiceqxl.xorg.conf')
>  # Don't use any options that are already used by Xorg (unless we must)
>  # specifically, don't use -p and -s.
> -parser.add_argument('--port', type=int, help='standard spice port')
> +parser.add_argument('--port', type=int, help="Use the specified port as Spice's regular unencrypted port.")
> +parser.add_argument('--tls-port', type=int, help='Use the specified port as a TLS (encrypted) port.', default=0)
> +add_boolean('--disable-ticketing', help="Do not require a client password.")
> +parser.add_argument('--password', help="Set the password required to connect to the server.")
> +add_boolean('--sasl', help="Use SASL to authenticate to the server.")
> +parser.add_argument('--x509-dir', help="Set the directory where the CA certificate, server key and server certificate are searched for TLS, using the same predefined names QEMU uses.")
> +parser.add_argument('--cacert-file', help="Set the CA certificate file location for TLS.")
> +parser.add_argument('--x509-key-file', help="Set the server key file location for TLS.")
> +parser.add_argument('--x509-key-password', help="Set the server key's password for TLS.")
> +parser.add_argument('--x509-cert-file', help="Set the server certificate file location for TLS.")
> +parser.add_argument('--dh-file', help="Set the server DH file location for TLS.")
> +parser.add_argument('--tls-ciphers', help="Set the TLS ciphers preference order.")
> +add_boolean('--ipv4-only')
> +add_boolean('--ipv6-only')

No help for these?

>  parser.add_argument('--exit-on-disconnect', action='store_true', help='Exit the X server when any client disconnects')
>  parser.add_argument('--numheads', type=int, help='Number of virtual heads to create.')
> -parser.add_argument('--deferred-fps', type=int, help='If given, render to a buffer and send updates only this many times per second')
> -parser.add_argument('--tls-port', type=int, help='spice tls port', default=0)
> -add_boolean('--disable-ticketing', help="do not require a client password")
> -add_boolean('--sasl', help="enable sasl")
> -parser.add_argument('--x509-dir', help="x509 directory for tls")
> -parser.add_argument('--cacert-file', help="ca certificate file for tls")
> -parser.add_argument('--x509-cert-file', help="server certificate file for tls")
> -parser.add_argument('--x509-key-file', help="server key file for tls")
> -parser.add_argument('--x509-key-password', help="key file password for tls")
> -parser.add_argument('--tls-ciphers')
> -parser.add_argument('--dh-file')
> -parser.add_argument('--password', help="set password required to connect to server")
>  parser.add_argument('--image-compression',
>                      choices = ['off', 'auto_glz', 'auto_lz', 'quic',
>                                 'glz', 'lz'],
> -                    help='auto_glz by default')
> +                    help="Set image compression.")
>  parser.add_argument('--jpeg-wan-compression',
>                      choices=wan_compression_options,
> -                    help='auto by default')
> +                    help="Set jpeg wan compression.")
>  parser.add_argument('--zlib-glz-wan-compression',
>                      choices=wan_compression_options,
> -                    help='auto by default')
> +                    help="Set zlib glz wan compression.")
> +parser.add_argument('--deferred-fps', type=int, help='If non zero, the driver will render all operations to the frame buffer, and keep track of a changed rectangle list. The changed rectangles will be transmitted at the rate requested (e.g. 10 frames per second). This can dramatically reduce network bandwidth for some use cases.')
>  # TODO - sound support
>  parser.add_argument('--streaming-video', choices=['off', 'all', 'filter'],
> -                    help='filter by default')
> -parser.add_argument('--video-codecs', help="Sets a semicolon-separated list of preferred video codecs to use. Each takes the form encoder:codec, with spice:mjpeg being the default and other options being provided by gstreamer for the mjpeg, vp8 and h264 codecs.")
> -add_boolean('--ipv4-only')
> -add_boolean('--ipv6-only')
> +                    help='Set the streaming video method.')
> +parser.add_argument('--video-codecs', help='Sets a semicolon-separated list of preferred video codecs to use. Each takes the form encoder:codec, with spice:mjpeg being the default and other options being provided by gstreamer for the mjpeg, vp8 and h264 codecs.')
>  parser.add_argument('--vdagent', action='store_true', dest='vdagent_enabled', default=False, help='launch vdagent & vdagentd. They provide clipboard & resolution automation')
>  parser.add_argument('--vdagent-virtio-path', help='virtio socket path used by vdagentd')
>  parser.add_argument('--vdagent-uinput-path', help='uinput socket path used by vdagent')
> @@ -99,7 +99,7 @@ parser.add_argument('--vdagent-exec', help='path to spice-vdagent (used with --v
>  parser.add_argument('--vdagent-no-launch', default=True, action='store_false', dest='vdagent_launch', help='do not launch vdagent & vdagentd, used for debugging or if some external script wants to take care of that')
>  parser.add_argument('--vdagent-uid', default=str(os.getuid()), help='set vdagent user id. changing it makes sense only in conjunction with --vdagent-no-launch')
>  parser.add_argument('--vdagent-gid', default=str(os.getgid()), help='set vdagent group id. changing it makes sense only in conjunction with --vdagent-no-launch')
> -parser.add_argument('--audio-fifo-dir', help="set fifo directory for playback audio. designed to work with PulseAudio's module-pipe-sink")
> +parser.add_argument('--audio-fifo-dir', help="If a directory is given, any file in that directory will be read for audio data to be sent to the client. This is designed to work with PulseAudio's module-pipe-sink.")
>  
>  #TODO
>  #Option "SpiceAddr" ""
> @@ -278,14 +278,13 @@ if args.vdagent_enabled:
>  
>      cleanup_files.extend([args.vdagent_udcs_path, args.vdagent_virtio_path, args.vdagent_uinput_path])
>  
> -var_args = ['port', 'tls_port', 'disable_ticketing',
> -    'x509_dir', 'sasl', 'cacert_file', 'x509_cert_file',
> -    'x509_key_file', 'x509_key_password',
> -    'tls_ciphers', 'dh_file', 'password', 'image_compression',
> -    'jpeg_wan_compression', 'zlib_glz_wan_compression',
> -    'streaming_video', 'video_codecs', 'deferred_fps', 'exit_on_disconnect',
> -    'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path',
> -    'vdagent_uid', 'vdagent_gid']
> +var_args = ['port', 'tls_port', 'disable_ticketing', 'password',
> +            'sasl', 'x509_dir', 'cacert_file', 'x509_key_file',
> +            'x509_key_password', 'x509_cert_file', 'dh_file', 'tls_ciphers',
> +            'exit_on_disconnect', 'image_compression', 'jpeg_wan_compression',
> +            'zlib_glz_wan_compression', 'deferred_fps', 'streaming_video',
> +            'video_codecs', 'vdagent_enabled', 'vdagent_virtio_path',
> +            'vdagent_uinput_path', 'vdagent_uid', 'vdagent_gid']
>  

I'd drop this last hunk.

Looks good to me otherwise.

Christophe
> 
> On Mon, Oct 31, 2016 at 09:50:54PM +0100, Francois Gouget wrote:
> > Group the options more logically and improve their descriptions.
> > In the Xorg configuration, always show the default in the
> > commented-out sample.
> > 
> > Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> > ---
> >  examples/spiceqxl.xorg.conf.example | 166
> >  +++++++++++++++++++-----------------
> >  scripts/Xspice                      |  57 ++++++-------
> >  2 files changed, 117 insertions(+), 106 deletions(-)
> > 
> > diff --git a/scripts/Xspice b/scripts/Xspice
> > index bf8112f..6d1afaf 100755
> > --- a/scripts/Xspice
> > +++ b/scripts/Xspice
> > @@ -59,37 +59,37 @@ parser.add_argument('--xsession', help='If given, will
> > run after Xorg launch.  S
> >  parser.add_argument('--config', default='spiceqxl.xorg.conf')
> >  # Don't use any options that are already used by Xorg (unless we must)
> >  # specifically, don't use -p and -s.
> > -parser.add_argument('--port', type=int, help='standard spice port')
> > +parser.add_argument('--port', type=int, help="Use the specified port as
> > Spice's regular unencrypted port.")
> > +parser.add_argument('--tls-port', type=int, help='Use the specified port
> > as a TLS (encrypted) port.', default=0)
> > +add_boolean('--disable-ticketing', help="Do not require a client
> > password.")
> > +parser.add_argument('--password', help="Set the password required to
> > connect to the server.")
> > +add_boolean('--sasl', help="Use SASL to authenticate to the server.")
> > +parser.add_argument('--x509-dir', help="Set the directory where the CA
> > certificate, server key and server certificate are searched for TLS, using
> > the same predefined names QEMU uses.")
> > +parser.add_argument('--cacert-file', help="Set the CA certificate file
> > location for TLS.")
> > +parser.add_argument('--x509-key-file', help="Set the server key file
> > location for TLS.")
> > +parser.add_argument('--x509-key-password', help="Set the server key's
> > password for TLS.")
> > +parser.add_argument('--x509-cert-file', help="Set the server certificate
> > file location for TLS.")
> > +parser.add_argument('--dh-file', help="Set the server DH file location for
> > TLS.")
> > +parser.add_argument('--tls-ciphers', help="Set the TLS ciphers preference
> > order.")
> > +add_boolean('--ipv4-only')
> > +add_boolean('--ipv6-only')
> 
> No help for these?
> 
> >  parser.add_argument('--exit-on-disconnect', action='store_true',
> >  help='Exit the X server when any client disconnects')
> >  parser.add_argument('--numheads', type=int, help='Number of virtual heads
> >  to create.')
> > -parser.add_argument('--deferred-fps', type=int, help='If given, render to
> > a buffer and send updates only this many times per second')
> > -parser.add_argument('--tls-port', type=int, help='spice tls port',
> > default=0)
> > -add_boolean('--disable-ticketing', help="do not require a client
> > password")
> > -add_boolean('--sasl', help="enable sasl")
> > -parser.add_argument('--x509-dir', help="x509 directory for tls")
> > -parser.add_argument('--cacert-file', help="ca certificate file for tls")
> > -parser.add_argument('--x509-cert-file', help="server certificate file for
> > tls")
> > -parser.add_argument('--x509-key-file', help="server key file for tls")
> > -parser.add_argument('--x509-key-password', help="key file password for
> > tls")
> > -parser.add_argument('--tls-ciphers')
> > -parser.add_argument('--dh-file')
> > -parser.add_argument('--password', help="set password required to connect
> > to server")
> >  parser.add_argument('--image-compression',
> >                      choices = ['off', 'auto_glz', 'auto_lz', 'quic',
> >                                 'glz', 'lz'],
> > -                    help='auto_glz by default')
> > +                    help="Set image compression.")
> >  parser.add_argument('--jpeg-wan-compression',
> >                      choices=wan_compression_options,
> > -                    help='auto by default')
> > +                    help="Set jpeg wan compression.")
> >  parser.add_argument('--zlib-glz-wan-compression',
> >                      choices=wan_compression_options,
> > -                    help='auto by default')
> > +                    help="Set zlib glz wan compression.")
> > +parser.add_argument('--deferred-fps', type=int, help='If non zero, the
> > driver will render all operations to the frame buffer, and keep track of a
> > changed rectangle list. The changed rectangles will be transmitted at the
> > rate requested (e.g. 10 frames per second). This can dramatically reduce
> > network bandwidth for some use cases.')
> >  # TODO - sound support
> >  parser.add_argument('--streaming-video', choices=['off', 'all', 'filter'],
> > -                    help='filter by default')
> > -parser.add_argument('--video-codecs', help="Sets a semicolon-separated
> > list of preferred video codecs to use. Each takes the form encoder:codec,
> > with spice:mjpeg being the default and other options being provided by
> > gstreamer for the mjpeg, vp8 and h264 codecs.")
> > -add_boolean('--ipv4-only')
> > -add_boolean('--ipv6-only')
> > +                    help='Set the streaming video method.')
> > +parser.add_argument('--video-codecs', help='Sets a semicolon-separated
> > list of preferred video codecs to use. Each takes the form encoder:codec,
> > with spice:mjpeg being the default and other options being provided by
> > gstreamer for the mjpeg, vp8 and h264 codecs.')
> >  parser.add_argument('--vdagent', action='store_true',
> >  dest='vdagent_enabled', default=False, help='launch vdagent & vdagentd.
> >  They provide clipboard & resolution automation')
> >  parser.add_argument('--vdagent-virtio-path', help='virtio socket path used
> >  by vdagentd')
> >  parser.add_argument('--vdagent-uinput-path', help='uinput socket path used
> >  by vdagent')
> > @@ -99,7 +99,7 @@ parser.add_argument('--vdagent-exec', help='path to
> > spice-vdagent (used with --v
> >  parser.add_argument('--vdagent-no-launch', default=True,
> >  action='store_false', dest='vdagent_launch', help='do not launch vdagent
> >  & vdagentd, used for debugging or if some external script wants to take
> >  care of that')
> >  parser.add_argument('--vdagent-uid', default=str(os.getuid()), help='set
> >  vdagent user id. changing it makes sense only in conjunction with
> >  --vdagent-no-launch')
> >  parser.add_argument('--vdagent-gid', default=str(os.getgid()), help='set
> >  vdagent group id. changing it makes sense only in conjunction with
> >  --vdagent-no-launch')
> > -parser.add_argument('--audio-fifo-dir', help="set fifo directory for
> > playback audio. designed to work with PulseAudio's module-pipe-sink")
> > +parser.add_argument('--audio-fifo-dir', help="If a directory is given, any
> > file in that directory will be read for audio data to be sent to the
> > client. This is designed to work with PulseAudio's module-pipe-sink.")
> >  

OT: looks like some option helps start with capital and others not.
How is the end result of --help ?

Frediano
On Mon, 7 Nov 2016, Christophe Fergeau wrote:
[...]
> > +parser.add_argument('--dh-file', help="Set the server DH file location for TLS.")
> > +parser.add_argument('--tls-ciphers', help="Set the TLS ciphers preference order.")
> > +add_boolean('--ipv4-only')
> > +add_boolean('--ipv6-only')
> 
> No help for these?

I was mostly tweaking existing documentation, not adding more. But I 
added them in the new revision.


[...]
> > -var_args = ['port', 'tls_port', 'disable_ticketing',
> > -    'x509_dir', 'sasl', 'cacert_file', 'x509_cert_file',
> > -    'x509_key_file', 'x509_key_password',
> > -    'tls_ciphers', 'dh_file', 'password', 'image_compression',
> > -    'jpeg_wan_compression', 'zlib_glz_wan_compression',
> > -    'streaming_video', 'video_codecs', 'deferred_fps', 'exit_on_disconnect',
> > -    'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path',
> > -    'vdagent_uid', 'vdagent_gid']
> > +var_args = ['port', 'tls_port', 'disable_ticketing', 'password',
> > +            'sasl', 'x509_dir', 'cacert_file', 'x509_key_file',
> > +            'x509_key_password', 'x509_cert_file', 'dh_file', 'tls_ciphers',
> > +            'exit_on_disconnect', 'image_compression', 'jpeg_wan_compression',
> > +            'zlib_glz_wan_compression', 'deferred_fps', 'streaming_video',
> > +            'video_codecs', 'vdagent_enabled', 'vdagent_virtio_path',
> > +            'vdagent_uinput_path', 'vdagent_uid', 'vdagent_gid']
> >  
> 
> I'd drop this last hunk.

Ok.
On Mon, 7 Nov 2016, Frediano Ziglio wrote:
[...]
> > > -parser.add_argument('--audio-fifo-dir', help="set fifo directory for
> > > playback audio. designed to work with PulseAudio's module-pipe-sink")
> > > +parser.add_argument('--audio-fifo-dir', help="If a directory is given, any
> > > file in that directory will be read for audio data to be sent to the
> > > client. This is designed to work with PulseAudio's module-pipe-sink.")
> > >  
> 
> OT: looks like some option helps start with capital and others not.

Yes. It turns out that the help message for --help is hardcoded and 
start with a lowercase and does not have a trailing period. So I 
standardized on that to match.


> How is the end result of --help ?

I'll include it in v2.