On Thu, May 12, 2022 at 2:28 AM Jonathon Jongsma <jjong...@redhat.com>
wrote:

> On 5/11/22 9:56 AM, Han Han wrote:
> >
> >
> > On Wed, May 11, 2022 at 12:26 AM Jonathon Jongsma <jjong...@redhat.com
> > <mailto:jjong...@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 <jjong...@redhat.com
> >     <mailto:jjong...@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...
>
Generally, the minimal required QEMU version is 3.1.0(
https://libvirt.org/drvqemu.html):
"The libvirt KVM/QEMU driver can manage any QEMU emulator from version
3.1.0 or later"
That's to say, a user can use 3.1.0<=QEMU<6.2.0 with current libvirt.
So it's better to mention the required QEMU version for 'blob' to help
users resolved the unsupported error.

>
> >
> >     +   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
> >
>
>

Reply via email to