10:27 a.m.
Setting heads to 0 in case that *max_outputs* is not supported while building
command line doesn't have any real effect. It only removes *heads* attribute
from live XML, but after restarting libvirt the default value is restored.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_command.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 55df23d..06f5091 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4298,8 +4298,6 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
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 &&
--
2.9.2
8:29 a.m.
On Fri, Aug 12, 2016 at 05:27:28PM +0200, Pavel Hrdina wrote:
Setting heads to 0 in case that *max_outputs* is not supported while
building
command line doesn't have any real effect. It only removes *heads* attribute
from live XML, but after restarting libvirt the default value is restored.
So that the XML reflects how the guest was started instead of
arbitrarily showing some number that wasn't used for the guest
initialization at all.
Moreover the zero will be sent in the migratable XML when migrating to
another host, hence keeping the setting disabled (so that some screens
that are running already will not be removed). At least that's what I
had in mind, maybe the behaviour changed in the meantime.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_command.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 55df23d..06f5091 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4298,8 +4298,6 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
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 &&
--
2.9.2
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
12:10 p.m.
On Sat, Aug 13, 2016 at 03:29:11PM +0200, Martin Kletzander wrote:
On Fri, Aug 12, 2016 at 05:27:28PM +0200, Pavel Hrdina wrote:
>Setting heads to 0 in case that *max_outputs* is not supported while building
>command line doesn't have any real effect. It only removes *heads* attribute
>from live XML, but after restarting libvirt the default value is restored.
>
So that the XML reflects how the guest was started instead of
arbitrarily showing some number that wasn't used for the guest
initialization at all.
This is the only thing what it does and it also has a side effect on downstream
screenshot API [1].
Moreover the zero will be sent in the migratable XML when migrating
to
another host, hence keeping the setting disabled (so that some screens
that are running already will not be removed). At least that's what I
had in mind, maybe the behaviour changed in the meantime.
In migratable XML there is the default heads='1', because our parser set this
default if "heads" is missing in the XML and we don't print
heads='0' to XML.
I've just tested it, to be sure, that the heads attribute doesn't have effect on
migration. If you start a guest on libvirt where *heads* isn't passed to qemu,
use virt-viewer and open two displays, and migrate to second host with newer
libvirt where *heads* is passed to qemu, it still have all the heads from
the source and no opened screen is removed.
It was also written during one of the review of the original patches that this
can be changed while the guest is running and from what I know there is no way
how to get the current number of heads back to track the correct number in
libvirt's XML.
[1] <https://bugzilla.redhat.com/show_bug.cgi?id=1366119>
Pavel
4:16 p.m.
On Sat, Aug 13, 2016 at 07:10:35PM +0200, Pavel Hrdina wrote:
On Sat, Aug 13, 2016 at 03:29:11PM +0200, Martin Kletzander wrote:
> On Fri, Aug 12, 2016 at 05:27:28PM +0200, Pavel Hrdina wrote:
> >Setting heads to 0 in case that *max_outputs* is not supported while building
> >command line doesn't have any real effect. It only removes *heads*
attribute
> >from live XML, but after restarting libvirt the default value is restored.
> >
>
> So that the XML reflects how the guest was started instead of
> arbitrarily showing some number that wasn't used for the guest
> initialization at all.
This is the only thing what it does and it also has a side effect on downstream
screenshot API [1].
OK, so there *is* a real effect ;)
> Moreover the zero will be sent in the migratable XML when
migrating to
> another host, hence keeping the setting disabled (so that some screens
> that are running already will not be removed). At least that's what I
> had in mind, maybe the behaviour changed in the meantime.
In migratable XML there is the default heads='1', because our parser set this
default if "heads" is missing in the XML and we don't print
heads='0' to XML.
I've just tested it, to be sure, that the heads attribute doesn't have effect on
migration. If you start a guest on libvirt where *heads* isn't passed to qemu,
use virt-viewer and open two displays, and migrate to second host with newer
libvirt where *heads* is passed to qemu, it still have all the heads from
the source and no opened screen is removed.
OK, that's only fair. There were like 7 or so divergent versions (based
on the reviewer) so I probably just messed that up. I kinda remember
that I hated 'heads' being initialized to '1' even when not used, that
was the main pain point of the whole series :-/
It was also written during one of the review of the original patches
that this
can be changed while the guest is running and from what I know there is no way
how to get the current number of heads back to track the correct number in
libvirt's XML.
Well, yes, it is supposed to be changeable, although I never managed to
achieve that.
You convinced me, ACK. With the commit message actually mentioning the
fix at least.
3023
days inactive
3024
days old
3 comments
2 participants
participants (2)
-
Martin Kletzander -
Pavel Hrdina