Re: [libvirt PATCH v3 4/6] conf: add support for 'blob' in virtio video device

On 5/11/22 9:56 AM, Han Han wrote: > > > On Wed, May 11, 2022 at 12:26 AM Jonathon Jongsma <jjongsma(a)redhat.com > <mailto:jjongsma@redhat.com>> wrote: > > Add the ability to enable blob resources for the virtio video device. > This will accelerate the display path due to less or no copying of pixel > data. > > Blob resource support can be enabled with e.g.: > > <video> > <model type='virtio' blob='on'/> > </video> > > Some additional background information about blob resources: > https://lists.freedesktop.org/archives/dri-devel/2020-August/275972.html > < https://lists.freedesktop.org/archives/dri-devel/2020-August/275972.html> > https://www.kraxel.org/blog/2021/05/virtio-gpu-qemu-graphics-update/ > < https://www.kraxel.org/blog/2021/05/virtio-gpu-qemu-graphics-update/> > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2032406 > <https://bugzilla.redhat.com/show_bug.cgi?id=2032406> > > Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com > <mailto:jjongsma@redhat.com>> > --- > docs/formatdomain.rst | 6 ++++++ > src/conf/domain_conf.c | 6 ++++++ > src/conf/domain_conf.h | 1 + > src/conf/domain_validate.c | 13 ++++++++++--- > src/conf/schemas/domaincommon.rng | 5 +++++ > 5 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index 993c65e50b..2161cc7e30 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -6198,6 +6198,12 @@ A video device. > :since:`since 1.3.3` ) extends secondary bar and makes it > addressable as > 64bit memory. > > + :since:`Since 8.2.0`, devices with type "virtio" have an > optional ``blob`` > > Since 8.4.0 for libvirt. Oops. Didn't catch that in the rebase. > And please mention the QEMU version 6.1.0 There are a lot of libvirt features that require minimum versions of qemu, but it doesn't look like we generally specify these qemu version requirements in the libvirt documentation. Not sure if I should add it here...

> > + attribute that can be set to "on" or "off". Setting ``blob`` to > "on" will > + enable the use of blob resources in the device. This can > accelerate the > + display path by reducing or eliminating copying of pixel data > between the > + guest and host. > + > :since:`Since 5.9.0` , the ``model`` element may also have an > optional > ``resolution`` sub-element. The ``resolution`` element has > attributes ``x`` > and ``y`` to set the minimum resolution for the video device. This > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 60c27ddd34..ae86f455bd 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -14154,6 +14154,9 @@ > virDomainVideoModelDefParseXML(virDomainVideoDef *def, > else if (rc == 0) > def->heads = 1; > > + if (virXMLPropTristateSwitch(node, "blob", VIR_XML_PROP_NONE, > &def->blob) < 0) > + return -1; > + > return 0; > } > > @@ -26075,6 +26078,9 @@ virDomainVideoDefFormat(virBuffer *buf, > virBufferAsprintf(buf, " heads='%u'", def->heads); > if (def->primary) > virBufferAddLit(buf, " primary='yes'"); > + if (def->blob != VIR_TRISTATE_SWITCH_ABSENT) > + virBufferAsprintf(buf, " blob='%s'", > + virTristateSwitchTypeToString(def->blob)); > if (def->accel || def->res) { > virBufferAddLit(buf, ">\n"); > virBufferAdjustIndent(buf, 2); > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index acd0588e9b..afa6db2403 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1763,6 +1763,7 @@ struct _virDomainVideoDef { > bool primary; > virDomainVideoAccelDef *accel; > virDomainVideoResolutionDef *res; > + virTristateSwitch blob; > virDomainVideoDriverDef *driver; > virDomainDeviceInfo info; > virDomainVirtioOptions *virtio; > diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c > index 18eb8d697d..da0ff06570 100644 > --- a/src/conf/domain_validate.c > +++ b/src/conf/domain_validate.c > @@ -226,9 +226,16 @@ virDomainVideoDefValidate(const > virDomainVideoDef *video, > } > } > > - if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO && > - (virDomainCheckVirtioOptionsAreAbsent(video->virtio) < 0)) > - return -1; > + if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { > + if (virDomainCheckVirtioOptionsAreAbsent(video->virtio) < 0) > + return -1; > + if (video->blob != VIR_TRISTATE_SWITCH_ABSENT) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("video type '%s' does not support blob > resources"), > + virDomainVideoTypeToString(video->type)); > + return -1; > + } > + } > > return 0; > } > diff --git a/src/conf/schemas/domaincommon.rng > b/src/conf/schemas/domaincommon.rng > index 2544864eb4..16b3fd9c53 100644 > --- a/src/conf/schemas/domaincommon.rng > +++ b/src/conf/schemas/domaincommon.rng > @@ -4251,6 +4251,11 @@ > <ref name="virYesNo"/> > </attribute> > </optional> > + <optional> > + <attribute name="blob"> > + <ref name="virOnOff"/> > + </attribute> > + </optional> > <optional> > <element name="acceleration"> > <optional> > -- > 2.35.1 >