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

[Peter Krempa]

On Wed, May 11, 2022 at 13:28:02 -0500, Jonathon Jongsma wrote:
> On 5/11/22 9:56 AM, Han Han wrote:
> > 
> > 
> > On Wed, May 11, 2022 at 12:26 AM Jonathon Jongsma  > > wrote:

[...]

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

We mention the required qemu version in many cases, so it definitely
will not be an outlier.

Usually it's when either it's important, a recent addition or somebody
cares about adding it.

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

[Han Han]

On Thu, May 12, 2022 at 2:28 AM Jonathon Jongsma 
wrote:

> On 5/11/22 9:56 AM, Han Han wrote:
> >
> >
> > On Wed, May 11, 2022 at 12:26 AM 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.:
> >
> >  
> >
> >  
> >
> > 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
> > 
> >
> > Signed-off-by: Jonathon Jongsma  > >
> > ---
> >   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.

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

[Jonathon Jongsma]

On 5/11/22 9:56 AM, Han Han wrote:



On Wed, May 11, 2022 at 12:26 AM 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.:

     
       
     

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




+   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 

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

[Han Han]

On Wed, May 11, 2022 at 12:26 AM 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.:
>
> 
>   
> 
>
> 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 
> ---
>  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.
And please mention the QEMU version 6.1.0

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

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

[Jonathon Jongsma]

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.:


  


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 
---
 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``
+   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 @@
 
   
 
+
+  
+
+  
+
 
   
 
-- 
2.35.1