[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 1/3] qemu: Make all SPICE command-line args optional



Hey,

After this patch series, the QEMU command line may not contain port nor
tls-port if they both were set to '0'. However, QEMU versions older than
2.3.0 will error out because they don't have this commit:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=cf7856adefebe86e0

I assume we want to keep supporting older QEMU binaries, and that this
needs to be fixed on libvirt side?

Christophe


On Wed, Mar 16, 2016 at 05:45:03PM +0100, Christophe Fergeau wrote:
> The end goal is to avoid adding -spice port=0,addr=127.0.0.1 to QEMU command
> line when no SPICE port is specified in libvirt XML.
> 
> Currently, the code relies on port=xx to always be present, so subsequent
> args can be unconditionally appended with a leading ','. Since port=0
> will no longer be added in a subsequent commit, we append a ',' to every
> arg instead of prepending, and remove the last one before adding it to
> the arg list.
> ---
>  src/qemu/qemu_command.c | 68 +++++++++++++++++++++++++++----------------------
>  1 file changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 32d32b1..cd20a16 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7030,7 +7030,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>      }
>  
>      if (port > 0 || tlsPort <= 0)
> -        virBufferAsprintf(&opt, "port=%u", port);
> +        virBufferAsprintf(&opt, "port=%u,", port);
>  
>      if (tlsPort > 0) {
>          if (!cfg->spiceTLS) {
> @@ -7039,13 +7039,12 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>                               " but TLS is disabled in qemu.conf"));
>              goto error;
>          }
> -        if (port > 0)
> -            virBufferAddChar(&opt, ',');
> -        virBufferAsprintf(&opt, "tls-port=%u", tlsPort);
> +
> +        virBufferAsprintf(&opt, "tls-port=%u,", tlsPort);
>      }
>  
>      if (cfg->spiceSASL) {
> -        virBufferAddLit(&opt, ",sasl");
> +        virBufferAddLit(&opt, "sasl,");
>  
>          if (cfg->spiceSASLdir)
>              virCommandAddEnvPair(cmd, "SASL_CONF_PATH",
> @@ -7085,17 +7084,17 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>      if (!listenAddr)
>          listenAddr = cfg->spiceListen;
>      if (listenAddr)
> -        virBufferAsprintf(&opt, ",addr=%s", listenAddr);
> +        virBufferAsprintf(&opt, "addr=%s,", listenAddr);
>  
>      VIR_FREE(netAddr);
>  
>      if (graphics->data.spice.mousemode) {
>          switch (graphics->data.spice.mousemode) {
>          case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER:
> -            virBufferAddLit(&opt, ",agent-mouse=off");
> +            virBufferAddLit(&opt, "agent-mouse=off,");
>              break;
>          case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT:
> -            virBufferAddLit(&opt, ",agent-mouse=on");
> +            virBufferAddLit(&opt, "agent-mouse=on,");
>              break;
>          default:
>              break;
> @@ -7106,18 +7105,19 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>       * making it visible on CLI, so there's no use of password=XXX
>       * in this bit of the code */
>      if (!graphics->data.spice.auth.passwd &&
> -        !cfg->spicePassword)
> -        virBufferAddLit(&opt, ",disable-ticketing");
> +        !cfg->spicePassword) {
> +        virBufferAddLit(&opt, "disable-ticketing,");
> +    }
>  
>      if (tlsPort > 0)
> -        virBufferAsprintf(&opt, ",x509-dir=%s", cfg->spiceTLSx509certdir);
> +        virBufferAsprintf(&opt, "x509-dir=%s,", cfg->spiceTLSx509certdir);
>  
>      switch (defaultMode) {
>      case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> -        virBufferAddLit(&opt, ",tls-channel=default");
> +        virBufferAddLit(&opt, "tls-channel=default,");
>          break;
>      case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE:
> -        virBufferAddLit(&opt, ",plaintext-channel=default");
> +        virBufferAddLit(&opt, "plaintext-channel=default,");
>          break;
>      case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY:
>          /* nothing */
> @@ -7133,7 +7133,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>                                   "but TLS port is not provided"));
>                  goto error;
>              }
> -            virBufferAsprintf(&opt, ",tls-channel=%s",
> +            virBufferAsprintf(&opt, "tls-channel=%s,",
>                                virDomainGraphicsSpiceChannelNameTypeToString(i));
>              break;
>  
> @@ -7144,7 +7144,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>                                   "configuration, but plain port is not provided"));
>                  goto error;
>              }
> -            virBufferAsprintf(&opt, ",plaintext-channel=%s",
> +            virBufferAsprintf(&opt, "plaintext-channel=%s,",
>                                virDomainGraphicsSpiceChannelNameTypeToString(i));
>              break;
>  
> @@ -7175,30 +7175,36 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>          }
>      }
>  
> -    if (graphics->data.spice.image)
> -        virBufferAsprintf(&opt, ",image-compression=%s",
> +    if (graphics->data.spice.image) {
> +        virBufferAsprintf(&opt, "image-compression=%s,",
>                            virDomainGraphicsSpiceImageCompressionTypeToString(graphics->data.spice.image));
> -    if (graphics->data.spice.jpeg)
> -        virBufferAsprintf(&opt, ",jpeg-wan-compression=%s",
> +    }
> +    if (graphics->data.spice.jpeg) {
> +        virBufferAsprintf(&opt, "jpeg-wan-compression=%s,",
>                            virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg));
> -    if (graphics->data.spice.zlib)
> -        virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s",
> +    }
> +    if (graphics->data.spice.zlib) {
> +        virBufferAsprintf(&opt, "zlib-glz-wan-compression=%s,",
>                            virDomainGraphicsSpiceZlibCompressionTypeToString(graphics->data.spice.zlib));
> -    if (graphics->data.spice.playback)
> -        virBufferAsprintf(&opt, ",playback-compression=%s",
> +    }
> +    if (graphics->data.spice.playback) {
> +        virBufferAsprintf(&opt, "playback-compression=%s,",
>                            virTristateSwitchTypeToString(graphics->data.spice.playback));
> -    if (graphics->data.spice.streaming)
> -        virBufferAsprintf(&opt, ",streaming-video=%s",
> +    }
> +    if (graphics->data.spice.streaming) {
> +        virBufferAsprintf(&opt, "streaming-video=%s,",
>                            virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming));
> -    if (graphics->data.spice.copypaste == VIR_TRISTATE_BOOL_NO)
> -        virBufferAddLit(&opt, ",disable-copy-paste");
> +    }
> +    if (graphics->data.spice.copypaste == VIR_TRISTATE_BOOL_NO) {
> +        virBufferAddLit(&opt, "disable-copy-paste,");
> +    }
>      if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO) {
>          if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) {
>             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                            _("This QEMU can't disable file transfers through spice"));
>              goto error;
>          } else {
> -            virBufferAddLit(&opt, ",disable-agent-file-xfer");
> +            virBufferAddLit(&opt, "disable-agent-file-xfer,");
>          }
>      }
>  
> @@ -7211,7 +7217,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>  
>          /* spice.gl is a TristateBool, but qemu expects on/off: use
>           * TristateSwitch helper */
> -        virBufferAsprintf(&opt, ",gl=%s",
> +        virBufferAsprintf(&opt, "gl=%s,",
>                            virTristateSwitchTypeToString(graphics->data.spice.gl));
>      }
>  
> @@ -7220,9 +7226,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>           * unconditionally on. If migration destination
>           * doesn't support it, it fallbacks to previous
>           * migration algorithm silently. */
> -        virBufferAddLit(&opt, ",seamless-migration=on");
> +        virBufferAddLit(&opt, "seamless-migration=on,");
>      }
>  
> +    virBufferTrim(&opt, ",", -1);
> +
>      virCommandAddArg(cmd, "-spice");
>      virCommandAddArgBuffer(cmd, &opt);
>      if (graphics->data.spice.keymap)
> -- 
> 2.5.0
> 
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]