[libvirt] [PATCH v2 1/3] qemu: refactor graphics code to not hardcode a single display
Jiri Denemark
jdenemar at redhat.com
Sat Nov 10 14:23:51 UTC 2012
On Sat, Nov 10, 2012 at 02:40:23 +0100, Alon Levy wrote:
> The check for a single display remains so no new functionality is added.
> ---
> v2 changes:
> removed enum, use virReportOOMError directly.
> use --patience
> added a one line label fix patch.
>
> The second patch changes a string that needs to be translated - how do I go about it?
>
> src/qemu/qemu_command.c | 641 +++++++++++++++++++++++++-----------------------
> src/qemu/qemu_process.c | 70 +++---
> 2 files changed, 364 insertions(+), 347 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1e96982..ba99c7a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4410,6 +4410,331 @@ error:
> return -1;
> }
>
> +static int
> +qemuBuildGraphicsCommandLine(struct qemud_driver *driver,
> + virCommandPtr cmd,
> + virDomainDefPtr def,
> + qemuCapsPtr caps,
> + virDomainGraphicsDefPtr graphics)
> +{
> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> + virBuffer opt = VIR_BUFFER_INITIALIZER;
> +
> + if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("vnc graphics are not supported with this QEMU"));
> + goto error;
> + }
> +
> + if (graphics->data.vnc.socket ||
> + driver->vncAutoUnixSocket) {
> +
> + if (!graphics->data.vnc.socket &&
> + virAsprintf(&graphics->data.vnc.socket,
> + "%s/%s.vnc", driver->libDir, def->name) == -1) {
> + goto no_memory;
> + }
> +
> + virBufferAsprintf(&opt, "unix:%s",
> + graphics->data.vnc.socket);
> +
> + } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) {
> + const char *listenNetwork;
> + const char *listenAddr = NULL;
> + char *netAddr = NULL;
> + bool escapeAddr;
> + int ret;
> +
> + switch (virDomainGraphicsListenGetType(graphics, 0)) {
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
> + break;
> +
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
> + if (!listenNetwork)
> + break;
> + ret = networkGetNetworkAddress(listenNetwork, &netAddr);
> + if (ret <= -2) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + "%s", _("network-based listen not possible, "
> + "network driver not present"));
> + goto error;
> + }
> + if (ret < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("listen network '%s' had no usable address"),
> + listenNetwork);
> + goto error;
> + }
> + listenAddr = netAddr;
> + /* store the address we found in the <graphics> element so it will
> + * show up in status. */
> + if (virDomainGraphicsListenSetAddress(graphics, 0,
> + listenAddr, -1, false) < 0)
> + goto error;
> + break;
> + }
> +
> + if (!listenAddr)
> + listenAddr = driver->vncListen;
> +
> + escapeAddr = strchr(listenAddr, ':') != NULL;
> + if (escapeAddr)
> + virBufferAsprintf(&opt, "[%s]", listenAddr);
> + else
> + virBufferAdd(&opt, listenAddr, -1);
> + virBufferAsprintf(&opt, ":%d",
> + graphics->data.vnc.port - 5900);
> +
> + VIR_FREE(netAddr);
> + } else {
> + virBufferAsprintf(&opt, "%d",
> + graphics->data.vnc.port - 5900);
> + }
> +
> + if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) {
> + if (graphics->data.vnc.auth.passwd ||
> + driver->vncPassword)
> + virBufferAddLit(&opt, ",password");
> +
> + if (driver->vncTLS) {
> + virBufferAddLit(&opt, ",tls");
> + if (driver->vncTLSx509verify) {
> + virBufferAsprintf(&opt, ",x509verify=%s",
> + driver->vncTLSx509certdir);
> + } else {
> + virBufferAsprintf(&opt, ",x509=%s",
> + driver->vncTLSx509certdir);
> + }
> + }
> +
> + if (driver->vncSASL) {
> + virBufferAddLit(&opt, ",sasl");
> +
> + if (driver->vncSASLdir)
> + virCommandAddEnvPair(cmd, "SASL_CONF_DIR",
> + driver->vncSASLdir);
> +
> + /* TODO: Support ACLs later */
> + }
> + }
> +
> + virCommandAddArg(cmd, "-vnc");
> + virCommandAddArgBuffer(cmd, &opt);
> + if (graphics->data.vnc.keymap) {
> + virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap,
> + NULL);
> + }
> +
> + /* Unless user requested it, set the audio backend to none, to
> + * prevent it opening the host OS audio devices, since that causes
> + * security issues and might not work when using VNC.
> + */
> + if (driver->vncAllowHostAudio) {
> + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
> + } else {
> + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> + }
> + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
> + if (qemuCapsGet(caps, QEMU_CAPS_0_10) &&
> + !qemuCapsGet(caps, QEMU_CAPS_SDL)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("sdl not supported by '%s'"),
> + def->emulator);
> + goto error;
> + }
> +
> + if (graphics->data.sdl.xauth)
> + virCommandAddEnvPair(cmd, "XAUTHORITY",
> + graphics->data.sdl.xauth);
> + if (graphics->data.sdl.display)
> + virCommandAddEnvPair(cmd, "DISPLAY",
> + graphics->data.sdl.display);
> + if (graphics->data.sdl.fullscreen)
> + virCommandAddArg(cmd, "-full-screen");
> +
> + /* If using SDL for video, then we should just let it
> + * use QEMU's host audio drivers, possibly SDL too
> + * User can set these two before starting libvirtd
> + */
> + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
> + virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
> +
> + /* New QEMU has this flag to let us explicitly ask for
> + * SDL graphics. This is better than relying on the
> + * default, since the default changes :-( */
> + if (qemuCapsGet(caps, QEMU_CAPS_SDL))
> + virCommandAddArg(cmd, "-sdl");
> +
> + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> + virBuffer opt = VIR_BUFFER_INITIALIZER;
> + const char *listenNetwork;
> + const char *listenAddr = NULL;
> + char *netAddr = NULL;
> + int ret;
> + int defaultMode = graphics->data.spice.defaultMode;
> +
> + if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("spice graphics are not supported with this QEMU"));
> + goto error;
> + }
> +
> + virBufferAsprintf(&opt, "port=%u", graphics->data.spice.port);
> +
> + if (graphics->data.spice.tlsPort > 0) {
> + if (!driver->spiceTLS) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("spice TLS port set in XML configuration,"
> + " but TLS is disabled in qemu.conf"));
> + return 1;
This should be goto error;
> + }
> + virBufferAsprintf(&opt, ",tls-port=%u",
> + graphics->data.spice.tlsPort);
> + }
> +
> + switch (virDomainGraphicsListenGetType(graphics, 0)) {
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
> + break;
> +
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
> + if (!listenNetwork)
> + break;
> + ret = networkGetNetworkAddress(listenNetwork, &netAddr);
> + if (ret <= -2) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + "%s", _("network-based listen not possible, "
> + "network driver not present"));
> + goto error;
> + }
> + if (ret < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("listen network '%s' had no usable address"),
> + listenNetwork);
> + goto error;
> + }
> + listenAddr = netAddr;
> + /* store the address we found in the <graphics> element so it will
> + * show up in status. */
> + if (virDomainGraphicsListenSetAddress(graphics, 0,
> + listenAddr, -1, false) < 0)
> + goto error;
> + break;
> + }
> +
> + if (!listenAddr)
> + listenAddr = driver->spiceListen;
> + if (listenAddr)
> + virBufferAsprintf(&opt, ",addr=%s", listenAddr);
> +
> + VIR_FREE(netAddr);
> +
> + int mm = graphics->data.spice.mousemode;
> + if (mm) {
> + switch (mm) {
> + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER:
> + virBufferAsprintf(&opt, ",agent-mouse=off");
> + break;
> + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT:
> + virBufferAsprintf(&opt, ",agent-mouse=on");
> + break;
> + default:
> + break;
> + }
> + }
> +
> + /* In the password case we set it via monitor command, to avoid
> + * 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 &&
> + !driver->spicePassword)
> + virBufferAddLit(&opt, ",disable-ticketing");
> +
> + if (driver->spiceTLS)
> + virBufferAsprintf(&opt, ",x509-dir=%s",
> + driver->spiceTLSx509certdir);
> +
> + switch (defaultMode) {
> + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> + virBufferAsprintf(&opt, ",tls-channel=default");
> + break;
> + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE:
> + virBufferAsprintf(&opt, ",plaintext-channel=default");
> + break;
> + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY:
> + /* nothing */
> + break;
> + }
> +
> + for (int i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) {
> + int mode = graphics->data.spice.channels[i];
> + switch (mode) {
> + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> + if (!driver->spiceTLS) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf"));
> + return 1;
goto error;
> + }
> + virBufferAsprintf(&opt, ",tls-channel=%s",
> + virDomainGraphicsSpiceChannelNameTypeToString(i));
> + break;
> + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE:
> + virBufferAsprintf(&opt, ",plaintext-channel=%s",
> + virDomainGraphicsSpiceChannelNameTypeToString(i));
> + break;
> + }
> + }
> + 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",
> + virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg));
> + 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",
> + virDomainGraphicsSpicePlaybackCompressionTypeToString(graphics->data.spice.playback));
> + if (graphics->data.spice.streaming)
> + virBufferAsprintf(&opt, ",streaming-video=%s",
> + virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming));
> + if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO)
> + virBufferAddLit(&opt, ",disable-copy-paste");
> +
> + if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) {
> + /* If qemu supports seamless migration turn it
> + * unconditionally on. If migration destination
> + * doesn't support it, it fallbacks to previous
> + * migration algorithm silently. */
> + virBufferAddLit(&opt, ",seamless-migration=on");
> + }
> +
> + virCommandAddArg(cmd, "-spice");
> + virCommandAddArgBuffer(cmd, &opt);
> + if (graphics->data.spice.keymap)
> + virCommandAddArgList(cmd, "-k",
> + graphics->data.spice.keymap, NULL);
> + /* SPICE includes native support for tunnelling audio, so we
> + * set the audio backend to point at SPICE's own driver
> + */
> + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");
> +
> + } else {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported graphics type '%s'"),
> + virDomainGraphicsTypeToString(graphics->type));
> + goto error;
> + }
> +no_memory:
> + virReportOOMError();
> +error:
> + return 0;
> +}
> +
The end of this function is wrong, it would always report no memory and return
success. The following is how it should like:
return 0;
no_memory:
virReportOOMError();
error:
return -1;
Alternatively, all ``goto error'' statements could be replaced with
``return -1'' but this way minimizes modifications to the code being moved so
I'm fine with it as-is.
> /*
> * Constructs a argv suitable for launching qemu with config defined
> * for a given virtual machine.
> @@ -5863,322 +6188,12 @@ qemuBuildCommandLine(virConnectPtr conn,
> goto error;
> }
>
> - if ((def->ngraphics == 1) &&
> - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> - virBuffer opt = VIR_BUFFER_INITIALIZER;
> -
> - if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("vnc graphics are not supported with this QEMU"));
> + for (i = 0 ; i < def->ngraphics ; ++i) {
> + if (qemuBuildGraphicsCommandLine(driver, cmd, def, caps,
> + def->graphics[i]) == -1) {
"def->graphics[i]" should be aligned with "driver" above.
> goto error;
> }
> -
> - if (def->graphics[0]->data.vnc.socket ||
> - driver->vncAutoUnixSocket) {
> -
> - if (!def->graphics[0]->data.vnc.socket &&
> - virAsprintf(&def->graphics[0]->data.vnc.socket,
> - "%s/%s.vnc", driver->libDir, def->name) == -1) {
> - goto no_memory;
> - }
> -
> - virBufferAsprintf(&opt, "unix:%s",
> - def->graphics[0]->data.vnc.socket);
> -
> - } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) {
> - const char *listenNetwork;
> - const char *listenAddr = NULL;
> - char *netAddr = NULL;
> - bool escapeAddr;
> - int ret;
> -
> - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) {
> - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
> - break;
> -
> - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0);
> - if (!listenNetwork)
> - break;
> - ret = networkGetNetworkAddress(listenNetwork, &netAddr);
> - if (ret <= -2) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - "%s", _("network-based listen not possible, "
> - "network driver not present"));
> - goto error;
> - }
> - if (ret < 0) {
> - virReportError(VIR_ERR_XML_ERROR,
> - _("listen network '%s' had no usable address"),
> - listenNetwork);
> - goto error;
> - }
> - listenAddr = netAddr;
> - /* store the address we found in the <graphics> element so it will
> - * show up in status. */
> - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0,
> - listenAddr, -1, false) < 0)
> - goto error;
> - break;
> - }
> -
> - if (!listenAddr)
> - listenAddr = driver->vncListen;
> -
> - escapeAddr = strchr(listenAddr, ':') != NULL;
> - if (escapeAddr)
> - virBufferAsprintf(&opt, "[%s]", listenAddr);
> - else
> - virBufferAdd(&opt, listenAddr, -1);
> - virBufferAsprintf(&opt, ":%d",
> - def->graphics[0]->data.vnc.port - 5900);
> -
> - VIR_FREE(netAddr);
> - } else {
> - virBufferAsprintf(&opt, "%d",
> - def->graphics[0]->data.vnc.port - 5900);
> - }
> -
> - if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) {
> - if (def->graphics[0]->data.vnc.auth.passwd ||
> - driver->vncPassword)
> - virBufferAddLit(&opt, ",password");
> -
> - if (driver->vncTLS) {
> - virBufferAddLit(&opt, ",tls");
> - if (driver->vncTLSx509verify) {
> - virBufferAsprintf(&opt, ",x509verify=%s",
> - driver->vncTLSx509certdir);
> - } else {
> - virBufferAsprintf(&opt, ",x509=%s",
> - driver->vncTLSx509certdir);
> - }
> - }
> -
> - if (driver->vncSASL) {
> - virBufferAddLit(&opt, ",sasl");
> -
> - if (driver->vncSASLdir)
> - virCommandAddEnvPair(cmd, "SASL_CONF_DIR",
> - driver->vncSASLdir);
> -
> - /* TODO: Support ACLs later */
> - }
> - }
> -
> - virCommandAddArg(cmd, "-vnc");
> - virCommandAddArgBuffer(cmd, &opt);
> - if (def->graphics[0]->data.vnc.keymap) {
> - virCommandAddArgList(cmd, "-k", def->graphics[0]->data.vnc.keymap,
> - NULL);
> - }
> -
> - /* Unless user requested it, set the audio backend to none, to
> - * prevent it opening the host OS audio devices, since that causes
> - * security issues and might not work when using VNC.
> - */
> - if (driver->vncAllowHostAudio) {
> - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
> - } else {
> - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> - }
> - } else if ((def->ngraphics == 1) &&
> - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
> - if (qemuCapsGet(caps, QEMU_CAPS_0_10) &&
> - !qemuCapsGet(caps, QEMU_CAPS_SDL)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("sdl not supported by '%s'"),
> - def->emulator);
> - goto error;
> - }
> -
> - if (def->graphics[0]->data.sdl.xauth)
> - virCommandAddEnvPair(cmd, "XAUTHORITY",
> - def->graphics[0]->data.sdl.xauth);
> - if (def->graphics[0]->data.sdl.display)
> - virCommandAddEnvPair(cmd, "DISPLAY",
> - def->graphics[0]->data.sdl.display);
> - if (def->graphics[0]->data.sdl.fullscreen)
> - virCommandAddArg(cmd, "-full-screen");
> -
> - /* If using SDL for video, then we should just let it
> - * use QEMU's host audio drivers, possibly SDL too
> - * User can set these two before starting libvirtd
> - */
> - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
> - virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
> -
> - /* New QEMU has this flag to let us explicitly ask for
> - * SDL graphics. This is better than relying on the
> - * default, since the default changes :-( */
> - if (qemuCapsGet(caps, QEMU_CAPS_SDL))
> - virCommandAddArg(cmd, "-sdl");
> -
> - } else if ((def->ngraphics == 1) &&
> - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> - virBuffer opt = VIR_BUFFER_INITIALIZER;
> - const char *listenNetwork;
> - const char *listenAddr = NULL;
> - char *netAddr = NULL;
> - int ret;
> - int defaultMode = def->graphics[0]->data.spice.defaultMode;
> -
> - if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("spice graphics are not supported with this QEMU"));
> - goto error;
> - }
> -
> - virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port);
> -
> - if (def->graphics[0]->data.spice.tlsPort > 0) {
> - if (!driver->spiceTLS) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("spice TLS port set in XML configuration,"
> - " but TLS is disabled in qemu.conf"));
> - goto error;
> - }
> - virBufferAsprintf(&opt, ",tls-port=%u",
> - def->graphics[0]->data.spice.tlsPort);
> - }
> -
> - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) {
> - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
> - break;
> -
> - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0);
> - if (!listenNetwork)
> - break;
> - ret = networkGetNetworkAddress(listenNetwork, &netAddr);
> - if (ret <= -2) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - "%s", _("network-based listen not possible, "
> - "network driver not present"));
> - goto error;
> - }
> - if (ret < 0) {
> - virReportError(VIR_ERR_XML_ERROR,
> - _("listen network '%s' had no usable address"),
> - listenNetwork);
> - goto error;
> - }
> - listenAddr = netAddr;
> - /* store the address we found in the <graphics> element so it will
> - * show up in status. */
> - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0,
> - listenAddr, -1, false) < 0)
> - goto error;
> - break;
> - }
> -
> - if (!listenAddr)
> - listenAddr = driver->spiceListen;
> - if (listenAddr)
> - virBufferAsprintf(&opt, ",addr=%s", listenAddr);
> -
> - VIR_FREE(netAddr);
> -
> - int mm = def->graphics[0]->data.spice.mousemode;
> - if (mm) {
> - switch (mm) {
> - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER:
> - virBufferAsprintf(&opt, ",agent-mouse=off");
> - break;
> - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT:
> - virBufferAsprintf(&opt, ",agent-mouse=on");
> - break;
> - default:
> - break;
> - }
> - }
> -
> - /* In the password case we set it via monitor command, to avoid
> - * making it visible on CLI, so there's no use of password=XXX
> - * in this bit of the code */
> - if (!def->graphics[0]->data.spice.auth.passwd &&
> - !driver->spicePassword)
> - virBufferAddLit(&opt, ",disable-ticketing");
> -
> - if (driver->spiceTLS)
> - virBufferAsprintf(&opt, ",x509-dir=%s",
> - driver->spiceTLSx509certdir);
> -
> - switch (defaultMode) {
> - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> - virBufferAsprintf(&opt, ",tls-channel=default");
> - break;
> - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE:
> - virBufferAsprintf(&opt, ",plaintext-channel=default");
> - break;
> - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY:
> - /* nothing */
> - break;
> - }
> -
> - for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) {
> - int mode = def->graphics[0]->data.spice.channels[i];
> - switch (mode) {
> - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> - if (!driver->spiceTLS) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf"));
> - goto error;
> - }
> - virBufferAsprintf(&opt, ",tls-channel=%s",
> - virDomainGraphicsSpiceChannelNameTypeToString(i));
> - break;
> - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE:
> - virBufferAsprintf(&opt, ",plaintext-channel=%s",
> - virDomainGraphicsSpiceChannelNameTypeToString(i));
> - break;
> - }
> - }
> - if (def->graphics[0]->data.spice.image)
> - virBufferAsprintf(&opt, ",image-compression=%s",
> - virDomainGraphicsSpiceImageCompressionTypeToString(def->graphics[0]->data.spice.image));
> - if (def->graphics[0]->data.spice.jpeg)
> - virBufferAsprintf(&opt, ",jpeg-wan-compression=%s",
> - virDomainGraphicsSpiceJpegCompressionTypeToString(def->graphics[0]->data.spice.jpeg));
> - if (def->graphics[0]->data.spice.zlib)
> - virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s",
> - virDomainGraphicsSpiceZlibCompressionTypeToString(def->graphics[0]->data.spice.zlib));
> - if (def->graphics[0]->data.spice.playback)
> - virBufferAsprintf(&opt, ",playback-compression=%s",
> - virDomainGraphicsSpicePlaybackCompressionTypeToString(def->graphics[0]->data.spice.playback));
> - if (def->graphics[0]->data.spice.streaming)
> - virBufferAsprintf(&opt, ",streaming-video=%s",
> - virDomainGraphicsSpiceStreamingModeTypeToString(def->graphics[0]->data.spice.streaming));
> - if (def->graphics[0]->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO)
> - virBufferAddLit(&opt, ",disable-copy-paste");
> -
> - if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) {
> - /* If qemu supports seamless migration turn it
> - * unconditionally on. If migration destination
> - * doesn't support it, it fallbacks to previous
> - * migration algorithm silently. */
> - virBufferAddLit(&opt, ",seamless-migration=on");
> - }
> -
> - virCommandAddArg(cmd, "-spice");
> - virCommandAddArgBuffer(cmd, &opt);
> - if (def->graphics[0]->data.spice.keymap)
> - virCommandAddArgList(cmd, "-k",
> - def->graphics[0]->data.spice.keymap, NULL);
> - /* SPICE includes native support for tunnelling audio, so we
> - * set the audio backend to point at SPICE's own driver
> - */
> - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");
> -
> - } else if ((def->ngraphics == 1)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("unsupported graphics type '%s'"),
> - virDomainGraphicsTypeToString(def->graphics[0]->type));
> - goto error;
> }
> -
> if (def->nvideos > 0) {
> if (qemuCapsGet(caps, QEMU_CAPS_VGA)) {
> if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) {
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d8cf4c3..2a77290 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2081,16 +2081,17 @@ qemuProcessInitPasswords(virConnectPtr conn,
> int ret = 0;
> qemuDomainObjPrivatePtr priv = vm->privateData;
>
> - if (vm->def->ngraphics == 1) {
> - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> + for (int i = 0 ; i < vm->def->ngraphics; ++i) {
> + virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> ret = qemuDomainChangeGraphicsPasswords(driver, vm,
> VIR_DOMAIN_GRAPHICS_TYPE_VNC,
> - &vm->def->graphics[0]->data.vnc.auth,
> + &graphics->data.vnc.auth,
> driver->vncPassword);
> - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> ret = qemuDomainChangeGraphicsPasswords(driver, vm,
> VIR_DOMAIN_GRAPHICS_TYPE_SPICE,
> - &vm->def->graphics[0]->data.spice.auth,
> + &graphics->data.spice.auth,
> driver->spicePassword);
> }
> }
> @@ -3484,21 +3485,22 @@ int qemuProcessStart(virConnectPtr conn,
> VIR_DEBUG("Ensuring no historical cgroup is lying around");
> qemuRemoveCgroup(driver, vm, 1);
>
> - if (vm->def->ngraphics == 1) {
> - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> - !vm->def->graphics[0]->data.vnc.socket &&
> - vm->def->graphics[0]->data.vnc.autoport) {
> + for (i = 0 ; i < vm->def->ngraphics; ++i) {
> + virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> + !graphics->data.vnc.socket &&
> + graphics->data.vnc.autoport) {
> int port = qemuProcessNextFreePort(driver, driver->remotePortMin);
> if (port < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("Unable to find an unused port for VNC"));
> goto cleanup;
> }
> - vm->def->graphics[0]->data.vnc.port = port;
> - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> + graphics->data.vnc.port = port;
> + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> int port = -1;
> - if (vm->def->graphics[0]->data.spice.autoport ||
> - vm->def->graphics[0]->data.spice.port == -1) {
> + if (graphics->data.spice.autoport ||
> + graphics->data.spice.port == -1) {
> port = qemuProcessNextFreePort(driver, driver->remotePortMin);
>
> if (port < 0) {
> @@ -3507,13 +3509,13 @@ int qemuProcessStart(virConnectPtr conn,
> goto cleanup;
> }
>
> - vm->def->graphics[0]->data.spice.port = port;
> + graphics->data.spice.port = port;
> }
> if (driver->spiceTLS &&
> - (vm->def->graphics[0]->data.spice.autoport ||
> - vm->def->graphics[0]->data.spice.tlsPort == -1)) {
> + (graphics->data.spice.autoport ||
> + graphics->data.spice.tlsPort == -1)) {
> int tlsPort = qemuProcessNextFreePort(driver,
> - vm->def->graphics[0]->data.spice.port + 1);
> + graphics->data.spice.port + 1);
> if (tlsPort < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("Unable to find an unused port for SPICE TLS"));
> @@ -3521,20 +3523,19 @@ int qemuProcessStart(virConnectPtr conn,
> goto cleanup;
> }
>
> - vm->def->graphics[0]->data.spice.tlsPort = tlsPort;
> + graphics->data.spice.tlsPort = tlsPort;
> }
> }
>
> - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ||
> - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> - virDomainGraphicsDefPtr graphics = vm->def->graphics[0];
> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ||
> + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> if (graphics->nListens == 0) {
> if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) {
> virReportOOMError();
> goto cleanup;
> }
> graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS;
> - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC)
> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC)
> graphics->listens[0].address = strdup(driver->vncListen);
> else
> graphics->listens[0].address = strdup(driver->spiceListen);
> @@ -4150,19 +4151,20 @@ retry:
>
> qemuProcessRemoveDomainStatus(driver, vm);
>
> - /* Remove VNC port from port reservation bitmap, but only if it was
> - reserved by the driver (autoport=yes)
> + /* Remove VNC and Spice ports from port reservation bitmap, but only if
> + they were reserved by the driver (autoport=yes)
> */
> - if ((vm->def->ngraphics == 1) &&
> - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> - vm->def->graphics[0]->data.vnc.autoport) {
> - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.vnc.port);
> - }
> - if ((vm->def->ngraphics == 1) &&
> - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
> - vm->def->graphics[0]->data.spice.autoport) {
> - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.port);
> - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.tlsPort);
> + for (i = 0 ; i < vm->def->ngraphics; ++i) {
> + virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> + graphics->data.vnc.autoport) {
> + qemuProcessReturnPort(driver, graphics->data.vnc.port);
> + }
> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
> + graphics->data.spice.autoport) {
> + qemuProcessReturnPort(driver, graphics->data.spice.port);
> + qemuProcessReturnPort(driver, graphics->data.spice.tlsPort);
> + }
> }
>
> vm->taint = 0;
I'd prefer if the refactoring of qemuBuildCommandLine was separated from
removing the limit for number of graphics cards. But since I already reviewed
the patch and I don't want to do that again, I'm giving a formal ACK to this
version (with the small issues fixed, of course).
Since you don't have commit rights, I'll fix the issues and push the patches.
Jirka
More information about the libvir-list
mailing list