Re: [libvirt PATCH 4/9] qemu: add -display dbus support

On 11/5/21 11:51, marcandre.lureau(a)redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > Without any extra option, libvirt will start and tell QEMU to connect to > the private VM bus. > > Instead, a D-Bus "address" to connect to can be specified. Or the p2p > mode enabled. > > D-Bus display best works with GL & a rendernode, which can be specified > with <gl> child element. > > Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > --- > docs/schemas/basictypes.rng | 7 ++ > docs/schemas/domaincommon.rng | 33 ++++++++++ > src/conf/domain_conf.c | 65 ++++++++++++++++++- > src/conf/domain_conf.h | 7 ++ > src/libxl/libxl_conf.c | 1 + > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_command.c | 57 ++++++++++++++-- > src/qemu/qemu_domain.c | 1 + > src/qemu/qemu_driver.c | 10 ++- > src/qemu/qemu_hotplug.c | 1 + > src/qemu/qemu_process.c | 11 +++- > src/qemu/qemu_validate.c | 1 + > src/vmx/vmx.c | 1 + > .../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_6.2.0.x86_64.xml | 1 + > .../graphics-dbus-address.args | 31 +++++++++ > .../graphics-dbus-address.xml | 34 ++++++++++ > tests/qemuxml2argvdata/graphics-dbus-p2p.args | 31 +++++++++ > tests/qemuxml2argvdata/graphics-dbus-p2p.xml | 36 ++++++++++ > tests/qemuxml2argvdata/graphics-dbus.args | 31 +++++++++ > tests/qemuxml2argvdata/graphics-dbus.xml | 34 ++++++++++ > tests/qemuxml2argvtest.c | 7 ++ > .../graphics-dbus-address.xml | 39 +++++++++++ > .../qemuxml2xmloutdata/graphics-dbus-p2p.xml | 39 +++++++++++ > tests/qemuxml2xmloutdata/graphics-dbus.xml | 39 +++++++++++ > tests/qemuxml2xmltest.c | 10 +++ > 27 files changed, 523 insertions(+), 8 deletions(-) > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-address.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-address.xml > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-p2p.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-p2p.xml > create mode 100644 tests/qemuxml2argvdata/graphics-dbus.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus.xml > create mode 100644 tests/qemuxml2xmloutdata/graphics-dbus-address.xml > create mode 100644 tests/qemuxml2xmloutdata/graphics-dbus-p2p.xml > create mode 100644 tests/qemuxml2xmloutdata/graphics-dbus.xml We like to separate changes to public structures (like XML and/or APIs) and changes to drviers. IOW, this should be split into two patches, in the first you include all XML related changes (parsing, formatting) can even introduce those .args files so that xml2xmltest can check XML parsing & formatting. In here, you can put 'case VIR_DOMAIN_GRAPHICS_TYPE_DBUS:' just next to 'VIR_DOMAIN_GRAPHICS_TYPE_LAST' so that using dbus type errors out. And then in the second patch you provide the qemu implementation.

Oh, and don't forget to document your changes in docs/formatdomain.rst so that users know there's a new type and what attributes they can set.

> > diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng > index a221ff6295c0..ae4d5682229c 100644 > --- a/docs/schemas/basictypes.rng > +++ b/docs/schemas/basictypes.rng > @@ -220,6 +220,13 @@ > </define> > <!--======================================================================--> > > + <!-- A D-Bus server address: https://dbus.freedesktop.org/doc/dbus-specification.html#addresses --> > + <define name="dbusAddr"> > + <data type="string"> > + <param name="pattern">.+</param> > + </data> > + </define> Or use "filePath" type. They are the same.

