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 <[email protected]> > > --- > > 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
> [email protected]
> https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
