Re: [libvirt] [PATCH 09/11] qemu_capabilities: check for existence of virtio-vga

On Sat, Oct 08, 2016 at 10:01:51AM -0400, John Ferlan wrote: > > > On 09/30/2016 12:02 PM, Pavel Hrdina wrote: >> Commit 21373feb added support for primary virtio-vga device but it was >> checking for virtio-gpu. Let's check for existence of virtio-vga if we >> want to use it. >> >> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; >> --- >> src/qemu/qemu_capabilities.c | 2 ++ >> src/qemu/qemu_capabilities.h | 1 + >> tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + >> tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + >> tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + >> tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + >> tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + >> 7 files changed, 8 insertions(+) >> > > It seems this is related to your patch 6 changes or what I said should > be split. There's something funky there and I think everything should be > kept "closer" if not merged together. It could get ugly, but getting it > all done and explained at once would be best IMO. > > It seems what you're indicating is that the two shouldn't have been > combined into one CAP. Still that seems to be counter intuitive to the > first part of this series which was merging caps. It's a bit confusing > to say the least ;-)! The thing is that there is a difference in QXL video and virtio video. If QXL video device is compiled in QEMU there are always both models *qxl-vga* and *qxl*. But this is not the same as for virtio video. It is currently represented by three different models *virtio-gpu-device*, *virtio-gpu-pci* and *virtio-vga*. The first two models are tied together and if virtio video devices is compiled in they both exist. However, the *virtio-vga* model doesn't have to exist on some architectures even if the first two models exist. So we cannot group all three together.

Pavel > > Also you'll have a merge to handle with this patch... > >> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c >> index 5449b07..55e5527 100644 >> --- a/src/qemu/qemu_capabilities.c >> +++ b/src/qemu/qemu_capabilities.c >> @@ -344,6 +344,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, >> "query-hotpluggable-cpus", >> >> "virtio-net.rx_queue_size", /* 235 */ >> + "virtio-vga", >> ); >> >> >> @@ -1570,6 +1571,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { >> { "virtio-net-device", QEMU_CAPS_DEVICE_VIRTIO_NET }, >> { "virtio-gpu-pci", QEMU_CAPS_DEVICE_VIRTIO_GPU }, >> { "virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU }, >> + { "virtio-vga", QEMU_CAPS_DEVICE_VIRTIO_VGA }, >> { "virtio-keyboard-device", QEMU_CAPS_VIRTIO_KEYBOARD }, >> { "virtio-keyboard-pci", QEMU_CAPS_VIRTIO_KEYBOARD }, >> { "virtio-mouse-device", QEMU_CAPS_VIRTIO_MOUSE }, >> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h >> index 1766aa6..b2c8c9f 100644 >> --- a/src/qemu/qemu_capabilities.h >> +++ b/src/qemu/qemu_capabilities.h >> @@ -378,6 +378,7 @@ typedef enum { >> >> /* 235 */ >> QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */ >> + QEMU_CAPS_DEVICE_VIRTIO_VGA, /* virtio-vga */ > > /* -device virtio-vga */ > > Before an ACK - I'll be interested to understand the relationship w/ > patch 6 and a bit of the history/reasoning for a separate cap > (especially in light of combining other caps). > > John > >> >> QEMU_CAPS_LAST /* this must always be the last item */ >> } virQEMUCapsFlags; >> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml >> index 8d53988..17bd4bb 100644 >> --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml >> +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml >> @@ -180,6 +180,7 @@ >> <flag name='intel-iommu'/> >> <flag name='smm'/> >> <flag name='virtio-pci-disable-legacy'/> >> + <flag name='virtio-vga'/> >> <version>2004000</version> >> <kvmVersion>0</kvmVersion> >> <package></package> >> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml >> index a4f5663..fe058d9 100644 >> --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml >> +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml >> @@ -185,6 +185,7 @@ >> <flag name='intel-iommu'/> >> <flag name='smm'/> >> <flag name='virtio-pci-disable-legacy'/> >> + <flag name='virtio-vga'/> >> <version>2005000</version> >> <kvmVersion>0</kvmVersion> >> <package></package> >> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml >> index 60f5392..e63e7c1 100644 >> --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml >> +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml >> @@ -152,6 +152,7 @@ >> <flag name='display'/> >> <flag name='smm'/> >> <flag name='virtio-pci-disable-legacy'/> >> + <flag name='virtio-vga'/> >> <version>2005094</version> >> <kvmVersion>0</kvmVersion> >> <package></package> >> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml >> index 50506ba..5f93d59 100644 >> --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml >> +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml >> @@ -191,6 +191,7 @@ >> <flag name='intel-iommu'/> >> <flag name='smm'/> >> <flag name='virtio-pci-disable-legacy'/> >> + <flag name='virtio-vga'/> >> <version>2006000</version> >> <kvmVersion>0</kvmVersion> >> <package></package> >> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml >> index b0b4ab1..dc11677 100644 >> --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml >> +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml >> @@ -192,6 +192,7 @@ >> <flag name='smm'/> >> <flag name='virtio-pci-disable-legacy'/> >> <flag name='query-hotpluggable-cpus'/> >> + <flag name='virtio-vga'/> >> <version>2007000</version> >> <kvmVersion>0</kvmVersion> >> <package> (v2.7.0)</package> >> > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list