[libvirt] [PATCH v6 2/2] qemu: Add support to QXL's max_outputs parameter
Martin Kletzander
mkletzan at redhat.com
Tue Jun 7 11:00:28 UTC 2016
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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160607/b2f8fc42/attachment-0001.sig>
More information about the libvir-list
mailing list