On 12/07/2015 06:49 PM, Pavel Hrdina wrote:
On Mon, Dec 07, 2015 at 12:36:46PM -0500, Laine Stump wrote:
On 12/04/2015 02:30 PM, Pavel Hrdina wrote:
The current code was a little bit odd.
Understatement of the Week :-) (also you get bonus points for being polite!)
At first we've removed all
possible implicit input devices from domain definition to add them later
back if there was any graphics device defined while parsing XML
description. That's not all, while formating domain definition to XML
description we at first ignore any input devices with bus different to
USB and VIRTIO and few lines later we add implicit input devices to XML.
Looking back at the history, it seems that the bit that ignores
particular input devices when there is a graphics device present was
included in the original commit of domain parsing code; I guess at the
time we didn't think all guest hardware should be represented in the
config. At some later time we decided that "if it's in the guest, it
needs to be in the config", and the 2nd bit that adds in the implicit
devices was added without noticed the earlier bit. Does seem pretty
pointless though :-P
This seems to me as a lot of code for nothing. This patch could seems
to be more complicated than original approach, but this is a preferred
way to modify/add driver specific staff only in those drivers and not
deal with them in common parsing/formating functions.
The update is to add those implicit input devices only into running XML
and don't put them automatically to offline XML. In addition we won't
remove any input devices specified by user.
I haven't looked at the code yet, but if my understanding of this
description is correct, your changes cause the implicitly added devices
to *not* be stored in the config as written to disk? Sometime a few
years ago, based on problems that users had with OSes complaining of
"hardware changed" I think, we started down the path of "every device
that is in the guest should be represented in the config" in order to
guarantee that those same devices will still be there the next time the
domain is started, even if libvirt changes what it adds implicitly.
Or am I jumping to incorrect conclusions about what the patch does?
Yes, that's correct, but the downside of this approach is that in the future
qemu developers can decide, that some devices are not longer implicit and what
if those devices cannot be passed to qemu command line? This will result in
scenario, where you have some implicit devices stored in the XML, but actually
the device doesn't exists in the guest.
Yes, I see your point, and the merit. It does go against what we've
previously said we want to do though (and implies the need for
further-reaching changes to be consistent, e.g. the builtin IDE and FDC
controllers on many architectures including i440fx, and builtin SATA
controller on Q35). So before we do it, I want to make sure that
everyone is okay with this.
Dan?
(also, another thing we would want to think about if we do this - do we
also want to go through all the existing config files during an upgrade
and remove these implicit devices from what's stored on disk? There are
pros and cons to both sides of that too)
I think that whatever hardware is in the guest needs to be represented
in the config, not just in the live XML.
I'm not preferring any of those two solutions, but implicit devices that
exists in the hypervisor and we cannot do anything about it should be only in
live definition. Than we can simply stop adding those devices into live
definition if hypervisor decide to remove those implicit devices instead of
removing those devices from persistent definition to make sure, that the
definition represents current state of the guest hardware.
There was also inconsistence between our behavior and QEMU's in the way,
that in QEMU there is no way how to disable those implicit input devices
and they are available always, even without graphics device.
Is this the case for every version of qemu that we support?
Yes, this applies for every version of qemu so far.
Okay. I was just curious how that code (that is, making it conditionally
happen based on the presence of a video device) got in in the first
place - either it was incompletely researched, or qemu has changed.
This
applies also to XEN hypervisor. VZ driver already does its part by
putting correct implicit devices into live XML.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list