[libvirt] [PATCH v6 0/2] qemu: Support for QXL heads

v6: - Change heads when migrating (revert v2) - https://www.redhat.com/archives/libvir-list/2016-June/msg00106.html v5: - Just a rebase, plus v4 had some hunk in that didn't belong there so this one's cleaner to look at - https://www.redhat.com/archives/libvir-list/2016-April/msg00721.html v4: - Added test case without "heads=" - Don't consolidate 'size_t i, j' declarations - https://www.redhat.com/archives/libvir-list/2016-March/msg01436.html v3: - rebase on top of current master in order to cleanly apply - https://www.redhat.com/archives/libvir-list/2016-March/msg01198.html v2: - don't change heads when migrating - https://www.redhat.com/archives/libvir-list/2016-March/msg00811.html v1: - https://www.redhat.com/archives/libvir-list/2016-March/msg00410.html Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283207 *** BLURB HERE *** Martin Kletzander (2): qemu: Check for qxl's max_outputs parameter qemu: Add support to QXL's max_outputs parameter src/qemu/qemu_capabilities.c | 5 +++ src/qemu/qemu_capabilities.h | 4 ++ src/qemu/qemu_command.c | 8 ++++ src/qemu/qemu_migration.c | 1 - tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 + .../qemuxml2argv-video-qxl-heads.args | 28 +++++++++++++ .../qemuxml2argv-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2argv-video-qxl-noheads.args | 24 +++++++++++ .../qemuxml2argv-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2argvtest.c | 16 ++++++++ .../qemuxml2xmlout-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2xmlout-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 15 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml -- 2.8.3

There is max_outputs parameter for both qxl and qxl-vga and there is no easy way of saying that we want the capability enabled only if both are supported. So let's have two of them and only use them together. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 4 ++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 ++ 5 files changed, 15 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d32e71f12d24..43ac906c6e29 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -329,6 +329,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "nec-usb-xhci-ports", "virtio-scsi-pci.iothread", "name-guest", + + "qxl.max_outputs", /* 225 */ + "qxl-vga.max_outputs", ); @@ -1643,11 +1646,13 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVmwareSvga[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxl[] = { { "vgamem_mb", QEMU_CAPS_QXL_VGAMEM }, { "vram64_size_mb", QEMU_CAPS_QXL_VRAM64 }, + { "max_outputs", QEMU_CAPS_QXL_MAX_OUTPUTS }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = { { "vgamem_mb", QEMU_CAPS_QXL_VGA_VGAMEM }, { "vram64_size_mb", QEMU_CAPS_QXL_VGA_VRAM64 }, + { "max_outputs", QEMU_CAPS_QXL_VGA_MAX_OUTPUTS }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioGpu[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 368996ad29f1..77e4b9873b51 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -361,6 +361,10 @@ typedef enum { QEMU_CAPS_VIRTIO_SCSI_IOTHREAD, /* virtio-scsi-{pci,ccw}.iothread */ QEMU_CAPS_NAME_GUEST, /* -name guest= */ + /* 225 */ + QEMU_CAPS_QXL_MAX_OUTPUTS, /* -device qxl,max-outputs= */ + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS, /* -device qxl-vga,max-outputs= */ + 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 e56b7e58be85..ade80c8c815d 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -178,6 +178,8 @@ <flag name='nec-usb-xhci-ports'/> <flag name='virtio-scsi-pci.iothread'/> <flag name='name-guest'/> + <flag name='qxl.max_outputs'/> + <flag name='qxl-vga.max_outputs'/> <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 4df48435c460..81ec3312b1b2 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -182,6 +182,8 @@ <flag name='nec-usb-xhci-ports'/> <flag name='virtio-scsi-pci.iothread'/> <flag name='name-guest'/> + <flag name='qxl.max_outputs'/> + <flag name='qxl-vga.max_outputs'/> <version>2005000</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 8af08943693f..47b5a941a20a 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -188,6 +188,8 @@ <flag name='nec-usb-xhci-ports'/> <flag name='virtio-scsi-pci.iothread'/> <flag name='name-guest'/> + <flag name='qxl.max_outputs'/> + <flag name='qxl-vga.max_outputs'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> -- 2.8.3

