On 2/9/19 9:56 AM, John Ferlan wrote: >> + <ref name="unsignedLong"/> > > I haven't looked ahead to conf processing, but I assume this is the same > type as whatever this gets read into. Whenever I see size in reference > to disks I'm thinking ULL's. It comes from docs/schemas/basictypes.rng. We don't document it very well, but in the RNG, 'int' is 32-bit, 'unsignedLong' is 64-bit, and we don't really do much to enforce things. Your question in later patches about using 'unsigned long long' C type to match 'unsignedLong' in the RNG is not unexpected, but this is not the first time we've had that sort of mismatch in naming (as the 'unsignedLong' as 64-bit type is a common idiom in the RNG). >> +++ b/tests/domainbackupxml2xmlin/empty.xml >> @@ -0,0 +1 @@ >> +<domainbackup/> > > Should add a 'block' and 'network' example too for completeness - of > course that depends on the matching between docs and rng. > > Should add target w/ shallow=on for completeness. > > Yes, I recall the commit message indicating essentially minimal. Yes, I need to beef up my examples, and make sure the testsuite passes. >> +++ b/tests/domaincheckpointxml2xmlout/empty.xml >> @@ -0,0 +1,10 @@ >> +<domaincheckpoint> >> + <name>1525889631</name> >> + <creationTime>1525889631</creationTime> >> + <disks> >> + <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/> >> + </disks> >> + <domain> >> + <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> > > This looks strange without the name... and the domain type like the > sample.xml output has Here's part of my reason for still having 'wip' on later patches - the snapshot code USED to just take a single UUID, until we released a couple of releases later that you can't revert to a domain state unless you capture the entire <domain> at the point in time you plan to revert to (as hot-plug actions after that time must be rolled back for the reversion to be correct). So the snapshot code has a horrible hack of taking either a UUID or a full <domain> sub-element. I don't want to repeat that hack in <checkpoint> - I want a full <domain> element every time. But coming up with the absolute minimum <domain> that does not make the test balloon into a super-long demonstration while still passing the RNG as a valid XML was where I got stuck, especially since the user does NOT pass in a <domain> sublement except when using the _REDEFINE flag (the rest of the time, the <domain> sublement is computed at creation time from the virDomainPtr that is gaining a new checkpoint). >> +++ b/tests/domaincheckpointxml2xmlout/sample.xml >> @@ -0,0 +1,16 @@ >> +<domaincheckpoint> >> + <name>1525889631</name> >> + <description>Completion of updates after OS install</description> >> + <creationTime>1525889631</creationTime> >> + <parent> >> + <name>1525111885</name> >> + </parent> >> + <disks> >> + <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/> > > Maybe this one should show the size attribute too... No, I want size to be an output-only element, and one you have to pass in an additional flag to get (because it requires runtime computation, rather than a static output). Oh, and that reminds me of another long-standing yak hair - we introduced the ability to validate some of our XML against the RNG, but we have not finished wiring it up to all of the APIs that take XML. Domain snapshots don't have a validation flag, and hence my copy-and-paste code for checkpoints don't have one, but both need one. (Otherwise, the presence or absence of an ignored <size> element won't really matter whether the RNG permits or rejects it) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
