On Tue, Dec 08, 2015 at 03:02:13PM -0500, Laine Stump wrote: > 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? Definitely we should think about it and take our time to make such decision. Since you've asked Dan for his opinion, adding him to cc. > > (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 we should handle only implicit devices, that we cannot enable/disable via qemu command line. > > > > >> 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. > That was my question too, why this code took assumption, that the input device is implicitly added only if there is some graphics device. > > > > >>> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list