[libvirt] [PATCH 10/11] qemu_command: properly detect which model to use for video device
Pavel Hrdina
phrdina at redhat.com
Mon Oct 10 10:55:59 UTC 2016
On Sat, Oct 08, 2016 at 10:02:00AM -0400, John Ferlan wrote:
>
>
> On 09/30/2016 12:02 PM, Pavel Hrdina wrote:
> > This improves commit 706b5b6277 in a way that we instead use qemu
> > capabilities to detect whether we can use virto-vga or not.
> >
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> > src/qemu/qemu_command.c | 4 +--
> > src/qemu/qemu_domain.c | 12 ++++++++
> > src/qemu/qemu_domain.h | 3 ++
> > .../qemuxml2argv-video-virtio-gpu-device.args | 2 +-
> > .../qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +-
> > .../qemuxml2argv-video-virtio-gpu-virgl.args | 2 +-
> > .../qemuxml2argv-video-virtio-vga.args | 24 ++++++++++++++++
> > .../qemuxml2argv-video-virtio-vga.xml | 33 ++++++++++++++++++++++
> > tests/qemuxml2argvtest.c | 4 +++
> > 9 files changed, 80 insertions(+), 6 deletions(-)
> > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.args
> > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.xml
> >
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 15bb381..bb8128e 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -4307,10 +4307,8 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
> > virDomainVideoTypeToString(video->type));
> > goto error;
> > }
> > - if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
> > - qemuDomainMachineIsVirt(def)) {
> > + if (!qemuDomainSupportVideoVga(video, qemuCaps))
> > model = "virtio-gpu-pci";
> > - }
> > } else {
> > model = "qxl";
> > }
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 2b7e6d4..79c945a 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -6230,3 +6230,15 @@ qemuDomainCheckMonitor(virQEMUDriverPtr driver,
> >
> > return ret;
> > }
> > +
> > +
> > +bool
> > +qemuDomainSupportVideoVga(virDomainVideoDefPtr video,
> > + virQEMUCapsPtr qemuCaps)
> > +{
> > + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
> > + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_VGA))
>
> Shall I assume losing the qemuDomainMachineIsVirt check is expected?
> Perhaps a quick explanation in the commit message would suffice...
Yes, because previous patch adds proper detection whether we can use
*virtio-vga* model or not, so we can use that capability instead of
listing architectures (which may change in the future).
> As in, commit id 'xxx' use MachineIsVirt, when it should have been
> checking the cap instead...
At that time it was a quick fix to resolve that bug and using the capability
was not possible because that capability did not exist.
> There's two things going on in this one patch - not only creating a
> helper, but you're altering the logic such that you have differences in
> output (.args) that should have been "that way" when the code was first
> introduced. In a way it feels like fixing a bug... Following the
> breadcrumbs is, well mind numbing!
This basically fixes a bug by improving the logic, I'll try to explain it
better in the commit message.
Pavel
>
> ACK in principle - still just a better explanation would be good!
>
>
> John
>
>
>
> > + return false;
> > +
> > + return true;
> > +}
> > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> > index 521531b..b968858 100644
> > --- a/src/qemu/qemu_domain.h
> > +++ b/src/qemu/qemu_domain.h
> > @@ -738,4 +738,7 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver,
> > virDomainObjPtr vm,
> > qemuDomainAsyncJob asyncJob);
> >
> > +bool qemuDomainSupportVideoVga(virDomainVideoDefPtr video,
> > + virQEMUCapsPtr qemuCaps);
> > +
> > #endif /* __QEMU_DOMAIN_H__ */
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.args
> > index fefa2b6..1107409 100644
> > --- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.args
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.args
> > @@ -20,5 +20,5 @@ QEMU_AUDIO_DRV=none \
> > -drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\
> > id=drive-ide0-0-0,cache=none \
> > -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> > --device virtio-vga,id=video0,bus=pci.0,addr=0x2 \
> > +-device virtio-gpu-pci,id=video0,bus=pci.0,addr=0x2 \
> > -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
> > index 8844498..f1ebb62 100644
> > --- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
> > @@ -20,5 +20,5 @@ QEMU_AUDIO_DRV=spice \
> > id=drive-ide0-0-0,cache=none \
> > -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> > -spice port=0,gl=on \
> > --device virtio-vga,id=video0,virgl=on,bus=pci.0,addr=0x2 \
> > +-device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2 \
> > -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.args
> > index 6a55311..0131be8 100644
> > --- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.args
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.args
> > @@ -20,5 +20,5 @@ QEMU_AUDIO_DRV=none \
> > -drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\
> > id=drive-ide0-0-0,cache=none \
> > -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> > --device virtio-vga,id=video0,virgl=on,bus=pci.0,addr=0x2 \
> > +-device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2 \
> > -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.args
> > new file mode 100644
> > index 0000000..fefa2b6
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.args
> > @@ -0,0 +1,24 @@
> > +LC_ALL=C \
> > +PATH=/bin \
> > +HOME=/home/test \
> > +USER=test \
> > +LOGNAME=test \
> > +QEMU_AUDIO_DRV=none \
> > +/usr/bin/qemu \
> > +-name QEMUGuest1 \
> > +-S \
> > +-M pc \
> > +-m 1024 \
> > +-smp 1,sockets=1,cores=1,threads=1 \
> > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> > +-nographic \
> > +-nodefaults \
> > +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> > +-no-acpi \
> > +-boot c \
> > +-usb \
> > +-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\
> > +id=drive-ide0-0-0,cache=none \
> > +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> > +-device virtio-vga,id=video0,bus=pci.0,addr=0x2 \
> > +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.xml
> > new file mode 100644
> > index 0000000..e3d28bb
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-vga.xml
> > @@ -0,0 +1,33 @@
> > +<domain type='qemu'>
> > + <name>QEMUGuest1</name>
> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> > + <memory unit='KiB'>1048576</memory>
> > + <currentMemory unit='KiB'>1048576</currentMemory>
> > + <vcpu placement='static'>1</vcpu>
> > + <os>
> > + <type arch='i686' machine='pc'>hvm</type>
> > + <boot dev='hd'/>
> > + </os>
> > + <clock offset='utc'/>
> > + <on_poweroff>destroy</on_poweroff>
> > + <on_reboot>restart</on_reboot>
> > + <on_crash>destroy</on_crash>
> > + <devices>
> > + <emulator>/usr/bin/qemu</emulator>
> > + <disk type='file' device='disk'>
> > + <driver name='qemu' type='qcow2' cache='none'/>
> > + <source file='/var/lib/libvirt/images/QEMUGuest1'/>
> > + <target dev='hda' bus='ide'/>
> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> > + </disk>
> > + <controller type='ide' index='0'/>
> > + <controller type='usb' index='0'/>
> > + <controller type='pci' index='0' model='pci-root'/>
> > + <input type='mouse' bus='ps2'/>
> > + <input type='keyboard' bus='ps2'/>
> > + <video>
> > + <model type='virtio' heads='1'/>
> > + </video>
> > + <memballoon model='virtio'/>
> > + </devices>
> > +</domain>
> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> > index 6404865..4cbe57e 100644
> > --- a/tests/qemuxml2argvtest.c
> > +++ b/tests/qemuxml2argvtest.c
> > @@ -1616,6 +1616,10 @@ mymain(void)
> > QEMU_CAPS_SPICE,
> > QEMU_CAPS_SPICE_GL,
> > QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
> > + DO_TEST("video-virtio-vga",
> > + QEMU_CAPS_DEVICE_VIRTIO_GPU,
> > + QEMU_CAPS_DEVICE_VIRTIO_VGA,
> > + QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
> > DO_TEST_PARSE_ERROR("video-invalid", NONE);
> >
> > DO_TEST("virtio-rng-default", QEMU_CAPS_DEVICE_VIRTIO_RNG,
> >
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161010/ae4e0e4a/attachment-0001.sig>
More information about the libvir-list
mailing list