> + > <!-- An ipv4 "dotted quad" address --> > <define name="ipv4Addr"> > <data type="string"> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index f01b7a64704b..1dba199db7ec 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -3996,6 +3996,39 @@ > </optional> > </interleave> > </group> > + <group> > + <attribute name="type"> > + <value>dbus</value> > + </attribute> > + <optional> > + <choice> > + <group> > + <attribute name="address"> > + <ref name="dbusAddr"/> > + </attribute> > + </group> > + <group> > + <attribute name="p2p"> > + <ref name="virYesNo"/> > + </attribute> > + </group> > + </choice> > + </optional> > + <interleave> > + <optional> > + <element name="gl"> > + <attribute name="enable"> > + <ref name="virYesNo"/> > + </attribute> > + <optional> > + <attribute name="rendernode"> > + <ref name="absFilePath"/> > + </attribute> > + </optional> > + </element> > + </optional> > + </interleave> > + </group> > <group> > <attribute name="type"> > <value>rdp</value> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index da0c64b46009..8a2f3c4115e0 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -911,6 +911,7 @@ VIR_ENUM_IMPL(virDomainGraphics, > "desktop", > "spice", > "egl-headless", > + "dbus", > ); > > VIR_ENUM_IMPL(virDomainGraphicsListen, > @@ -1891,6 +1892,11 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef *def) > g_free(def->data.egl_headless.rendernode); > break; > > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + g_free(def->data.dbus.address); > + g_free(def->data.dbus.rendernode); > + break; > + > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > @@ -12896,6 +12902,38 @@ virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDef *def, > } > > > +static int > +virDomainGraphicsDefParseXMLDBus(virDomainGraphicsDef *def, > + xmlNodePtr node, > + xmlXPathContextPtr ctxt) > +{ > + VIR_XPATH_NODE_AUTORESTORE(ctxt) > + xmlNodePtr cur; > + virTristateBool p2p; > + > + if (virXMLPropTristateBool(node, "p2p", VIR_XML_PROP_NONE, > + &p2p) < 0) Any reason for not passing def->data.dbus.p2p directly?

> + return -1; > + def->data.dbus.p2p = p2p == VIR_TRISTATE_BOOL_YES; > + > + def->data.dbus.address = virXMLPropString(node, "address"); So here p2p and address attributes are parsed, but later when generating cmd line you treat them as mutually exclusive. I think we could have a validate callback that does the exclusivity check. virDomainGraphicsDefValidate() looks like a good candidate.

> + > + ctxt->node = node; > + > + if ((cur = virXPathNode("./gl", ctxt))) { > + def->data.dbus.rendernode = virXMLPropString(cur, > + "rendernode"); > + > + if (virXMLPropTristateBool(cur, "enable", > + VIR_XML_PROP_REQUIRED, > + &def->data.dbus.gl) < 0) > + return -1; > + } > + > + return 0; > +} > + > + > virDomainGraphicsDef * > virDomainGraphicsDefNew(virDomainXMLOption *xmlopt) > { > @@ -12970,6 +13008,10 @@ virDomainGraphicsDefParseXML(virDomainXMLOption *xmlopt, > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > virDomainGraphicsDefParseXMLEGLHeadless(def, node, ctxt); > break; > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + if (virDomainGraphicsDefParseXMLDBus(def, node, ctxt) < 0) > + goto error; > + break; > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > @@ -26661,6 +26703,15 @@ virDomainGraphicsDefFormat(virBuffer *buf, > def->data.egl_headless.rendernode); > virBufferAddLit(buf, "/>\n"); > break; > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + if (def->data.dbus.p2p) > + virBufferAddLit(buf, " p2p='yes'"); > + if (def->data.dbus.address) > + virBufferAsprintf(buf, " address='%s'", > + def->data.dbus.address); > + if (!def->data.dbus.rendernode) > + break; > + break; > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > @@ -31231,6 +31282,11 @@ virDomainGraphicsDefHasOpenGL(const virDomainDef *def) > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > return true; > > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + if (graphics->data.dbus.gl == VIR_TRISTATE_BOOL_YES) > + return true; > + > + continue; > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > @@ -31246,7 +31302,8 @@ virDomainGraphicsSupportsRenderNode(const virDomainGraphicsDef *graphics) > bool ret = false; > > if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE || > - graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS) > + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS || > + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_DBUS) > ret = true; > > return ret; > @@ -31265,6 +31322,9 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics) > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > ret = graphics->data.egl_headless.rendernode; > break; > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + ret = graphics->data.dbus.rendernode; > + break; > case VIR_DOMAIN_GRAPHICS_TYPE_SDL: > case VIR_DOMAIN_GRAPHICS_TYPE_VNC: > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > @@ -31286,6 +31346,9 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics) > if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && > graphics->data.spice.gl != VIR_TRISTATE_BOOL_YES) > return false; > + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_DBUS && > + graphics->data.dbus.gl != VIR_TRISTATE_BOOL_YES) > + return false; > > if (virDomainGraphicsGetRenderNode(graphics)) > return false; > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index ab9a7d66f86d..42fe583418a6 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1733,6 +1733,7 @@ typedef enum { > VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP, > VIR_DOMAIN_GRAPHICS_TYPE_SPICE, > VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS, > + VIR_DOMAIN_GRAPHICS_TYPE_DBUS, > > VIR_DOMAIN_GRAPHICS_TYPE_LAST > } virDomainGraphicsType; > @@ -1916,6 +1917,12 @@ struct _virDomainGraphicsDef { > struct { > char *rendernode; > } egl_headless; > + struct { > + bool p2p; > + char *address; > + char *rendernode; > + virTristateBool gl; > + } dbus; > } data; > /* nListens, listens, and *port are only useful if type is vnc, > * rdp, or spice. They've been extracted from the union only to > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index 9f0739e1faff..b19009f270aa 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -1551,6 +1551,7 @@ libxlMakeVfb(virPortAllocatorRange *graphicsports, > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index c1579a2bf4b3..2c3a81eda245 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -6021,6 +6021,8 @@ virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUCaps *qemuCaps, > VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_SPICE); > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS)) > VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS); > + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISPLAY_DBUS)) > + VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_DBUS); > } > > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 483041f584d8..1960861015c6 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -8601,7 +8601,46 @@ qemuBuildGraphicsEGLHeadlessCommandLine(virQEMUDriverConfig *cfg G_GNUC_UNUSED, > > > static int > -qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg, > +qemuBuildGraphicsDBusCommandLine(virQEMUDriver *driver, > + virDomainObj *vm, > + virCommand *cmd, > + virDomainGraphicsDef *graphics) > +{ > + g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; > + > + if (!graphics->data.dbus.p2p && !graphics->data.dbus.address) { > + graphics->data.dbus.address = qemuDBusGetAddress(driver, vm); Ideally, building a cmd line wouldn't change domain definition (my dream is to have qemuBuildCommandLine() accept 'const virDomainDef *' one day), but we are far from it. So if you think this could be moved somewhere under qemuProcessSetupGraphics() (trainsitionally even) then you'll make me happy :-)

> + QEMU_DOMAIN_PRIVATE(vm)->dbusDaemonWanted = true; > + } > + > + virBufferAddLit(&opt, "dbus"); > + > + if (graphics->data.dbus.p2p) { > + virBufferAddLit(&opt, ",p2p=on"); > + } else { > + virBufferAddLit(&opt, ",addr="); > + virQEMUBuildBufferEscapeComma(&opt, graphics->data.dbus.address); > + } This is the exclusivity I had on mind above. > + if (graphics->data.dbus.gl != VIR_TRISTATE_BOOL_ABSENT) > + virBufferAsprintf(&opt, ",gl=%s", > + virTristateSwitchTypeToString(graphics->data.dbus.gl)); > + if (graphics->data.dbus.rendernode) { > + virBufferAddLit(&opt, ",rendernode="); > + virQEMUBuildBufferEscapeComma(&opt, > + graphics->data.dbus.rendernode); > + } > + > + virCommandAddArg(cmd, "-display"); > + virCommandAddArgBuffer(cmd, &opt); > + > + return 0; > +} > + > + > +static int > +qemuBuildGraphicsCommandLine(virQEMUDriver *driver, > + virDomainObj *vm, > + virQEMUDriverConfig *cfg, > virCommand *cmd, > virDomainDef *def, > virQEMUCaps *qemuCaps) > @@ -8635,6 +8674,12 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg, > graphics) < 0) > return -1; > > + break; > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + if (qemuBuildGraphicsDBusCommandLine(driver, vm, cmd, > + graphics) < 0) > + return -1; > + > break; > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > @@ -10363,6 +10408,7 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver, > int vnc = 0; > int spice = 0; > int egl_headless = 0; > + int dbus = 0; > > if (!driver->privileged) { > /* If we have no cgroups then we can have no tunings that > @@ -10407,6 +10453,9 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver, > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > ++egl_headless; > break; > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + ++dbus; > + break; > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > @@ -10414,10 +10463,10 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver, > } > } > > - if (sdl > 1 || vnc > 1 || spice > 1 || egl_headless > 1) { > + if (sdl > 1 || vnc > 1 || spice > 1 || egl_headless > 1 || dbus > 1) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("only 1 graphics device of each type " > - "(sdl, vnc, spice, headless) is supported")); > + "(sdl, vnc, spice, headless, dbus) is supported")); > return -1; > } > > @@ -10789,7 +10838,7 @@ qemuBuildCommandLine(virQEMUDriver *driver, > if (qemuBuildAudioCommandLine(cmd, def, qemuCaps) < 0) > return NULL; > > - if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps) < 0) > + if (qemuBuildGraphicsCommandLine(driver, vm, cfg, cmd, def, qemuCaps) < 0) > return NULL; > > if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index fb203bc83085..b027443ca346 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -3453,6 +3453,7 @@ qemuDomainDefSuggestDefaultAudioBackend(virQEMUDriver *driver, > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > break; > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > default: > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 7d13ae9754b4..d7cf7d43b267 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -15741,12 +15741,15 @@ qemuDomainOpenGraphics(virDomainPtr dom, > case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: > protocol = "spice"; > break; > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + protocol = "@dbus-display"; > + break; > case VIR_DOMAIN_GRAPHICS_TYPE_SDL: > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - _("Can only open VNC or SPICE graphics backends, not %s"), > + _("Can only open VNC, SPICE or D-Bus p2p graphics backends, not %s"), > virDomainGraphicsTypeToString(vm->def->graphics[idx]->type)); > goto endjob; > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > @@ -15810,12 +15813,15 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom, > case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: > protocol = "spice"; > break; > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > + protocol = "@dbus-display"; > + break; > case VIR_DOMAIN_GRAPHICS_TYPE_SDL: > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - _("Can only open VNC or SPICE graphics backends, not %s"), > + _("Can only open VNC, SPICE or D-Bus p2p graphics backends, not %s"), > virDomainGraphicsTypeToString(vm->def->graphics[idx]->type)); > goto cleanup; > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 328b06245f42..6037b4b92c34 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -4451,6 +4451,7 @@ qemuDomainChangeGraphics(virQEMUDriver *driver, > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > virReportError(VIR_ERR_INTERNAL_ERROR, > _("unable to change config on '%s' graphics type"), type); > break; > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 4ca9b100a802..2a413d602a10 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -4784,6 +4784,7 @@ qemuProcessGraphicsReservePorts(virDomainGraphicsDef *graphics, > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > @@ -4823,6 +4824,7 @@ qemuProcessGraphicsAllocatePorts(virQEMUDriver *driver, > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > @@ -4979,6 +4981,7 @@ qemuProcessGraphicsSetupListen(virQEMUDriver *driver, > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > @@ -5047,11 +5050,16 @@ qemuProcessGraphicsSetupRenderNode(virDomainGraphicsDef *graphics, > return 0; > > rendernode = &graphics->data.spice.rendernode; > - } else { > + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS) { > if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS_RENDERNODE)) > return 0; > > rendernode = &graphics->data.egl_headless.rendernode; > + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_DBUS) { > + rendernode = &graphics->data.dbus.rendernode; > + } else { > + g_warn_if_reached(); > + return -1; Right, in such cases we tend to switch if-else to switch() and call virReportEnumRangeError(); or just 'break' (we are inconsistent, yes) or do nothing at all.

> } > > if (!(*rendernode = virHostGetDRMRenderNode())) > @@ -5284,6 +5292,7 @@ qemuProcessStartValidateGraphics(virDomainObj *vm) > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c > index 397eea5edeaa..23c7e9f94cd0 100644 > --- a/src/qemu/qemu_validate.c > +++ b/src/qemu/qemu_validate.c > @@ -4168,6 +4168,7 @@ qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics, > case VIR_DOMAIN_GRAPHICS_TYPE_SDL: > case VIR_DOMAIN_GRAPHICS_TYPE_RDP: > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > case VIR_DOMAIN_GRAPHICS_TYPE_LAST: > break; > } > diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c > index d3540acd84a8..2c48bbc5e812 100644 > --- a/src/vmx/vmx.c > +++ b/src/vmx/vmx.c > @@ -3446,6 +3446,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOption *xmlopt, virDomainDef > case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: > case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: > case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: > + case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > _("Unsupported graphics type '%s'"), > virDomainGraphicsTypeToString(def->graphics[i]->type)); > diff --git a/tests/domaincapsdata/qemu_6.2.0-q35.x86_64.xml b/tests/domaincapsdata/qemu_6.2.0-q35.x86_64.xml > index df8bdae10227..ce3577f582d4 100644 > --- a/tests/domaincapsdata/qemu_6.2.0-q35.x86_64.xml > +++ b/tests/domaincapsdata/qemu_6.2.0-q35.x86_64.xml > @@ -154,6 +154,7 @@ > <value>vnc</value> > <value>spice</value> > <value>egl-headless</value> > + <value>dbus</value> > </enum> > </graphics> > <video supported='yes'> > diff --git a/tests/domaincapsdata/qemu_6.2.0-tcg.x86_64.xml b/tests/domaincapsdata/qemu_6.2.0-tcg.x86_64.xml > index 4d1210565968..c553d8d0b245 100644 > --- a/tests/domaincapsdata/qemu_6.2.0-tcg.x86_64.xml > +++ b/tests/domaincapsdata/qemu_6.2.0-tcg.x86_64.xml > @@ -160,6 +160,7 @@ > <value>vnc</value> > <value>spice</value> > <value>egl-headless</value> > + <value>dbus</value> > </enum> > </graphics> > <video supported='yes'> > diff --git a/tests/domaincapsdata/qemu_6.2.0.x86_64.xml b/tests/domaincapsdata/qemu_6.2.0.x86_64.xml > index c382ec462ca6..214285c958bc 100644 > --- a/tests/domaincapsdata/qemu_6.2.0.x86_64.xml > +++ b/tests/domaincapsdata/qemu_6.2.0.x86_64.xml > @@ -154,6 +154,7 @@ > <value>vnc</value> > <value>spice</value> > <value>egl-headless</value> > + <value>dbus</value> > </enum> > </graphics> > <video supported='yes'> > diff --git a/tests/qemuxml2argvdata/graphics-dbus-address.args b/tests/qemuxml2argvdata/graphics-dbus-address.args > new file mode 100644 > index 000000000000..12a5fadca782 > --- /dev/null > +++ b/tests/qemuxml2argvdata/graphics-dbus-address.args > @@ -0,0 +1,31 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/tmp/lib/domain--1-QEMUGuest1 \ > +USER=test \ > +LOGNAME=test \ > +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ > +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ > +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ > +/usr/bin/qemu-system-i386 \ > +-name guest=QEMUGuest1,debug-threads=on \ > +-S \ > +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ > +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ > +-m 214 \ > +-realtime mlock=off \ > +-smp 1,sockets=1,cores=1,threads=1 \ > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ > +-no-user-config \ > +-nodefaults \ > +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server=on,wait=off \ > +-mon chardev=charmonitor,id=monitor,mode=control \ > +-rtc base=utc \ > +-no-shutdown \ > +-no-acpi \ > +-boot strict=on \ > +-usb \ > +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ > +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ > +-display dbus,addr=unix:path=/tmp/foo \ > +-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \ > +-msg timestamp=on > diff --git a/tests/qemuxml2argvdata/graphics-dbus-address.xml b/tests/qemuxml2argvdata/graphics-dbus-address.xml > new file mode 100644 > index 000000000000..958e6a6a4059 > --- /dev/null > +++ b/tests/qemuxml2argvdata/graphics-dbus-address.xml > @@ -0,0 +1,34 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i386</emulator> > + <disk type='block' device='disk'> > + <driver name='qemu' type='raw'/> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='hda' bus='ide'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> I guess this <disk/> can be dropped as the test doesn't aim on testing it anyway.

> + <controller type='usb' index='0'/> > + <controller type='ide' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <graphics type='dbus' address='unix:path=/tmp/foo'/> This is weird. adress is set, p2p is not but ...

> + <video> > + <model type='cirrus' vram='16384' heads='1'/> > + </video> > + <memballoon model='none'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvdata/graphics-dbus-p2p.args b/tests/qemuxml2argvdata/graphics-dbus-p2p.args > new file mode 100644 > index 000000000000..c3b1cc0c12cf > --- /dev/null > +++ b/tests/qemuxml2argvdata/graphics-dbus-p2p.args > @@ -0,0 +1,31 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/tmp/lib/domain--1-QEMUGuest1 \ > +USER=test \ > +LOGNAME=test \ > +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ > +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ > +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ > +/usr/bin/qemu-system-i386 \ > +-name guest=QEMUGuest1,debug-threads=on \ > +-S \ > +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ > +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ > +-m 214 \ > +-realtime mlock=off \ > +-smp 1,sockets=1,cores=1,threads=1 \ > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ > +-no-user-config \ > +-nodefaults \ > +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server=on,wait=off \ > +-mon chardev=charmonitor,id=monitor,mode=control \ > +-rtc base=utc \ > +-no-shutdown \ > +-no-acpi \ > +-boot strict=on \ > +-usb \ > +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ > +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ > +-display dbus,p2p=on,gl=on,rendernode=/dev/dri/foo \ ... here p2p is set and address is not.

> +-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \ > +-msg timestamp=on > diff --git a/tests/qemuxml2argvdata/graphics-dbus-p2p.xml b/tests/qemuxml2argvdata/graphics-dbus-p2p.xml > new file mode 100644 > index 000000000000..80c2e42b4dc8 > --- /dev/null > +++ b/tests/qemuxml2argvdata/graphics-dbus-p2p.xml > @@ -0,0 +1,36 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i386</emulator> > + <disk type='block' device='disk'> > + <driver name='qemu' type='raw'/> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='hda' bus='ide'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> > + <controller type='usb' index='0'/> > + <controller type='ide' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <graphics type='dbus' p2p='yes'> > + <gl enable='yes' rendernode='/dev/dri/foo'/> > + </graphics> > + <video> > + <model type='cirrus' vram='16384' heads='1'/> > + </video> > + <memballoon model='none'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvdata/graphics-dbus.args b/tests/qemuxml2argvdata/graphics-dbus.args > new file mode 100644 > index 000000000000..7149e0177671 > --- /dev/null > +++ b/tests/qemuxml2argvdata/graphics-dbus.args > @@ -0,0 +1,31 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/tmp/lib/domain--1-QEMUGuest1 \ > +USER=test \ > +LOGNAME=test \ > +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ > +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ > +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ > +/usr/bin/qemu-system-i386 \ > +-name guest=QEMUGuest1,debug-threads=on \ > +-S \ > +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ > +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ > +-m 214 \ > +-realtime mlock=off \ > +-smp 1,sockets=1,cores=1,threads=1 \ > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ > +-no-user-config \ > +-nodefaults \ > +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server=on,wait=off \ > +-mon chardev=charmonitor,id=monitor,mode=control \ > +-rtc base=utc \ > +-no-shutdown \ > +-no-acpi \ > +-boot strict=on \ > +-usb \ > +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ > +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ > +-display dbus,addr=unix:path=/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/dbus/-1-QEMUGuest1-dbus.sock \ > +-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \ > +-msg timestamp=on > diff --git a/tests/qemuxml2argvdata/graphics-dbus.xml b/tests/qemuxml2argvdata/graphics-dbus.xml > new file mode 100644 > index 000000000000..ab0dcf1187af > --- /dev/null > +++ b/tests/qemuxml2argvdata/graphics-dbus.xml > @@ -0,0 +1,34 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i386</emulator> > + <disk type='block' device='disk'> > + <driver name='qemu' type='raw'/> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='hda' bus='ide'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> > + <controller type='usb' index='0'/> > + <controller type='ide' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <graphics type='dbus'/> > + <video> > + <model type='cirrus' vram='16384' heads='1'/> > + </video> > + <memballoon model='none'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index a0498a0d9201..4345ab47e053 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -1518,6 +1518,13 @@ mymain(void) > DO_TEST_CAPS_LATEST_PARSE_ERROR("graphics-spice-invalid-egl-headless"); > DO_TEST_CAPS_LATEST("graphics-spice-gl-auto-rendernode"); > > + DO_TEST("graphics-dbus", > + QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DISPLAY_DBUS); > + DO_TEST("graphics-dbus-address", > + QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DISPLAY_DBUS); > + DO_TEST("graphics-dbus-p2p", > + QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DISPLAY_DBUS); > + > DO_TEST_NOCAPS("input-usbmouse"); > DO_TEST_NOCAPS("input-usbtablet"); > DO_TEST_NOCAPS("misc-acpi"); > diff --git a/tests/qemuxml2xmloutdata/graphics-dbus-address.xml b/tests/qemuxml2xmloutdata/graphics-dbus-address.xml > new file mode 100644 > index 000000000000..8ead97baacd4 > --- /dev/null > +++ b/tests/qemuxml2xmloutdata/graphics-dbus-address.xml This file is not that much different to its image (tests/qemuxml2argvdata/graphics-dbus-address.xml). What we tend to do in theses cases is to make xml2xmlout/ file a symlink to the xml2argv file. It allows us to bring the size of tests/ down.

> @@ -0,0 +1,39 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i386</emulator> > + <disk type='block' device='disk'> > + <driver name='qemu' type='raw'/> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='hda' bus='ide'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> > + <controller type='usb' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> > + </controller> > + <controller type='ide' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> > + </controller> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <graphics type='dbus' address='unix:path=/tmp/foo'/> > + <video> > + <model type='cirrus' vram='16384' heads='1' primary='yes'/> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> > + </video> > + <memballoon model='none'/> > + </devices> > +</domain> This is where I stop my review. I skimmed throuh the rest of patches and if you apply my comments to them too they will look okay. Michal