On Sun, Jun 05, 2016 at 02:36:03AM +0200, Martin Kletzander wrote:
There is max_outputs parameter for both qxl and qxl-vga and there is no easy way of saying that we want the capability enabled only if both are supported. So let's have two of them and only use them together.
I would rephrase the commit message. The qxl and qxl-vga are different devices and even though they are both usually present.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 4 ++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 ++ 5 files changed, 15 insertions(+)
ACK

Historically, we added heads=1 to videos, but for example for qxl, we did not reflect that on the command line. Implementing that now could mean that if user were to migrate from older to newer libvirt, the command-line for qemu would differ. In order for that not to happen a migration cookie flag is introduced. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283207 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 8 ++++ src/qemu/qemu_migration.c | 1 - .../qemuxml2argv-video-qxl-heads.args | 28 +++++++++++++ .../qemuxml2argv-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2argv-video-qxl-noheads.args | 24 +++++++++++ .../qemuxml2argv-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2argvtest.c | 16 ++++++++ .../qemuxml2xmlout-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2xmlout-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 10 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 368bd871f7e3..60b0049f3d22 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4199,6 +4199,14 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, /* QEMU accepts mebibytes for vgamem_mb. */ virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024); } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_MAX_OUTPUTS)) { + if (video->heads) + virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); + } else { + video->heads = 0; + } } else if (video->vram && ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c5b2963d65b6..bed5f0b581b9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3131,7 +3131,6 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, if (nmigrate_disks) { if (has_drive_mirror) { size_t i, j; - /* Check user requested only known disk targets. */ for (i = 0; i < nmigrate_disks; i++) { for (j = 0; j < vm->def->ndisks; j++) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args new file mode 100644 index 000000000000..a939177088f9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args @@ -0,0 +1,28 @@ +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 214 \ +-smp 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=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,max_outputs=1,\ +bus=pci.0,addr=0x2 \ +-device qxl,id=video1,ram_size=67108864,vram_size=33554432,max_outputs=3,\ +bus=pci.0,addr=0x4 \ +-device qxl,id=video2,ram_size=67108864,vram_size=67108864,max_outputs=7,\ +bus=pci.0,addr=0x5 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml new file mode 100644 index 000000000000..45d08745e652 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml @@ -0,0 +1,47 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </video> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='7'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </video> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args new file mode 100644 index 000000000000..c609d487e181 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.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 214 \ +-smp 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=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,max_outputs=1,\ +bus=pci.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml new file mode 100644 index 000000000000..de003be53707 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml @@ -0,0 +1,39 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index db42b7bd71be..716755b39827 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1972,6 +1972,22 @@ mymain(void) DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_PIIX3_USB_UHCI); + DO_TEST("video-qxl-heads", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS, + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS); + + DO_TEST("video-qxl-noheads", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS, + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS); + DO_TEST_PARSE_FLAGS_ERROR("missing-machine", VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS, NONE); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml new file mode 100644 index 000000000000..e7b18ab2b2ab --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml @@ -0,0 +1,47 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </video> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='7'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml new file mode 100644 index 000000000000..ee22e30ad026 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml @@ -0,0 +1,39 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e9d1f938360d..eca8e7822523 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -817,6 +817,9 @@ mymain(void) DO_TEST("acpi-table"); + DO_TEST("video-qxl-heads"); + DO_TEST("video-qxl-noheads"); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.8.3

On Sun, Jun 05, 2016 at 02:36:04AM +0200, Martin Kletzander wrote:
Historically, we added heads=1 to videos, but for example for qxl, we did not reflect that on the command line. Implementing that now could mean that if user were to migrate from older to newer libvirt, the command-line for qemu would differ. In order for that not to happen a migration cookie flag is introduced.
Remove the migration cookie from commit message.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283207
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 8 ++++ src/qemu/qemu_migration.c | 1 - .../qemuxml2argv-video-qxl-heads.args | 28 +++++++++++++ .../qemuxml2argv-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2argv-video-qxl-noheads.args | 24 +++++++++++ .../qemuxml2argv-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2argvtest.c | 16 ++++++++ .../qemuxml2xmlout-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2xmlout-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 10 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 368bd871f7e3..60b0049f3d22 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4199,6 +4199,14 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, /* QEMU accepts mebibytes for vgamem_mb. */ virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024); } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_MAX_OUTPUTS)) { + if (video->heads) + virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); + } else { + video->heads = 0; + }
Same as for the first patch, qxl and qxl-vga are different devices. Currently we have qxl-vga only as primary video device and qxl only as secondary. You should pair the capabilities with the correct video device like we do for vram64.
} else if (video->vram && ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c5b2963d65b6..bed5f0b581b9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3131,7 +3131,6 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, if (nmigrate_disks) { if (has_drive_mirror) { size_t i, j; -
Unrelated white space change.
/* Check user requested only known disk targets. */ for (i = 0; i < nmigrate_disks; i++) { for (j = 0; j < vm->def->ndisks; j++) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args new file mode 100644
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml new file mode 100644
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args new file mode 100644
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml new file mode 100644
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index db42b7bd71be..716755b39827 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1972,6 +1972,22 @@ mymain(void) DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_PIIX3_USB_UHCI);
+ DO_TEST("video-qxl-heads", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS, + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS); + + DO_TEST("video-qxl-noheads", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS, + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS); +
Please move it next to other video-qxl tests.
DO_TEST_PARSE_FLAGS_ERROR("missing-machine", VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS, NONE); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml new file mode 100644
ACK with the defect fixed.

On Mon, Jun 06, 2016 at 03:23:24PM +0200, Pavel Hrdina wrote:
On Sun, Jun 05, 2016 at 02:36:04AM +0200, Martin Kletzander wrote:
Historically, we added heads=1 to videos, but for example for qxl, we did not reflect that on the command line. Implementing that now could mean that if user were to migrate from older to newer libvirt, the command-line for qemu would differ. In order for that not to happen a migration cookie flag is introduced.
Remove the migration cookie from commit message.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283207
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 8 ++++ src/qemu/qemu_migration.c | 1 - .../qemuxml2argv-video-qxl-heads.args | 28 +++++++++++++ .../qemuxml2argv-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2argv-video-qxl-noheads.args | 24 +++++++++++ .../qemuxml2argv-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2argvtest.c | 16 ++++++++ .../qemuxml2xmlout-video-qxl-heads.xml | 47 ++++++++++++++++++++++ .../qemuxml2xmlout-video-qxl-noheads.xml | 39 ++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 10 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 368bd871f7e3..60b0049f3d22 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4199,6 +4199,14 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, /* QEMU accepts mebibytes for vgamem_mb. */ virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024); } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_MAX_OUTPUTS)) { + if (video->heads) + virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); + } else { + video->heads = 0; + }
Same as for the first patch, qxl and qxl-vga are different devices. Currently we have qxl-vga only as primary video device and qxl only as secondary. You should pair the capabilities with the correct video device like we do for vram64.
That's exactly what I did *not* want. The thing is (as explained in the commit message for the first patch, which I believed is correct) that I either wanted it to enabled for all qxl-like devices or none. But not confuse users if it works only for the primary and not secondary or the other way around. Since the two capabilities will mostly be both supported at the same time, I'm ok with your version as well.
} else if (video->vram && ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c5b2963d65b6..bed5f0b581b9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3131,7 +3131,6 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, if (nmigrate_disks) { if (has_drive_mirror) { size_t i, j; -
Unrelated white space change.
/* Check user requested only known disk targets. */ for (i = 0; i < nmigrate_disks; i++) { for (j = 0; j < vm->def->ndisks; j++) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args new file mode 100644
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml new file mode 100644
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args new file mode 100644
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml new file mode 100644
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index db42b7bd71be..716755b39827 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1972,6 +1972,22 @@ mymain(void) DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_PIIX3_USB_UHCI);
+ DO_TEST("video-qxl-heads", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS, + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS); + + DO_TEST("video-qxl-noheads", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS, + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS); +
Please move it next to other video-qxl tests.
DO_TEST_PARSE_FLAGS_ERROR("missing-machine", VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS, NONE); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml new file mode 100644
ACK with the defect fixed.
Fixed everything according to your comments and pushed. Thanks for the review. Martin
participants (2)
-
Martin Kletzander
-
Pavel Hrdina