6:39 a.m.
This series is supposed to refactor the existing parse logic by
using the proper utility function and some parameters type change.
1. VNC graphics pasing were refactored to use the vitXMLProp* utility functions.
'autoport' and 'websocketGenerated' remain their bool type, as this
structure
members can be also modified later and not only determined by the initial XML
definition.
2. RDP graphics refactored same as VNC graphics, exept that 'multiUser' and
'replaceUser' types changed from bool to tristate, as parameters determined
only by the initial XML.
3. SDL graphics fullscreen option type changed from bool to tristate, to avoid
unnecessary type convertions.
4. desktop fullscreen option is removed as it was never used.
Kirill Shchetiniuk (4):
conf: VNC graphics parse refactor
conf: SDL fullscreen option refactor
conf: RDP graphics parse refactor
conf: desktop graphics remove unused option
docs/formatdomain.rst | 5 +-
src/conf/domain_conf.c | 125 ++++++++++++++----------------
src/conf/domain_conf.h | 7 +-
src/conf/schemas/domaincommon.rng | 5 --
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_process.c | 4 +-
src/qemu/qemu_validate.c | 4 +-
src/vbox/vbox_common.c | 8 +-
8 files changed, 71 insertions(+), 89 deletions(-)
--
2.49.0
6:39 a.m.
New subject: [PATCH 1/4] conf: VNC graphics parse refactor
From: Kirill Shchetiniuk <kshcheti(a)redhat.com>
Previously, the VNC graphics definition parsing were implemented by
string parsing, the virDomainGraphicsDefParseXMLVNC function was refactored
to use the appropriate virXMLProp* utility functions. Overall parsing logic
was not changed and results the same output as before.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/conf/domain_conf.c | 48 +++++++++++++++++++++---------------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b3b0bd7329..c16aa279fb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11815,47 +11815,46 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDef *def,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
- g_autofree char *port = virXMLPropString(node, "port");
- g_autofree char *websocketGenerated = virXMLPropString(node,
"websocketGenerated");
- g_autofree char *autoport = virXMLPropString(node, "autoport");
xmlNodePtr audioNode;
+ virTristateBool autoport;
+ virTristateBool websocketGenerated;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
return -1;
- if (port) {
- if (virStrToLong_i(port, NULL, 10, &def->data.vnc.port) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse vnc port %1$s"), port);
- return -1;
- }
+ if (virXMLPropInt(node, "port", 10, VIR_XML_PROP_NONE,
+ &def->data.vnc.port, 0) < 0)
+ return -1;
+
+ if (def->data.vnc.port == -1) {
/* Legacy compat syntax, used -1 for auto-port */
- if (def->data.vnc.port == -1) {
- if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
- def->data.vnc.port = 0;
- def->data.vnc.autoport = true;
- }
- } else {
- def->data.vnc.port = 0;
def->data.vnc.autoport = true;
}
- if (autoport) {
- ignore_value(virStringParseYesNo(autoport, &def->data.vnc.autoport));
-
- if (def->data.vnc.autoport && flags &
VIR_DOMAIN_DEF_PARSE_INACTIVE)
- def->data.vnc.port = 0;
+ if (def->data.vnc.port == 0) {
+ /* No port specified */
+ def->data.vnc.autoport = true;
}
+ if (virXMLPropTristateBool(node, "autoport", VIR_XML_PROP_NONE,
+ &autoport) < 0)
+ return -1;
+
+ virTristateBoolToBool(autoport, &def->data.vnc.autoport);
+
+ if (def->data.vnc.autoport && (flags &
VIR_DOMAIN_DEF_PARSE_INACTIVE))
+ def->data.vnc.port = 0;
+
if (virXMLPropInt(node, "websocket", 10, VIR_XML_PROP_NONE,
&def->data.vnc.websocket, 0) < 0)
return -1;
- if (websocketGenerated)
- ignore_value(virStringParseYesNo(websocketGenerated,
- &def->data.vnc.websocketGenerated));
+ if (virXMLPropTristateBool(node, "websocketGenerated", VIR_XML_PROP_NONE,
+ &websocketGenerated) < 0)
+ return -1;
+
+ virTristateBoolToBool(websocketGenerated, &def->data.vnc.websocketGenerated);
if (virXMLPropEnum(node, "sharePolicy",
virDomainGraphicsVNCSharePolicyTypeFromString,
--
2.49.0
6:39 a.m.
New subject: [PATCH 2/4] conf: SDL fullscreen option type refactor
From: Kirill Shchetiniuk <kshcheti(a)redhat.com>
Previously, the fullscreen option were parsed as a tristate but stored
as a bool type, changed the fullscreen option type to tristate bool to
avoid unnecessary type convertions.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/conf/domain_conf.c | 9 ++++-----
src/conf/domain_conf.h | 2 +-
src/qemu/qemu_command.c | 2 +-
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c16aa279fb..32a358d72c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11891,15 +11891,13 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDef *def,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
xmlNodePtr glNode;
- virTristateBool fullscreen;
ctxt->node = node;
if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE,
- &fullscreen) < 0)
+ &def->data.sdl.fullscreen) < 0)
return -1;
- virTristateBoolToBool(fullscreen, &def->data.sdl.fullscreen);
def->data.sdl.xauth = virXMLPropString(node, "xauth");
def->data.sdl.display = virXMLPropString(node, "display");
@@ -26956,8 +26954,9 @@ virDomainGraphicsDefFormatSDL(virBuffer *attrBuf,
virBufferEscapeString(attrBuf, " xauth='%s'",
def->data.sdl.xauth);
- if (def->data.sdl.fullscreen)
- virBufferAddLit(attrBuf, " fullscreen='yes'");
+ if (def->data.sdl.fullscreen != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(attrBuf, " fullscreen='%s'",
+ virTristateBoolTypeToString(def->data.sdl.fullscreen));
virDomainGraphicsDefFormatGL(childBuf, def->data.sdl.gl, NULL);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 58b97a2b54..8df404dc42 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2036,7 +2036,7 @@ struct _virDomainGraphicsDef {
struct {
char *display;
char *xauth;
- bool fullscreen;
+ virTristateBool fullscreen;
virTristateBool gl;
} sdl;
struct {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fec48edfc1..895af0ff6d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8108,8 +8108,7 @@ qemuBuildGraphicsSDLCommandLine(virQEMUDriverConfig *cfg
G_GNUC_UNUSED,
virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.sdl.xauth);
if (graphics->data.sdl.display)
virCommandAddEnvPair(cmd, "DISPLAY", graphics->data.sdl.display);
- if (graphics->data.sdl.fullscreen)
+ if (graphics->data.sdl.fullscreen == VIR_TRISTATE_BOOL_YES)
virCommandAddArg(cmd, "-full-screen");
virCommandAddArg(cmd, "-display");
--
2.49.0
6:39 a.m.
New subject: [PATCH 3/4] conf: RDP graphics parse refactor
From: Kirill Shchetiniuk <kshcheti(a)redhat.com>
Previously, the RDP graphics definition parsing were implemented by
string parsing, the virDomainGraphicsDefParseXMLRDP function was
refactored to use the appropriate virXMLProp* utility functions.
Overall parsing logic was not changed and results the same output as before.
Moreover, 'replaceUser' and 'mutliUser' params type was changed from
bool to tristate type, to avoid unnecessary type convertions. The
virDomainGraphicsDefFormatRDP fucntion was also refactored according
to the type changes.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/conf/domain_conf.c | 51 +++++++++++++++++++++-------------------
src/conf/domain_conf.h | 4 ++--
src/qemu/qemu_process.c | 4 ++--
src/qemu/qemu_validate.c | 4 ++--
src/vbox/vbox_common.c | 8 +++----
5 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 32a358d72c..201a4ded8b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11917,40 +11917,41 @@ virDomainGraphicsDefParseXMLRDP(virDomainGraphicsDef *def,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
- g_autofree char *port = virXMLPropString(node, "port");
- g_autofree char *autoport = virXMLPropString(node, "autoport");
- g_autofree char *replaceUser = virXMLPropString(node, "replaceUser");
- g_autofree char *multiUser = virXMLPropString(node, "multiUser");
+ virTristateBool autoport;
if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
return -1;
- if (port) {
- if (virStrToLong_i(port, NULL, 10, &def->data.rdp.port) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse rdp port %1$s"), port);
- return -1;
- }
- /* Legacy compat syntax, used -1 for auto-port */
- if (def->data.rdp.port == -1)
- def->data.rdp.autoport = true;
+ if (virXMLPropInt(node, "port", 10, VIR_XML_PROP_NONE,
+ &def->data.rdp.port, 0) < 0)
+ return -1;
- } else {
- def->data.rdp.port = 0;
+ if (def->data.rdp.port == -1) {
+ /* Legacy compat syntax, used -1 for auto-port */
def->data.rdp.autoport = true;
}
- if (STREQ_NULLABLE(autoport, "yes"))
+ if (def->data.rdp.port == 0) {
+ /* No port specified */
def->data.rdp.autoport = true;
+ }
+
+ if (virXMLPropTristateBool(node, "autoport", VIR_XML_PROP_NONE,
+ &autoport) < 0)
+ return -1;
+
+ virTristateBoolToBool(autoport, &def->data.rdp.autoport);
if (def->data.rdp.autoport && (flags &
VIR_DOMAIN_DEF_PARSE_INACTIVE))
def->data.rdp.port = 0;
- if (STREQ_NULLABLE(replaceUser, "yes"))
- def->data.rdp.replaceUser = true;
+ if (virXMLPropTristateBool(node, "replaceUser", VIR_XML_PROP_NONE,
+ &def->data.rdp.replaceUser))
+ return -1;
- if (STREQ_NULLABLE(multiUser, "yes"))
- def->data.rdp.multiUser = true;
+ if (virXMLPropTristateBool(node, "multiUser", VIR_XML_PROP_NONE,
+ &def->data.rdp.replaceUser))
+ return -1;
if (virDomainGraphicsAuthDefParseXML(node, &def->data.rdp.auth,
def->type) < 0)
@@ -26977,11 +26978,13 @@ virDomainGraphicsDefFormatRDP(virBuffer *attrBuf,
if (def->data.rdp.autoport)
virBufferAddLit(attrBuf, " autoport='yes'");
- if (def->data.rdp.replaceUser)
- virBufferAddLit(attrBuf, " replaceUser='yes'");
+ if (def->data.rdp.replaceUser != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(attrBuf, " replaceUser='%s'",
+ virTristateBoolTypeToString(def->data.rdp.replaceUser));
- if (def->data.rdp.multiUser)
- virBufferAddLit(attrBuf, " multiUser='yes'");
+ if (def->data.rdp.multiUser != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(attrBuf, " multiUser='%s'",
+ virTristateBoolTypeToString(def->data.rdp.multiUser));
virDomainGraphicsListenDefFormatAddr(attrBuf, glisten, flags);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8df404dc42..c909cfba3f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2043,8 +2043,8 @@ struct _virDomainGraphicsDef {
int port;
bool portReserved;
bool autoport;
- bool replaceUser;
- bool multiUser;
+ virTristateBool replaceUser;
+ virTristateBool multiUser;
virDomainGraphicsAuthDef auth;
} rdp;
struct {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 54b84922d6..02cca5fb6b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5550,12 +5550,12 @@ qemuProcessStartValidateGraphics(virDomainObj *vm)
_("qemu-rdp does not support multiple listens for one
graphics device."));
return -1;
}
- if (graphics->data.rdp.multiUser) {
+ if (graphics->data.rdp.multiUser == VIR_TRISTATE_BOOL_YES) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("qemu-rdp doesn't support the
'multiUser' attribute."));
return -1;
}
- if (graphics->data.rdp.replaceUser) {
+ if (graphics->data.rdp.replaceUser == VIR_TRISTATE_BOOL_YES) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("qemu-rdp doesn't support the
'replaceUser' attribute."));
return -1;
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index e45f636418..968533ccf3 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4512,12 +4512,12 @@ qemuValidateDomainDeviceDefDBusGraphics(const virDomainGraphicsDef
*graphics,
static int
qemuValidateDomainDeviceDefRDPGraphics(const virDomainGraphicsDef *graphics)
{
- if (graphics->data.rdp.replaceUser) {
+ if (graphics->data.rdp.replaceUser == VIR_TRISTATE_BOOL_YES) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("RDP doesn't support 'replaceUser'"));
return -1;
}
- if (graphics->data.rdp.multiUser) {
+ if (graphics->data.rdp.multiUser == VIR_TRISTATE_BOOL_YES) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("RDP doesn't support 'multiUser'"));
return -1;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 349ac832dc..929d452abd 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -1703,13 +1703,13 @@ vboxAttachDisplay(virDomainDef *def, struct _vboxDriver *data,
IMachine *machine
gVBoxAPI.UIVRDEServer.SetPorts(data, VRDEServer, def->graphics[i]);
- if (def->graphics[i]->data.rdp.replaceUser) {
+ if (def->graphics[i]->data.rdp.replaceUser ==
VIR_TRISTATE_BOOL_YES) {
gVBoxAPI.UIVRDEServer.SetReuseSingleConnection(VRDEServer,
PR_TRUE);
VIR_DEBUG("VRDP set to reuse single connection");
}
- if (def->graphics[i]->data.rdp.multiUser) {
+ if (def->graphics[i]->data.rdp.multiUser == VIR_TRISTATE_BOOL_YES)
{
gVBoxAPI.UIVRDEServer.SetAllowMultiConnection(VRDEServer,
PR_TRUE);
VIR_DEBUG("VRDP set to allow multiple connection");
@@ -3611,12 +3611,11 @@ vboxDumpDisplay(virDomainDef *def, struct _vboxDriver *data,
IMachine *machine)
gVBoxAPI.UIVRDEServer.GetAllowMultiConnection(VRDEServer,
&allowMultiConnection);
if (allowMultiConnection)
- graphics->data.rdp.multiUser = true;
+ graphics->data.rdp.multiUser = VIR_TRISTATE_BOOL_YES;
gVBoxAPI.UIVRDEServer.GetReuseSingleConnection(VRDEServer,
&reuseSingleConnection);
if (reuseSingleConnection)
- graphics->data.rdp.replaceUser = true;
+ graphics->data.rdp.replaceUser = VIR_TRISTATE_BOOL_YES;
VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics);
}
--
2.49.0
6:39 a.m.
New subject: [PATCH 4/4] conf: desktop graphics remove unused option
From: Kirill Shchetiniuk <kshcheti(a)redhat.com>
Previously, the 'desktop' graphics configuration contained the
'fullscreen' option which was never used, removed the unused option.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
docs/formatdomain.rst | 5 ++---
src/conf/domain_conf.c | 17 ++---------------
src/conf/domain_conf.h | 1 -
src/conf/schemas/domaincommon.rng | 5 -----
4 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 8753ee9c23..9940b2e38f 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -6697,7 +6697,7 @@ interaction with the admin.
<listen type='address' address='1.2.3.4'/>
</graphics>
<graphics type='rdp' autoport='yes' multiUser='yes'/>
- <graphics type='desktop' fullscreen='yes'/>
+ <graphics type='desktop'/>
<graphics type='spice'>
<listen type='network' network='rednet'/>
</graphics>
@@ -6878,8 +6878,7 @@ interaction with the admin.
``desktop``
This value is reserved for VirtualBox domains for the moment. It displays
a window on the host desktop, similarly to "sdl", but using the
VirtualBox
- viewer. Just like "sdl", it accepts the optional attributes ``display``
- and ``fullscreen``.
+ viewer. Just like "sdl", it accepts the optional attribute ``display``.
``egl-headless`` :since:`Since 4.6.0`
This display type provides support for an OpenGL accelerated display
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 201a4ded8b..be70ec3b96 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11961,20 +11961,11 @@ virDomainGraphicsDefParseXMLRDP(virDomainGraphicsDef *def,
}
-static int
+static void
virDomainGraphicsDefParseXMLDesktop(virDomainGraphicsDef *def,
xmlNodePtr node)
{
- virTristateBool fullscreen;
-
- if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE,
- &fullscreen) < 0)
- return -1;
-
- virTristateBoolToBool(fullscreen, &def->data.desktop.fullscreen);
def->data.desktop.display = virXMLPropString(node, "display");
-
- return 0;
}
@@ -12260,8 +12251,7 @@ virDomainGraphicsDefParseXML(virDomainXMLOption *xmlopt,
goto error;
break;
case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
- if (virDomainGraphicsDefParseXMLDesktop(def, node) < 0)
- goto error;
+ virDomainGraphicsDefParseXMLDesktop(def, node);
break;
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
if (virDomainGraphicsDefParseXMLSpice(def, node, ctxt, flags) < 0)
@@ -26998,9 +26988,6 @@ virDomainGraphicsDefFormatDesktop(virBuffer *attrBuf,
virDomainGraphicsDef *def)
{
virBufferEscapeString(attrBuf, " display='%s'",
def->data.desktop.display);
-
- if (def->data.desktop.fullscreen)
- virBufferAddLit(attrBuf, " fullscreen='yes'");
}
static int
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c909cfba3f..5e02c15ed5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2049,7 +2049,6 @@ struct _virDomainGraphicsDef {
} rdp;
struct {
char *display;
- bool fullscreen;
} desktop;
struct {
int port;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 5597d5a66b..81afa10083 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -4573,11 +4573,6 @@
<text/>
</attribute>
</optional>
- <optional>
- <attribute name="fullscreen">
- <ref name="virYesNo"/>
- </attribute>
- </optional>
</group>
<group>
<attribute name="type">
--
2.49.0
9:53 a.m.
On a Monday in 2025, Kirill Shchetiniuk via Devel wrote:
This series is supposed to refactor the existing parse logic by
using the proper utility function and some parameters type change.
1. VNC graphics pasing were refactored to use the vitXMLProp* utility functions.
'autoport' and 'websocketGenerated' remain their bool type, as this
structure
members can be also modified later and not only determined by the initial XML
definition.
2. RDP graphics refactored same as VNC graphics, exept that 'multiUser' and
'replaceUser' types changed from bool to tristate, as parameters determined
only by the initial XML.
3. SDL graphics fullscreen option type changed from bool to tristate, to avoid
unnecessary type convertions.
4. desktop fullscreen option is removed as it was never used.
Kirill Shchetiniuk (4):
conf: VNC graphics parse refactor
conf: SDL fullscreen option refactor
conf: RDP graphics parse refactor
conf: desktop graphics remove unused option
docs/formatdomain.rst | 5 +-
src/conf/domain_conf.c | 125 ++++++++++++++----------------
src/conf/domain_conf.h | 7 +-
src/conf/schemas/domaincommon.rng | 5 --
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_process.c | 4 +-
src/qemu/qemu_validate.c | 4 +-
src/vbox/vbox_common.c | 8 +-
8 files changed, 71 insertions(+), 89 deletions(-)
--
2.49.0
I am unable to apply these patches:
Applying: conf: VNC graphics parse refactor
error: patch failed: src/conf/domain_conf.c:11815
error: src/conf/domain_conf.c: patch does not apply
Patch failed at 0001 conf: VNC graphics parse refactor
Probably some server mangled them on the way. Not sure what magic
setting is needed for git-format-patch to get them through unmangled.
Patches from others using US-ASCII and quoted-printable encoding apply
for me.
Do you have this applied somewhere on gitlab or similar service?
Jano
1
days inactive
2
days old
5 comments
2 participants
participants (2)
-
Ján Tomko -
Kirill Shchetiniuk