[v2,2/5] conf: domain: add VIDEO_TYPE_DEFAULT

Message ID c29a571f305d69dcbc0ecfe91d400e31668eb160.1503846114.git.crobinso@redhat.com
State New
Headers show
Series
  • qemu: Default to video type=virtio for machvirt
Related show

Commit Message

Cole Robinson Aug. 27, 2017, 3:04 p.m.
Will be needed for future patches to pull the default video type
setting out of XML parsing routines.

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 src/conf/domain_conf.c              | 1 +
 src/conf/domain_conf.h              | 1 +
 src/qemu/qemu_command.c             | 3 +++
 src/qemu/qemu_domain_address.c      | 1 +
 tests/domaincapsschemadata/full.xml | 1 +
 5 files changed, 7 insertions(+)

-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Pavel Hrdina Aug. 28, 2017, 10:23 a.m. | #1
On Sun, Aug 27, 2017 at 11:04:39AM -0400, Cole Robinson wrote:
> Will be needed for future patches to pull the default video type

> setting out of XML parsing routines.

> 

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---

>  src/conf/domain_conf.c              | 1 +

>  src/conf/domain_conf.h              | 1 +

>  src/qemu/qemu_command.c             | 3 +++

>  src/qemu/qemu_domain_address.c      | 1 +

>  tests/domaincapsschemadata/full.xml | 1 +

>  5 files changed, 7 insertions(+)


[...]

> diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml

> index 82a92322e..ab6ef9f2e 100644

> --- a/tests/domaincapsschemadata/full.xml

> +++ b/tests/domaincapsschemadata/full.xml

> @@ -62,6 +62,7 @@

>      </graphics>

>      <video supported='yes'>

>        <enum name='modelType'>

> +        <value>default</value>

>          <value>vga</value>

>          <value>cirrus</value>

>          <value>vmvga</value>


This hunk should not be required, but I've checked the code that fills
in domain capabilities and we claim that every video model is supported.
Well that's wrong and should be fixed.

Anyway, the patch itself is good.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Pavel Hrdina Aug. 28, 2017, 10:39 a.m. | #2
On Mon, Aug 28, 2017 at 12:23:19PM +0200, Pavel Hrdina wrote:
> On Sun, Aug 27, 2017 at 11:04:39AM -0400, Cole Robinson wrote:

> > Will be needed for future patches to pull the default video type

> > setting out of XML parsing routines.

> > 

> > Signed-off-by: Cole Robinson <crobinso@redhat.com>

> > ---

> >  src/conf/domain_conf.c              | 1 +

> >  src/conf/domain_conf.h              | 1 +

> >  src/qemu/qemu_command.c             | 3 +++

> >  src/qemu/qemu_domain_address.c      | 1 +

> >  tests/domaincapsschemadata/full.xml | 1 +

> >  5 files changed, 7 insertions(+)

> 

> [...]

> 

> > diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml

> > index 82a92322e..ab6ef9f2e 100644

> > --- a/tests/domaincapsschemadata/full.xml

> > +++ b/tests/domaincapsschemadata/full.xml

> > @@ -62,6 +62,7 @@

> >      </graphics>

> >      <video supported='yes'>

> >        <enum name='modelType'>

> > +        <value>default</value>

> >          <value>vga</value>

> >          <value>cirrus</value>

> >          <value>vmvga</value>

> 

> This hunk should not be required, but I've checked the code that fills

> in domain capabilities and we claim that every video model is supported.

> Well that's wrong and should be fixed.


Sigh :) Ignore this comment, the code works correctly, this is a test
where we format all capabilities.

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 646b60a83..a6b793075 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -551,6 +551,7 @@  VIR_ENUM_IMPL(virDomainPanicModel, VIR_DOMAIN_PANIC_MODEL_LAST,
               "s390")
 
 VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
+              "default",
               "vga",
               "cirrus",
               "vmvga",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c3d684503..0c1660056 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1348,6 +1348,7 @@  struct _virDomainWatchdogDef {
 
 
 typedef enum {
+    VIR_DOMAIN_VIDEO_TYPE_DEFAULT,
     VIR_DOMAIN_VIDEO_TYPE_VGA,
     VIR_DOMAIN_VIDEO_TYPE_CIRRUS,
     VIR_DOMAIN_VIDEO_TYPE_VMVGA,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a68ff717f..bc027f85a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -97,6 +97,7 @@  VIR_ENUM_IMPL(qemuDiskCacheV2, VIR_DOMAIN_DISK_CACHE_LAST,
               "unsafe");
 
 VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
+              "", /* default value, we shouldn't see this */
               "std",
               "cirrus",
               "vmware",
@@ -110,6 +111,7 @@  VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
 VIR_ENUM_DECL(qemuDeviceVideo)
 
 VIR_ENUM_IMPL(qemuDeviceVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
+              "", /* default value, we shouldn't see this */
               "VGA",
               "cirrus-vga",
               "vmware-svga",
@@ -123,6 +125,7 @@  VIR_ENUM_IMPL(qemuDeviceVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
 VIR_ENUM_DECL(qemuDeviceVideoSecondary)
 
 VIR_ENUM_IMPL(qemuDeviceVideoSecondary, VIR_DOMAIN_VIDEO_TYPE_LAST,
+              "", /* default value, we shouldn't see this */
               "", /* no secondary device for VGA */
               "", /* no secondary device for cirrus-vga */
               "", /* no secondary device for vmware-svga */
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 16bf0cdf9..8afaa5209 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -753,6 +753,7 @@  qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
         case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
             return pciFlags;
 
+        case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
         case VIR_DOMAIN_VIDEO_TYPE_GOP:
         case VIR_DOMAIN_VIDEO_TYPE_LAST:
             return 0;
diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml
index 82a92322e..ab6ef9f2e 100644
--- a/tests/domaincapsschemadata/full.xml
+++ b/tests/domaincapsschemadata/full.xml
@@ -62,6 +62,7 @@ 
     </graphics>
     <video supported='yes'>
       <enum name='modelType'>
+        <value>default</value>
         <value>vga</value>
         <value>cirrus</value>
         <value>vmvga</value>