[libvirt PATCH 00/16] refactor and fix graphics formatting
Pavel Hrdina (16): domain_conf: graphics: use a function to format gl element domain_conf: graphics: use a function to format audio element domain_conf: modernize graphics formatting domain_conf: graphics: extract VNC formatting to separate function domain_conf: graphics: extract SDL formatting to separate function domain_conf: graphics: extract RDP formatting to separate function domain_conf: graphics: extract Desktop formatting to separate function domain_conf: graphics: extract Spice formatting to separate function domain_conf: graphics: extract EGL-Headless formatting to separate function domain_conf: graphics: extract DBus formatting to separate function domain_conf: graphics: extract listen formatting to separate function domain_conf: graphics: move listens formatting to relevant graphics types domain_conf: graphics: move remaining spice formatting domain_conf: graphics: move remaining VNC formatting domain_conf: graphics: fix error messages when formatting XML domain_conf: graphics: properly escape user provided strings when formatting XML src/conf/domain_conf.c | 724 +++++++++++++++++++++-------------------- 1 file changed, 372 insertions(+), 352 deletions(-) -- 2.48.1
Removes code duplication. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f42b7075ad..850fe7e90b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26357,14 +26357,21 @@ virDomainGraphicsListenDefFormatAddr(virBuffer *buf, } static void -virDomainSpiceGLDefFormat(virBuffer *buf, virDomainGraphicsDef *def) +virDomainGraphicsDefFormatGL(virBuffer *buf, + virTristateBool gl, + char *rendernode) { - if (def->data.spice.gl == VIR_TRISTATE_BOOL_ABSENT) + if (gl == VIR_TRISTATE_BOOL_ABSENT && !rendernode) return; - virBufferAsprintf(buf, "<gl enable='%s'", - virTristateBoolTypeToString(def->data.spice.gl)); - virBufferEscapeString(buf, " rendernode='%s'", def->data.spice.rendernode); + virBufferAddLit(buf, "<gl"); + + if (gl != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(buf, " enable='%s'", virTristateBoolTypeToString(gl)); + + if (rendernode) + virBufferEscapeString(buf, " rendernode='%s'", rendernode); + virBufferAddLit(buf, "/>\n"); } @@ -26466,12 +26473,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, children = true; } - if (def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) { - virBufferAsprintf(buf, "<gl enable='%s'", - virTristateBoolTypeToString(def->data.sdl.gl)); - virBufferAddLit(buf, "/>\n"); - } - + virDomainGraphicsDefFormatGL(buf, def->data.sdl.gl, NULL); break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: @@ -26569,10 +26571,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, children = true; } - virBufferAddLit(buf, "<gl"); - virBufferEscapeString(buf, " rendernode='%s'", - def->data.egl_headless.rendernode); - virBufferAddLit(buf, "/>\n"); + virDomainGraphicsDefFormatGL(buf, VIR_TRISTATE_BOOL_ABSENT, + def->data.egl_headless.rendernode); break; case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: if (def->data.dbus.p2p) @@ -26590,12 +26590,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, children = true; } - if (def->data.dbus.gl) { - virBufferAsprintf(buf, "<gl enable='%s'", - virTristateBoolTypeToString(def->data.dbus.gl)); - virBufferEscapeString(buf, " rendernode='%s'", def->data.dbus.rendernode); - virBufferAddLit(buf, "/>\n"); - } + virDomainGraphicsDefFormatGL(buf, def->data.dbus.gl, def->data.dbus.rendernode); if (def->data.dbus.audioId > 0) virBufferAsprintf(buf, "<audio id='%d'/>\n", @@ -26690,7 +26685,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, virBufferAsprintf(buf, "<filetransfer enable='%s'/>\n", virTristateBoolTypeToString(def->data.spice.filetransfer)); - virDomainSpiceGLDefFormat(buf, def); + virDomainGraphicsDefFormatGL(buf, def->data.spice.gl, def->data.spice.rendernode); } if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { -- 2.48.1
Removes code duplication. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 850fe7e90b..a0e68d6046 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26375,6 +26375,16 @@ virDomainGraphicsDefFormatGL(virBuffer *buf, virBufferAddLit(buf, "/>\n"); } +static void +virDomainGraphicsDefFormatAudio(virBuffer *buf, + unsigned int audioId) +{ + if (audioId <= 0) + return; + + virBufferAsprintf(buf, "<audio id='%d'/>\n", audioId); +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26592,9 +26602,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDefFormatGL(buf, def->data.dbus.gl, def->data.dbus.rendernode); - if (def->data.dbus.audioId > 0) - virBufferAsprintf(buf, "<audio id='%d'/>\n", - def->data.dbus.audioId); + virDomainGraphicsDefFormatAudio(buf, def->data.dbus.audioId); break; case VIR_DOMAIN_GRAPHICS_TYPE_LAST: @@ -26695,9 +26703,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, children = true; } - if (def->data.vnc.audioId > 0) - virBufferAsprintf(buf, "<audio id='%d'/>\n", - def->data.vnc.audioId); + virDomainGraphicsDefFormatAudio(buf, def->data.vnc.audioId); } if (children) { -- 2.48.1
Use separate buffers for attributes and children elements to make the code cleaner and to use the virXMLFormatElement() function. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 235 ++++++++++++++++------------------------- 1 file changed, 92 insertions(+), 143 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a0e68d6046..8de39e7767 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26287,13 +26287,14 @@ virDomainGraphicsListenDefFormat(virBuffer *buf, virDomainGraphicsListenDef *def, unsigned int flags) { + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + /* If generating migratable XML, skip listen address * dragged in from config file */ if ((flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) && def->fromConfig) return; - virBufferAddLit(buf, "<listen"); - virBufferAsprintf(buf, " type='%s'", + virBufferAsprintf(&attrBuf, " type='%s'", virDomainGraphicsListenTypeToString(def->type)); if (def->address && @@ -26302,28 +26303,28 @@ virDomainGraphicsListenDefFormat(virBuffer *buf, !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)))) { /* address may also be set to show current status when type='network', * but we don't want to print that if INACTIVE data is requested. */ - virBufferAsprintf(buf, " address='%s'", def->address); + virBufferAsprintf(&attrBuf, " address='%s'", def->address); } if (def->network && (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) { - virBufferEscapeString(buf, " network='%s'", def->network); + virBufferEscapeString(&attrBuf, " network='%s'", def->network); } if (def->socket && def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && !(def->autoGenerated && (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))) { - virBufferEscapeString(buf, " socket='%s'", def->socket); + virBufferEscapeString(&attrBuf, " socket='%s'", def->socket); } if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) { - virBufferAsprintf(buf, " fromConfig='%d'", def->fromConfig); - virBufferAsprintf(buf, " autoGenerated='%s'", + virBufferAsprintf(&attrBuf, " fromConfig='%d'", def->fromConfig); + virBufferAsprintf(&attrBuf, " autoGenerated='%s'", def->autoGenerated ? "yes" : "no"); } - virBufferAddLit(buf, "/>\n"); + virXMLFormatElement(buf, "listen", &attrBuf, NULL); } @@ -26361,28 +26362,27 @@ virDomainGraphicsDefFormatGL(virBuffer *buf, virTristateBool gl, char *rendernode) { - if (gl == VIR_TRISTATE_BOOL_ABSENT && !rendernode) - return; - - virBufferAddLit(buf, "<gl"); + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; if (gl != VIR_TRISTATE_BOOL_ABSENT) - virBufferAsprintf(buf, " enable='%s'", virTristateBoolTypeToString(gl)); + virBufferAsprintf(&attrBuf, " enable='%s'", virTristateBoolTypeToString(gl)); if (rendernode) - virBufferEscapeString(buf, " rendernode='%s'", rendernode); + virBufferEscapeString(&attrBuf, " rendernode='%s'", rendernode); - virBufferAddLit(buf, "/>\n"); + virXMLFormatElement(buf, "gl", &attrBuf, NULL); } static void virDomainGraphicsDefFormatAudio(virBuffer *buf, unsigned int audioId) { - if (audioId <= 0) - return; + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - virBufferAsprintf(buf, "<audio id='%d'/>\n", audioId); + if (audioId > 0) + virBufferAsprintf(&attrBuf, " id='%d'", audioId); + + virXMLFormatElement(buf, "audio", &attrBuf, NULL); } static int @@ -26390,9 +26390,10 @@ virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, unsigned int flags) { + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); virDomainGraphicsListenDef *glisten = virDomainGraphicsGetListen(def, 0); const char *type = virDomainGraphicsTypeToString(def->type); - bool children = false; size_t i; if (!type) { @@ -26401,7 +26402,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, return -1; } - virBufferAsprintf(buf, "<graphics type='%s'", type); + virBufferAsprintf(&attrBuf, " type='%s'", type); switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: @@ -26420,7 +26421,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, if (glisten->socket && !((glisten->autoGenerated || glisten->fromConfig) && (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))) { - virBufferEscapeString(buf, " socket='%s'", glisten->socket); + virBufferEscapeString(&attrBuf, " socket='%s'", glisten->socket); } break; @@ -26428,90 +26429,84 @@ virDomainGraphicsDefFormat(virBuffer *buf, case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: if (def->data.vnc.port && (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) - virBufferAsprintf(buf, " port='%d'", + virBufferAsprintf(&attrBuf, " port='%d'", def->data.vnc.port); else if (def->data.vnc.autoport) - virBufferAddLit(buf, " port='-1'"); + virBufferAddLit(&attrBuf, " port='-1'"); - virBufferAsprintf(buf, " autoport='%s'", + virBufferAsprintf(&attrBuf, " autoport='%s'", def->data.vnc.autoport ? "yes" : "no"); if (def->data.vnc.websocketGenerated && (flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) - virBufferAddLit(buf, " websocket='-1'"); + virBufferAddLit(&attrBuf, " websocket='-1'"); else if (def->data.vnc.websocket) - virBufferAsprintf(buf, " websocket='%d'", def->data.vnc.websocket); + virBufferAsprintf(&attrBuf, " websocket='%d'", def->data.vnc.websocket); if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) - virBufferAsprintf(buf, " websocketGenerated='%s'", + virBufferAsprintf(&attrBuf, " websocketGenerated='%s'", def->data.vnc.websocketGenerated ? "yes" : "no"); - virDomainGraphicsListenDefFormatAddr(buf, glisten, flags); + virDomainGraphicsListenDefFormatAddr(&attrBuf, glisten, flags); break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: break; } - virBufferEscapeString(buf, " keymap='%s'", + virBufferEscapeString(&attrBuf, " keymap='%s'", def->data.vnc.keymap); if (def->data.vnc.sharePolicy) - virBufferAsprintf(buf, " sharePolicy='%s'", + virBufferAsprintf(&attrBuf, " sharePolicy='%s'", virDomainGraphicsVNCSharePolicyTypeToString( def->data.vnc.sharePolicy)); if (def->data.vnc.powerControl) - virBufferAsprintf(buf, " powerControl='%s'", + virBufferAsprintf(&attrBuf, " powerControl='%s'", virTristateBoolTypeToString(def->data.vnc.powerControl)); - virDomainGraphicsAuthDefFormatAttr(buf, &def->data.vnc.auth, flags); + virDomainGraphicsAuthDefFormatAttr(&attrBuf, &def->data.vnc.auth, flags); break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - virBufferEscapeString(buf, " display='%s'", + virBufferEscapeString(&attrBuf, " display='%s'", def->data.sdl.display); - virBufferEscapeString(buf, " xauth='%s'", + virBufferEscapeString(&attrBuf, " xauth='%s'", def->data.sdl.xauth); if (def->data.sdl.fullscreen) - virBufferAddLit(buf, " fullscreen='yes'"); + virBufferAddLit(&attrBuf, " fullscreen='yes'"); - if (!children && def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - children = true; - } - - virDomainGraphicsDefFormatGL(buf, def->data.sdl.gl, NULL); + virDomainGraphicsDefFormatGL(&childBuf, def->data.sdl.gl, NULL); break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: if (def->data.rdp.port) - virBufferAsprintf(buf, " port='%d'", + virBufferAsprintf(&attrBuf, " port='%d'", def->data.rdp.port); else if (def->data.rdp.autoport) - virBufferAddLit(buf, " port='0'"); + virBufferAddLit(&attrBuf, " port='0'"); if (def->data.rdp.autoport) - virBufferAddLit(buf, " autoport='yes'"); + virBufferAddLit(&attrBuf, " autoport='yes'"); if (def->data.rdp.replaceUser) - virBufferAddLit(buf, " replaceUser='yes'"); + virBufferAddLit(&attrBuf, " replaceUser='yes'"); if (def->data.rdp.multiUser) - virBufferAddLit(buf, " multiUser='yes'"); + virBufferAddLit(&attrBuf, " multiUser='yes'"); - virDomainGraphicsListenDefFormatAddr(buf, glisten, flags); + virDomainGraphicsListenDefFormatAddr(&attrBuf, glisten, flags); break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - virBufferEscapeString(buf, " display='%s'", + virBufferEscapeString(&attrBuf, " display='%s'", def->data.desktop.display); if (def->data.desktop.fullscreen) - virBufferAddLit(buf, " fullscreen='yes'"); + virBufferAddLit(&attrBuf, " fullscreen='yes'"); break; @@ -26526,22 +26521,22 @@ virDomainGraphicsDefFormat(virBuffer *buf, case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: if (def->data.spice.port) - virBufferAsprintf(buf, " port='%d'", + virBufferAsprintf(&attrBuf, " port='%d'", def->data.spice.port); if (def->data.spice.tlsPort) - virBufferAsprintf(buf, " tlsPort='%d'", + virBufferAsprintf(&attrBuf, " tlsPort='%d'", def->data.spice.tlsPort); - virBufferAsprintf(buf, " autoport='%s'", + virBufferAsprintf(&attrBuf, " autoport='%s'", def->data.spice.autoport ? "yes" : "no"); - virDomainGraphicsListenDefFormatAddr(buf, glisten, flags); + virDomainGraphicsListenDefFormatAddr(&attrBuf, glisten, flags); break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) - virBufferAddLit(buf, " autoport='no'"); + virBufferAddLit(&attrBuf, " autoport='no'"); break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: @@ -26553,7 +26548,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, * parsed as listen type "none". */ if ((flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) && glisten->fromConfig) { - virBufferAddLit(buf, " autoport='yes'"); + virBufferAddLit(&attrBuf, " autoport='yes'"); } break; @@ -26561,48 +26556,31 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - virBufferEscapeString(buf, " keymap='%s'", + virBufferEscapeString(&attrBuf, " keymap='%s'", def->data.spice.keymap); if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) - virBufferAsprintf(buf, " defaultMode='%s'", + virBufferAsprintf(&attrBuf, " defaultMode='%s'", virDomainGraphicsSpiceChannelModeTypeToString(def->data.spice.defaultMode)); - virDomainGraphicsAuthDefFormatAttr(buf, &def->data.spice.auth, flags); + virDomainGraphicsAuthDefFormatAttr(&attrBuf, &def->data.spice.auth, flags); break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: - if (!def->data.egl_headless.rendernode) - break; - - if (!children) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - children = true; - } - - virDomainGraphicsDefFormatGL(buf, VIR_TRISTATE_BOOL_ABSENT, + virDomainGraphicsDefFormatGL(&childBuf, VIR_TRISTATE_BOOL_ABSENT, def->data.egl_headless.rendernode); break; case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: if (def->data.dbus.p2p) - virBufferAddLit(buf, " p2p='yes'"); + virBufferAddLit(&attrBuf, " p2p='yes'"); if (def->data.dbus.address) - virBufferAsprintf(buf, " address='%s'", + virBufferAsprintf(&attrBuf, " address='%s'", def->data.dbus.address); - if (!def->data.dbus.gl && def->data.dbus.audioId <= 0) - break; + virDomainGraphicsDefFormatGL(&childBuf, def->data.dbus.gl, + def->data.dbus.rendernode); - if (!children) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - children = true; - } - - virDomainGraphicsDefFormatGL(buf, def->data.dbus.gl, def->data.dbus.rendernode); - - virDomainGraphicsDefFormatAudio(buf, def->data.dbus.audioId); + virDomainGraphicsDefFormatAudio(&childBuf, def->data.dbus.audioId); break; case VIR_DOMAIN_GRAPHICS_TYPE_LAST: @@ -26635,83 +26613,54 @@ virDomainGraphicsDefFormat(virBuffer *buf, def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE) continue; } - if (!children) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - children = true; - } - virDomainGraphicsListenDefFormat(buf, &def->listens[i], flags); + virDomainGraphicsListenDefFormat(&childBuf, &def->listens[i], flags); } if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + g_auto(virBuffer) spiceBuf = VIR_BUFFER_INITIALIZER; + for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { int mode = def->data.spice.channels[i]; if (mode == VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) continue; - if (!children) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - children = true; - } - - virBufferAsprintf(buf, "<channel name='%s' mode='%s'/>\n", + virBufferAsprintf(&spiceBuf, " name='%s' mode='%s'", virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); + + virXMLFormatElement(&childBuf, "channel", &spiceBuf, NULL); } - if (!children && (def->data.spice.image || def->data.spice.jpeg || - def->data.spice.zlib || def->data.spice.playback || - def->data.spice.streaming || def->data.spice.copypaste || - def->data.spice.mousemode || def->data.spice.filetransfer || - def->data.spice.gl)) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - children = true; - } - if (def->data.spice.image) - virBufferAsprintf(buf, "<image compression='%s'/>\n", - virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image)); - if (def->data.spice.jpeg) - virBufferAsprintf(buf, "<jpeg compression='%s'/>\n", - virDomainGraphicsSpiceJpegCompressionTypeToString(def->data.spice.jpeg)); - if (def->data.spice.zlib) - virBufferAsprintf(buf, "<zlib compression='%s'/>\n", - virDomainGraphicsSpiceZlibCompressionTypeToString(def->data.spice.zlib)); - if (def->data.spice.playback) - virBufferAsprintf(buf, "<playback compression='%s'/>\n", - virTristateSwitchTypeToString(def->data.spice.playback)); - if (def->data.spice.streaming) - virBufferAsprintf(buf, "<streaming mode='%s'/>\n", - virDomainGraphicsSpiceStreamingModeTypeToString(def->data.spice.streaming)); - if (def->data.spice.mousemode) - virBufferAsprintf(buf, "<mouse mode='%s'/>\n", - virDomainMouseModeTypeToString(def->data.spice.mousemode)); - if (def->data.spice.copypaste) - virBufferAsprintf(buf, "<clipboard copypaste='%s'/>\n", - virTristateBoolTypeToString(def->data.spice.copypaste)); - if (def->data.spice.filetransfer) - virBufferAsprintf(buf, "<filetransfer enable='%s'/>\n", - virTristateBoolTypeToString(def->data.spice.filetransfer)); - virDomainGraphicsDefFormatGL(buf, def->data.spice.gl, def->data.spice.rendernode); - } +#define FORMAT_SPICE_FEATURE(name, attr, value, toStringFunc) \ + if (value) { \ + virBufferAsprintf(&spiceBuf, " " attr "='%s'", toStringFunc(value)); \ + } \ + virXMLFormatElement(&childBuf, name, &spiceBuf, NULL); - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - if (!children) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - children = true; - } + FORMAT_SPICE_FEATURE("image", "compression", def->data.spice.image, + virDomainGraphicsSpiceImageCompressionTypeToString); + FORMAT_SPICE_FEATURE("jpeg", "compression", def->data.spice.jpeg, + virDomainGraphicsSpiceJpegCompressionTypeToString); + FORMAT_SPICE_FEATURE("zlib", "compression", def->data.spice.zlib, + virDomainGraphicsSpiceZlibCompressionTypeToString); + FORMAT_SPICE_FEATURE("playback", "compression", def->data.spice.playback, + virTristateSwitchTypeToString); + FORMAT_SPICE_FEATURE("streaming", "mode", def->data.spice.streaming, + virDomainGraphicsSpiceStreamingModeTypeToString); + FORMAT_SPICE_FEATURE("mouse", "mode", def->data.spice.mousemode, + virDomainMouseModeTypeToString); + FORMAT_SPICE_FEATURE("clipboard", "copypaste", def->data.spice.copypaste, + virTristateBoolTypeToString); + FORMAT_SPICE_FEATURE("filetransfer", "enable", def->data.spice.filetransfer, + virTristateBoolTypeToString); - virDomainGraphicsDefFormatAudio(buf, def->data.vnc.audioId); + virDomainGraphicsDefFormatGL(&childBuf, def->data.spice.gl, def->data.spice.rendernode); } - if (children) { - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</graphics>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) + virDomainGraphicsDefFormatAudio(&childBuf, def->data.vnc.audioId); + + virXMLFormatElement(buf, "graphics", &attrBuf, &childBuf); return 0; } -- 2.48.1
virDomainGraphicsDefFormat function was way too long so split it into separate functions for each graphics type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 131 ++++++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 60 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8de39e7767..e1589527f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26385,6 +26385,76 @@ virDomainGraphicsDefFormatAudio(virBuffer *buf, virXMLFormatElement(buf, "audio", &attrBuf, NULL); } +static int +virDomainGraphicsDefFormatVNC(virBuffer *attrBuf, + virDomainGraphicsDef *def, + unsigned int flags) +{ + virDomainGraphicsListenDef *glisten = virDomainGraphicsGetListen(def, 0); + + if (!glisten) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element for graphics")); + return -1; + } + + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + /* To not break migration we shouldn't print the 'socket' attribute + * if it's auto-generated or if it's based on config option from + * qemu.conf. If the socket is provided by user we need to print it + * into migratable XML. */ + if (glisten->socket && + !((glisten->autoGenerated || glisten->fromConfig) && + (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))) { + virBufferEscapeString(attrBuf, " socket='%s'", glisten->socket); + } + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (def->data.vnc.port && + (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) + virBufferAsprintf(attrBuf, " port='%d'", def->data.vnc.port); + else if (def->data.vnc.autoport) + virBufferAddLit(attrBuf, " port='-1'"); + + virBufferAsprintf(attrBuf, " autoport='%s'", + def->data.vnc.autoport ? "yes" : "no"); + + if (def->data.vnc.websocketGenerated && + (flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) + virBufferAddLit(attrBuf, " websocket='-1'"); + else if (def->data.vnc.websocket) + virBufferAsprintf(attrBuf, " websocket='%d'", def->data.vnc.websocket); + + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + virBufferAsprintf(attrBuf, " websocketGenerated='%s'", + def->data.vnc.websocketGenerated ? "yes" : "no"); + + virDomainGraphicsListenDefFormatAddr(attrBuf, glisten, flags); + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } + + virBufferEscapeString(attrBuf, " keymap='%s'", def->data.vnc.keymap); + + if (def->data.vnc.sharePolicy) + virBufferAsprintf(attrBuf, " sharePolicy='%s'", + virDomainGraphicsVNCSharePolicyTypeToString( + def->data.vnc.sharePolicy)); + + if (def->data.vnc.powerControl) + virBufferAsprintf(attrBuf, " powerControl='%s'", + virTristateBoolTypeToString(def->data.vnc.powerControl)); + + virDomainGraphicsAuthDefFormatAttr(attrBuf, &def->data.vnc.auth, flags); + + return 0; +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26406,67 +26476,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if (!glisten) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing listen element for graphics")); + if (virDomainGraphicsDefFormatVNC(&attrBuf, def, flags) < 0) return -1; - } - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: - /* To not break migration we shouldn't print the 'socket' attribute - * if it's auto-generated or if it's based on config option from - * qemu.conf. If the socket is provided by user we need to print it - * into migratable XML. */ - if (glisten->socket && - !((glisten->autoGenerated || glisten->fromConfig) && - (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))) { - virBufferEscapeString(&attrBuf, " socket='%s'", glisten->socket); - } - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (def->data.vnc.port && - (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) - virBufferAsprintf(&attrBuf, " port='%d'", - def->data.vnc.port); - else if (def->data.vnc.autoport) - virBufferAddLit(&attrBuf, " port='-1'"); - - virBufferAsprintf(&attrBuf, " autoport='%s'", - def->data.vnc.autoport ? "yes" : "no"); - - if (def->data.vnc.websocketGenerated && - (flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) - virBufferAddLit(&attrBuf, " websocket='-1'"); - else if (def->data.vnc.websocket) - virBufferAsprintf(&attrBuf, " websocket='%d'", def->data.vnc.websocket); - - if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) - virBufferAsprintf(&attrBuf, " websocketGenerated='%s'", - def->data.vnc.websocketGenerated ? "yes" : "no"); - - virDomainGraphicsListenDefFormatAddr(&attrBuf, glisten, flags); - break; - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } - - virBufferEscapeString(&attrBuf, " keymap='%s'", - def->data.vnc.keymap); - - if (def->data.vnc.sharePolicy) - virBufferAsprintf(&attrBuf, " sharePolicy='%s'", - virDomainGraphicsVNCSharePolicyTypeToString( - def->data.vnc.sharePolicy)); - - if (def->data.vnc.powerControl) - virBufferAsprintf(&attrBuf, " powerControl='%s'", - virTristateBoolTypeToString(def->data.vnc.powerControl)); - - virDomainGraphicsAuthDefFormatAttr(&attrBuf, &def->data.vnc.auth, flags); break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: -- 2.48.1
virDomainGraphicsDefFormat function was way too long so split it into separate functions for each graphics type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e1589527f2..4314ef4dc4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26455,6 +26455,21 @@ virDomainGraphicsDefFormatVNC(virBuffer *attrBuf, return 0; } +static void +virDomainGraphicsDefFormatSDL(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainGraphicsDef *def) +{ + virBufferEscapeString(attrBuf, " display='%s'", def->data.sdl.display); + + virBufferEscapeString(attrBuf, " xauth='%s'", def->data.sdl.xauth); + + if (def->data.sdl.fullscreen) + virBufferAddLit(attrBuf, " fullscreen='yes'"); + + virDomainGraphicsDefFormatGL(childBuf, def->data.sdl.gl, NULL); +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26481,15 +26496,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - virBufferEscapeString(&attrBuf, " display='%s'", - def->data.sdl.display); - - virBufferEscapeString(&attrBuf, " xauth='%s'", - def->data.sdl.xauth); - if (def->data.sdl.fullscreen) - virBufferAddLit(&attrBuf, " fullscreen='yes'"); - - virDomainGraphicsDefFormatGL(&childBuf, def->data.sdl.gl, NULL); + virDomainGraphicsDefFormatSDL(&attrBuf, &childBuf, def); break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: -- 2.48.1
virDomainGraphicsDefFormat function was way too long so split it into separate functions for each graphics type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4314ef4dc4..9400206e8f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26470,6 +26470,30 @@ virDomainGraphicsDefFormatSDL(virBuffer *attrBuf, virDomainGraphicsDefFormatGL(childBuf, def->data.sdl.gl, NULL); } +static void +virDomainGraphicsDefFormatRDP(virBuffer *attrBuf, + virDomainGraphicsDef *def, + unsigned int flags) +{ + virDomainGraphicsListenDef *glisten = virDomainGraphicsGetListen(def, 0); + + if (def->data.rdp.port) + virBufferAsprintf(attrBuf, " port='%d'", def->data.rdp.port); + else if (def->data.rdp.autoport) + virBufferAddLit(attrBuf, " port='0'"); + + if (def->data.rdp.autoport) + virBufferAddLit(attrBuf, " autoport='yes'"); + + if (def->data.rdp.replaceUser) + virBufferAddLit(attrBuf, " replaceUser='yes'"); + + if (def->data.rdp.multiUser) + virBufferAddLit(attrBuf, " multiUser='yes'"); + + virDomainGraphicsListenDefFormatAddr(attrBuf, glisten, flags); +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26500,23 +26524,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - if (def->data.rdp.port) - virBufferAsprintf(&attrBuf, " port='%d'", - def->data.rdp.port); - else if (def->data.rdp.autoport) - virBufferAddLit(&attrBuf, " port='0'"); - - if (def->data.rdp.autoport) - virBufferAddLit(&attrBuf, " autoport='yes'"); - - if (def->data.rdp.replaceUser) - virBufferAddLit(&attrBuf, " replaceUser='yes'"); - - if (def->data.rdp.multiUser) - virBufferAddLit(&attrBuf, " multiUser='yes'"); - - virDomainGraphicsListenDefFormatAddr(&attrBuf, glisten, flags); - + virDomainGraphicsDefFormatRDP(&attrBuf, def, flags); break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: -- 2.48.1
virDomainGraphicsDefFormat function was way too long so split it into separate functions for each graphics type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9400206e8f..3edeffbe72 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26494,6 +26494,16 @@ virDomainGraphicsDefFormatRDP(virBuffer *attrBuf, virDomainGraphicsListenDefFormatAddr(attrBuf, glisten, flags); } +static void +virDomainGraphicsDefFormatDesktop(virBuffer *attrBuf, + virDomainGraphicsDef *def) +{ + virBufferEscapeString(attrBuf, " display='%s'", def->data.desktop.display); + + if (def->data.desktop.fullscreen) + virBufferAddLit(attrBuf, " fullscreen='yes'"); +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26528,12 +26538,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - virBufferEscapeString(&attrBuf, " display='%s'", - def->data.desktop.display); - - if (def->data.desktop.fullscreen) - virBufferAddLit(&attrBuf, " fullscreen='yes'"); - + virDomainGraphicsDefFormatDesktop(&attrBuf, def); break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: -- 2.48.1
virDomainGraphicsDefFormat function was way too long so split it into separate functions for each graphics type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 115 ++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 53 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3edeffbe72..636d4a5099 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26504,6 +26504,67 @@ virDomainGraphicsDefFormatDesktop(virBuffer *attrBuf, virBufferAddLit(attrBuf, " fullscreen='yes'"); } +static int +virDomainGraphicsDefFormatSpice(virBuffer *attrBuf, + virDomainGraphicsDef *def, + unsigned int flags) +{ + virDomainGraphicsListenDef *glisten = virDomainGraphicsGetListen(def, 0); + + if (!glisten) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element for spice graphics")); + return -1; + } + + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (def->data.spice.port) + virBufferAsprintf(attrBuf, " port='%d'", def->data.spice.port); + + if (def->data.spice.tlsPort) + virBufferAsprintf(attrBuf, " tlsPort='%d'", def->data.spice.tlsPort); + + virBufferAsprintf(attrBuf, " autoport='%s'", + def->data.spice.autoport ? "yes" : "no"); + + virDomainGraphicsListenDefFormatAddr(attrBuf, glisten, flags); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) + virBufferAddLit(attrBuf, " autoport='no'"); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + /* If socket is auto-generated based on config option we don't + * add any listen element into migratable XML because the original + * listen type is "address". + * We need to set autoport to make sure that libvirt on destination + * will parse it as listen type "address", without autoport it is + * parsed as listen type "none". */ + if ((flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) && + glisten->fromConfig) { + virBufferAddLit(attrBuf, " autoport='yes'"); + } + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } + + virBufferEscapeString(attrBuf, " keymap='%s'", def->data.spice.keymap); + + if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) + virBufferAsprintf(attrBuf, " defaultMode='%s'", + virDomainGraphicsSpiceChannelModeTypeToString(def->data.spice.defaultMode)); + + virDomainGraphicsAuthDefFormatAttr(attrBuf, &def->data.spice.auth, flags); + + return 0; +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26511,7 +26572,6 @@ virDomainGraphicsDefFormat(virBuffer *buf, { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); - virDomainGraphicsListenDef *glisten = virDomainGraphicsGetListen(def, 0); const char *type = virDomainGraphicsTypeToString(def->type); size_t i; @@ -26542,59 +26602,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (!glisten) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing listen element for spice graphics")); + if (virDomainGraphicsDefFormatSpice(&attrBuf, def, flags) < 0) return -1; - } - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (def->data.spice.port) - virBufferAsprintf(&attrBuf, " port='%d'", - def->data.spice.port); - - if (def->data.spice.tlsPort) - virBufferAsprintf(&attrBuf, " tlsPort='%d'", - def->data.spice.tlsPort); - - virBufferAsprintf(&attrBuf, " autoport='%s'", - def->data.spice.autoport ? "yes" : "no"); - - virDomainGraphicsListenDefFormatAddr(&attrBuf, glisten, flags); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) - virBufferAddLit(&attrBuf, " autoport='no'"); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: - /* If socket is auto-generated based on config option we don't - * add any listen element into migratable XML because the original - * listen type is "address". - * We need to set autoport to make sure that libvirt on destination - * will parse it as listen type "address", without autoport it is - * parsed as listen type "none". */ - if ((flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) && - glisten->fromConfig) { - virBufferAddLit(&attrBuf, " autoport='yes'"); - } - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } - - virBufferEscapeString(&attrBuf, " keymap='%s'", - def->data.spice.keymap); - - if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) - virBufferAsprintf(&attrBuf, " defaultMode='%s'", - virDomainGraphicsSpiceChannelModeTypeToString(def->data.spice.defaultMode)); - - virDomainGraphicsAuthDefFormatAttr(&attrBuf, &def->data.spice.auth, flags); break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: -- 2.48.1
virDomainGraphicsDefFormat function was way too long so split it into separate functions for each graphics type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 636d4a5099..83dccd87f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26565,6 +26565,14 @@ virDomainGraphicsDefFormatSpice(virBuffer *attrBuf, return 0; } +static void +virDomainGraphicsDefFormatEGLHeadless(virBuffer *childBuf, + virDomainGraphicsDef *def) +{ + virDomainGraphicsDefFormatGL(childBuf, VIR_TRISTATE_BOOL_ABSENT, + def->data.egl_headless.rendernode); +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26607,9 +26615,9 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: - virDomainGraphicsDefFormatGL(&childBuf, VIR_TRISTATE_BOOL_ABSENT, - def->data.egl_headless.rendernode); + virDomainGraphicsDefFormatEGLHeadless(&childBuf, def); break; + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: if (def->data.dbus.p2p) virBufferAddLit(&attrBuf, " p2p='yes'"); -- 2.48.1
virDomainGraphicsDefFormat function was way too long so split it into separate functions for each graphics type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 83dccd87f1..81068570a0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26573,6 +26573,23 @@ virDomainGraphicsDefFormatEGLHeadless(virBuffer *childBuf, def->data.egl_headless.rendernode); } +static void +virDomainGraphicsDefFormatDBus(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainGraphicsDef *def) +{ + if (def->data.dbus.p2p) + virBufferAddLit(attrBuf, " p2p='yes'"); + + if (def->data.dbus.address) + virBufferAsprintf(attrBuf, " address='%s'", def->data.dbus.address); + + virDomainGraphicsDefFormatGL(childBuf, def->data.dbus.gl, + def->data.dbus.rendernode); + + virDomainGraphicsDefFormatAudio(childBuf, def->data.dbus.audioId); +} + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -26619,18 +26636,9 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: - if (def->data.dbus.p2p) - virBufferAddLit(&attrBuf, " p2p='yes'"); - if (def->data.dbus.address) - virBufferAsprintf(&attrBuf, " address='%s'", - def->data.dbus.address); - - virDomainGraphicsDefFormatGL(&childBuf, def->data.dbus.gl, - def->data.dbus.rendernode); - - virDomainGraphicsDefFormatAudio(&childBuf, def->data.dbus.audioId); - + virDomainGraphicsDefFormatDBus(&attrBuf, &childBuf, def); break; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } -- 2.48.1
This will be used in specific graphics types that are using listen elements. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 67 ++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 81068570a0..d39b885101 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26328,6 +26328,44 @@ virDomainGraphicsListenDefFormat(virBuffer *buf, } +static void +virDomainGraphicsDefFormatListnes(virBuffer *childBuf, + virDomainGraphicsDef *def, + unsigned int flags) +{ + size_t i; + + for (i = 0; i < def->nListens; i++) { + if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) { + /* If the listen is based on config options from qemu.conf we need + * to skip it. It's up to user to properly configure both hosts for + * migration. */ + if (def->listens[i].fromConfig) + continue; + + /* If the socket is provided by user in the XML we need to skip this + * listen type to support migration back to old libvirt since old + * libvirt supports specifying socket path inside graphics element + * as 'socket' attribute. Auto-generated socket is a new feature + * thus we can generate it in the migrateble XML. */ + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && + def->listens[i].socket && + !def->listens[i].autoGenerated) + continue; + + /* The new listen type none is in the migratable XML represented as + * port=0 and autoport=no because old libvirt support this + * configuration for spice. */ + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE) + continue; + } + virDomainGraphicsListenDefFormat(childBuf, &def->listens[i], flags); + } +} + + /** * virDomainGraphicsListenDefFormatAddr: * @buf: buffer where the output XML is written @@ -26643,34 +26681,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - for (i = 0; i < def->nListens; i++) { - if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) { - /* If the listen is based on config options from qemu.conf we need - * to skip it. It's up to user to properly configure both hosts for - * migration. */ - if (def->listens[i].fromConfig) - continue; - - /* If the socket is provided by user in the XML we need to skip this - * listen type to support migration back to old libvirt since old - * libvirt supports specifying socket path inside graphics element - * as 'socket' attribute. Auto-generated socket is a new feature - * thus we can generate it in the migrateble XML. */ - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && - def->listens[i].socket && - !def->listens[i].autoGenerated) - continue; - - /* The new listen type none is in the migratable XML represented as - * port=0 and autoport=no because old libvirt support this - * configuration for spice. */ - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE) - continue; - } - virDomainGraphicsListenDefFormat(&childBuf, &def->listens[i], flags); - } + virDomainGraphicsDefFormatListnes(&childBuf, def, flags); if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { g_auto(virBuffer) spiceBuf = VIR_BUFFER_INITIALIZER; -- 2.48.1
Only VNC, RDP and Spice graphics types are using listen elements so call the function only where it is needed. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d39b885101..ad3a44b9b1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26425,6 +26425,7 @@ virDomainGraphicsDefFormatAudio(virBuffer *buf, static int virDomainGraphicsDefFormatVNC(virBuffer *attrBuf, + virBuffer *childBuf, virDomainGraphicsDef *def, unsigned int flags) { @@ -26490,6 +26491,8 @@ virDomainGraphicsDefFormatVNC(virBuffer *attrBuf, virDomainGraphicsAuthDefFormatAttr(attrBuf, &def->data.vnc.auth, flags); + virDomainGraphicsDefFormatListnes(childBuf, def, flags); + return 0; } @@ -26510,6 +26513,7 @@ virDomainGraphicsDefFormatSDL(virBuffer *attrBuf, static void virDomainGraphicsDefFormatRDP(virBuffer *attrBuf, + virBuffer *childBuf, virDomainGraphicsDef *def, unsigned int flags) { @@ -26530,6 +26534,8 @@ virDomainGraphicsDefFormatRDP(virBuffer *attrBuf, virBufferAddLit(attrBuf, " multiUser='yes'"); virDomainGraphicsListenDefFormatAddr(attrBuf, glisten, flags); + + virDomainGraphicsDefFormatListnes(childBuf, def, flags); } static void @@ -26544,6 +26550,7 @@ virDomainGraphicsDefFormatDesktop(virBuffer *attrBuf, static int virDomainGraphicsDefFormatSpice(virBuffer *attrBuf, + virBuffer *childBuf, virDomainGraphicsDef *def, unsigned int flags) { @@ -26600,6 +26607,8 @@ virDomainGraphicsDefFormatSpice(virBuffer *attrBuf, virDomainGraphicsAuthDefFormatAttr(attrBuf, &def->data.spice.auth, flags); + virDomainGraphicsDefFormatListnes(childBuf, def, flags); + return 0; } @@ -26648,7 +26657,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if (virDomainGraphicsDefFormatVNC(&attrBuf, def, flags) < 0) + if (virDomainGraphicsDefFormatVNC(&attrBuf, &childBuf, def, flags) < 0) return -1; break; @@ -26657,7 +26666,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - virDomainGraphicsDefFormatRDP(&attrBuf, def, flags); + virDomainGraphicsDefFormatRDP(&attrBuf, &childBuf, def, flags); break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: @@ -26665,7 +26674,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (virDomainGraphicsDefFormatSpice(&attrBuf, def, flags) < 0) + if (virDomainGraphicsDefFormatSpice(&attrBuf, &childBuf, def, flags) < 0) return -1; break; @@ -26681,8 +26690,6 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - virDomainGraphicsDefFormatListnes(&childBuf, def, flags); - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { g_auto(virBuffer) spiceBuf = VIR_BUFFER_INITIALIZER; -- 2.48.1
Now we are able to move the rest into virDomainGraphicsDefFormatSpice without breaking order of elements in the resulting XML. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 81 ++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ad3a44b9b1..34ed62cc99 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26554,7 +26554,9 @@ virDomainGraphicsDefFormatSpice(virBuffer *attrBuf, virDomainGraphicsDef *def, unsigned int flags) { + g_auto(virBuffer) spiceBuf = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDef *glisten = virDomainGraphicsGetListen(def, 0); + size_t i; if (!glisten) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -26609,6 +26611,43 @@ virDomainGraphicsDefFormatSpice(virBuffer *attrBuf, virDomainGraphicsDefFormatListnes(childBuf, def, flags); + for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { + int mode = def->data.spice.channels[i]; + if (mode == VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) + continue; + + virBufferAsprintf(&spiceBuf, " name='%s' mode='%s'", + virDomainGraphicsSpiceChannelNameTypeToString(i), + virDomainGraphicsSpiceChannelModeTypeToString(mode)); + + virXMLFormatElement(childBuf, "channel", &spiceBuf, NULL); + } + +#define FORMAT_SPICE_FEATURE(name, attr, value, toStringFunc) \ + if (value) { \ + virBufferAsprintf(&spiceBuf, " " attr "='%s'", toStringFunc(value)); \ + } \ + virXMLFormatElement(childBuf, name, &spiceBuf, NULL); + + FORMAT_SPICE_FEATURE("image", "compression", def->data.spice.image, + virDomainGraphicsSpiceImageCompressionTypeToString); + FORMAT_SPICE_FEATURE("jpeg", "compression", def->data.spice.jpeg, + virDomainGraphicsSpiceJpegCompressionTypeToString); + FORMAT_SPICE_FEATURE("zlib", "compression", def->data.spice.zlib, + virDomainGraphicsSpiceZlibCompressionTypeToString); + FORMAT_SPICE_FEATURE("playback", "compression", def->data.spice.playback, + virTristateSwitchTypeToString); + FORMAT_SPICE_FEATURE("streaming", "mode", def->data.spice.streaming, + virDomainGraphicsSpiceStreamingModeTypeToString); + FORMAT_SPICE_FEATURE("mouse", "mode", def->data.spice.mousemode, + virDomainMouseModeTypeToString); + FORMAT_SPICE_FEATURE("clipboard", "copypaste", def->data.spice.copypaste, + virTristateBoolTypeToString); + FORMAT_SPICE_FEATURE("filetransfer", "enable", def->data.spice.filetransfer, + virTristateBoolTypeToString); + + virDomainGraphicsDefFormatGL(childBuf, def->data.spice.gl, def->data.spice.rendernode); + return 0; } @@ -26645,7 +26684,6 @@ virDomainGraphicsDefFormat(virBuffer *buf, g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); const char *type = virDomainGraphicsTypeToString(def->type); - size_t i; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -26690,47 +26728,6 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - g_auto(virBuffer) spiceBuf = VIR_BUFFER_INITIALIZER; - - for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { - int mode = def->data.spice.channels[i]; - if (mode == VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) - continue; - - virBufferAsprintf(&spiceBuf, " name='%s' mode='%s'", - virDomainGraphicsSpiceChannelNameTypeToString(i), - virDomainGraphicsSpiceChannelModeTypeToString(mode)); - - virXMLFormatElement(&childBuf, "channel", &spiceBuf, NULL); - } - -#define FORMAT_SPICE_FEATURE(name, attr, value, toStringFunc) \ - if (value) { \ - virBufferAsprintf(&spiceBuf, " " attr "='%s'", toStringFunc(value)); \ - } \ - virXMLFormatElement(&childBuf, name, &spiceBuf, NULL); - - FORMAT_SPICE_FEATURE("image", "compression", def->data.spice.image, - virDomainGraphicsSpiceImageCompressionTypeToString); - FORMAT_SPICE_FEATURE("jpeg", "compression", def->data.spice.jpeg, - virDomainGraphicsSpiceJpegCompressionTypeToString); - FORMAT_SPICE_FEATURE("zlib", "compression", def->data.spice.zlib, - virDomainGraphicsSpiceZlibCompressionTypeToString); - FORMAT_SPICE_FEATURE("playback", "compression", def->data.spice.playback, - virTristateSwitchTypeToString); - FORMAT_SPICE_FEATURE("streaming", "mode", def->data.spice.streaming, - virDomainGraphicsSpiceStreamingModeTypeToString); - FORMAT_SPICE_FEATURE("mouse", "mode", def->data.spice.mousemode, - virDomainMouseModeTypeToString); - FORMAT_SPICE_FEATURE("clipboard", "copypaste", def->data.spice.copypaste, - virTristateBoolTypeToString); - FORMAT_SPICE_FEATURE("filetransfer", "enable", def->data.spice.filetransfer, - virTristateBoolTypeToString); - - virDomainGraphicsDefFormatGL(&childBuf, def->data.spice.gl, def->data.spice.rendernode); - } - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) virDomainGraphicsDefFormatAudio(&childBuf, def->data.vnc.audioId); -- 2.48.1
Now we are able to move the rest into virDomainGraphicsDefFormatVNC without breaking order of elements in the resulting XML. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 34ed62cc99..27323ea768 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26493,6 +26493,8 @@ virDomainGraphicsDefFormatVNC(virBuffer *attrBuf, virDomainGraphicsDefFormatListnes(childBuf, def, flags); + virDomainGraphicsDefFormatAudio(childBuf, def->data.vnc.audioId); + return 0; } @@ -26728,9 +26730,6 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) - virDomainGraphicsDefFormatAudio(&childBuf, def->data.vnc.audioId); - virXMLFormatElement(buf, "graphics", &attrBuf, &childBuf); return 0; -- 2.48.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 27323ea768..6bb30c6f22 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26433,7 +26433,7 @@ virDomainGraphicsDefFormatVNC(virBuffer *attrBuf, if (!glisten) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing listen element for graphics")); + _("missing listen element for VNC graphics")); return -1; } @@ -26689,7 +26689,7 @@ virDomainGraphicsDefFormat(virBuffer *buf, if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected net type %1$d"), def->type); + _("unexpected graphics type '%1$d'"), def->type); return -1; } -- 2.48.1
This was reported on virt-manager issue tracker as it was possible to provide `listen` attribute with properly escaped characters but libvirt would format XML without escaping it. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6bb30c6f22..7ce49993b9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26303,7 +26303,7 @@ virDomainGraphicsListenDefFormat(virBuffer *buf, !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)))) { /* address may also be set to show current status when type='network', * but we don't want to print that if INACTIVE data is requested. */ - virBufferAsprintf(&attrBuf, " address='%s'", def->address); + virBufferEscapeString(&attrBuf, " address='%s'", def->address); } if (def->network && @@ -26392,7 +26392,7 @@ virDomainGraphicsListenDefFormatAddr(virBuffer *buf, return; if (glisten->address) - virBufferAsprintf(buf, " listen='%s'", glisten->address); + virBufferEscapeString(buf, " listen='%s'", glisten->address); } static void -- 2.48.1
On a Thursday in 2025, Pavel Hrdina wrote:
Pavel Hrdina (16): domain_conf: graphics: use a function to format gl element domain_conf: graphics: use a function to format audio element domain_conf: modernize graphics formatting domain_conf: graphics: extract VNC formatting to separate function domain_conf: graphics: extract SDL formatting to separate function domain_conf: graphics: extract RDP formatting to separate function domain_conf: graphics: extract Desktop formatting to separate function domain_conf: graphics: extract Spice formatting to separate function domain_conf: graphics: extract EGL-Headless formatting to separate function domain_conf: graphics: extract DBus formatting to separate function domain_conf: graphics: extract listen formatting to separate function domain_conf: graphics: move listens formatting to relevant graphics types domain_conf: graphics: move remaining spice formatting domain_conf: graphics: move remaining VNC formatting domain_conf: graphics: fix error messages when formatting XML domain_conf: graphics: properly escape user provided strings when formatting XML
src/conf/domain_conf.c | 724 +++++++++++++++++++++-------------------- 1 file changed, 372 insertions(+), 352 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Pavel Hrdina