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

Jonathon Jongsma posted 6 patches 3 years, 3 months ago
There is a newer version of this series
[libvirt PATCH v5 4/6] conf: add support for 'blob' in virtio video device
Posted by Jonathon Jongsma 3 years, 3 months ago
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://www.kraxel.org/blog/2021/05/virtio-gpu-qemu-graphics-update/

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2032406

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 docs/formatdomain.rst             |  7 +++++++
 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, 29 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index e28b805009..b057fc4703 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -6346,6 +6346,13 @@ A video device.
    :since:`since 1.3.3` ) extends secondary bar and makes it addressable as
    64bit memory.
 
+   :since:`Since 8.10.0` (QEMU driver only), devices with type "virtio" have an
+   optional ``blob`` 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. Note that blob resource support requires
+   QEMU version 6.1 or newer.
+
    :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 7484059e47..adbb41d19e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12583,6 +12583,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;
 }
 
@@ -24623,6 +24626,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 760ad22071..9e184bef69 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1790,6 +1790,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 81f6d5dbd5..5ce795df1d 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -225,9 +225,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 cefe818044..6731490061 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -4410,6 +4410,11 @@
                 <ref name="virYesNo"/>
               </attribute>
             </optional>
+            <optional>
+              <attribute name="blob">
+                <ref name="virOnOff"/>
+              </attribute>
+            </optional>
             <optional>
               <element name="acceleration">
                 <optional>
-- 
2.38.1
Re: [libvirt PATCH v5 4/6] conf: add support for 'blob' in virtio video device
Posted by Ján Tomko 3 years, 3 months ago
On a Friday in 2022, Jonathon Jongsma 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://www.kraxel.org/blog/2021/05/virtio-gpu-qemu-graphics-update/
>
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2032406
>
>Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
>---
> docs/formatdomain.rst             |  7 +++++++
> 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, 29 insertions(+), 3 deletions(-)
>
>diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>index e28b805009..b057fc4703 100644
>--- a/docs/formatdomain.rst
>+++ b/docs/formatdomain.rst
>@@ -6346,6 +6346,13 @@ A video device.
>    :since:`since 1.3.3` ) extends secondary bar and makes it addressable as
>    64bit memory.
>
>+   :since:`Since 8.10.0` (QEMU driver only), devices with type "virtio" have an
>+   optional ``blob`` 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. Note that blob resource support requires
>+   QEMU version 6.1 or newer.
>+
>    :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 7484059e47..adbb41d19e 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -12583,6 +12583,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;
> }
>
>@@ -24623,6 +24626,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));

Thankfully we abolished the strict 80 column rule, so this can fit on
one line.
https://libvirt.org/coding-style.html#code-formatting-especially-for-new-code

>     if (def->accel || def->res) {
>         virBufferAddLit(buf, ">\n");
>         virBufferAdjustIndent(buf, 2);

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano