On 02/13/2012 03:36 AM, Philipp Hahn wrote: > Hello Eric, > > On Monday 13 February 2012 08:51:41 Philipp Hahn wrote: >> On Thursday 09 February 2012 23:01:38 Eric Blake wrote: > ... >>> But a better idea is to represent the XML in a way where the default >>> conversion makes sense, but where a user can explicitly change the XML >>> to get what they want. I'm thinking: >>> >>> If offset is 'utc' or 'localtime', we add a new attribute 'adjustment' >>> ... >> >> I'll give your approch a try, since I think this solves the problem with my >> STRICT. > > I encountered some problems while thinking more about your suggestions: > > 1. The problem here is that the libvirt XML parser (virDomainDefParseString()) > is more or less driver agnostic and thus does not know, which variants of > clock/@offset are supported by the driver: Correct - the domain_conf representation format must specify all combinations that at least one hypervisor finds useful, and hypervisor-specific code must then filter out the combinations that they do not support. > For example since for qemu the XML > is used as the native format, some values are only checked when you run the > domain, that is when the XML in converted to command-line arguments to kvm. > So your extra @reset and @adjustment attributes need to be pased into "struct > _virDomainClockDef". Yes - that means _virDomainClockDef needs to add new fields to track the new XML; where zero-initializing the field corresponds to omitting the attribute. > I see no canonical location to then convert <clock offset='utc' > adjustment='42'/> to <clock offset='variable' adjustment='42' basis='utc'/> > for everything but Xen-HV≥3.1, so the check / conversion has to be done for > every single driver. Yes, this is sometimes the case with adding new XML, that we have to go back and tweak all of the drivers to cope with the new fields. > > 2. Instead of implementing a second variant of offset='variable' it would be > enough to just add an attribute reset='true' (or > this_is_not_from_a_buggy_libvirt_and_I_really_need_the_reset='true') to > dumpxml, when a driver really implements the reset semantics. As long as it's > missing, libvirt can convert it to reset='false' in the Xen≥3.1 case. With an > explicit reset='true' libvirt would return an error for Xen-HV≥3.1. Once we add a new attribute, we can document that domain XML that omits the attribute will default to hypervisor-specific choices (xen < 3.1 can default one way, xen >= 3.1 can default another way, and qemu can default in a particular way as well). If the user provides the attribute, then they are requesting the specific behavior that they need. > > A more general solution would be to add a libvirt-version attribute to the XML > data, which libvirt could use to implement a bug-compatibility-mode: If we're > reading an old XML, treat offset='utc' as offset='variable', but if it's an > XML from ≥0.9.10, return an error. (how to treat XML without an explicit > version?) No. We _don't_ want a libvirt version attribute. What we want is a new attribute that, if present, determines the behavior according to the user, and if absent, provides sane upgrade semantics when parsing older XML into newer libvirt. > Yes, I know that the libvirt XML is supposed to be backward compatible, but > errors happen. You still haven't managed to convince me that we cannot solve this by adding new attributes. -- Eric Blake eblake@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list