6 a.m.
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(a)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