Hi On Mon, Nov 29, 2021 at 9:20 PM Michal Prívozník <mprivozn@xxxxxxxxxx> wrote: > > On 11/5/21 11:51, marcandre.lureau@xxxxxxxxxx wrote: > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > --- > > 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. Ok > > > 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. It's the last patch. It can be squashed. > > > > > 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. I would rather have a specific type. Some day, it could receive a better pattern. > > > + > > <!-- 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? It's not a tristate. There are other similar cases in the file. We could make it if it helps. (?) > > > + 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. Ack > > > + > > + 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 :-) ok > > > + 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. > ok > > > } > > > > 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. yes > > > + <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 ... this is -address test > > > + <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. > And this is p2p test. Seems ok to me. > > +-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. ok > > > @@ -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 >
