Re: [libvirt] [PATCH v4 06/20] backup: Document new XML for backups
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