On 06/22/2018 04:16 PM, John Ferlan wrote:
On 06/13/2018 12:42 PM, Eric Blake wrote:
Upcoming patches plan to introduce virDomainCheckpointPtr as a new object for use in incremental backups, along with documentation how incremental backups differ from snapshots. But first, we need to rename any existing mention of a 'system checkpoint' to instead be a 'full system state snapshot', so that we aren't overloading the term checkpoint.
Signed-off-by: Eric Blake <eblake@redhat.com>
--- Bikeshed suggestions on what to name the new object for use in backups is welcome, if we would rather keep the term 'checkpoint' for a disk+memory snapshot. ---
"Naming is hard" and opinions can vary greatly - be careful for what you ask in case you receive something not wanted ;-).
I haven't followed the discussions thus far all that closely, but I'll give this a go anyway since it's languishing and saying nothing is akin to implicitly agreeing everything is fine. Fair warning, I'm not all that familiar with snapshot algorithms having largely tried to ignore it since others (Eric and Peter) have far more in depth knowledge.
In any case, another option for the proposed "checkpoint" could be a "snapshot reference". One can start or end a reference period and then set or clear a reference point.
What I'm not clear on yet is whether the intention is to have this checkpoint (and backup) be integrated in any way to the existing snapshot algorithms. I guess part of me thinks that if I take a full system snapshot, then any backup/checkpoint data should be included so that if/when I go back to that point in time I can start from whence I left as it relates to my backup. Kind of a superset and/or integrated model rather than something bolted onto the side to resolve a specific need.
That's a tough call. My current design has incremental backups completely separate from the existing checkpoint code for several reasons: - the snapshot code is already confusing with lots of flags (internal/external, disk/memory, etc) - snapshots can be reverted to (well, in theory - we STILL can't revert to an external snapshot cleanly, even though the design supports it) - incremental backups are not direct revert points so, rather than bolt something on to the existing design, I went with a new concept. As you found later in the series, I then tried to provide a good summary page describing the different pieces, and what tradeoffs are involved in order to know which approach will work for a given need.
I suppose a reservation I have about separate virDomainCheckpoint* and virDomainBackup* API's is understanding the relationship between the two naming spaces. IIUC though a Checkpoint would be reference point in time within a Backup period.
A sequence of snapshots are different points in time you can revert to. A sequence of checkpoints are different points in time you can use as the reference point for starting an incremental backup. So if we don't like the term 'checkpoint', maybe virDomainBlockBackupReference would work. But it is longer, and would make for some mouthful API names. Also, you commented elsewhere that 'virDomainBackupBegin' misses out on the fact that under the hood, it is a block operation (only disk state); would 'virDomainBlockBackupBegin' be any better? There are fewer APIs with the term 'Backup' than with 'Checkpoint', if we do want go with that particular rename.
I do have more comments in patch2, but I want to make them coherent before posting. Still I wanted to be sure you got at least "some" feedback for this and well of course an opinion on checkpoint ;-)
docs/formatsnapshot.html.in | 14 +++++++------- include/libvirt/libvirt-domain-snapshot.h | 2 +- src/conf/snapshot_conf.c | 2 +- src/libvirt-domain-snapshot.c | 4 ++-- src/qemu/qemu_driver.c | 12 ++++++------ tools/virsh-snapshot.c | 2 +- tools/virsh.pod | 14 +++++++------- 7 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index fbbecfd242..f2e51df5ab 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -33,7 +33,7 @@ resume in a consistent state; but if the disks are modified externally in the meantime, this is likely to lead to data corruption.</dd> - <dt>system checkpoint</dt> + <dt>full system state</dt>
Is "state" superfluous in this context? IOW: Everywhere that "full system state" exists, it seems "full system" could be used.
Other synonyms that came up are complete, entire, integrated, or thorough (hah!). But I think "Full System" conveys enough meaning even though it could convey more meaning than intended.
Okay, I can live with shortening the replacement to 'full system'. Don't know if it will happen in the v2 series that I hope to post later tonight, or if it would be done on top (my immediate short-term goal is to get a demo of incremental backups working, to show the API is usable; although the demo depends on unreleased qemu code so only the API would actually go in this month's libvirt release, while the underlying src/qemu/* changes can be delayed and polished to be better than the demo for the time when qemu 3.0 releases the needed bitmap/NBD features).
<dd>A combination of disk snapshots for all disks as well as VM memory state, which can be used to resume the guest from where it left off with symptoms similar to hibernation (that is, TCP @@ -55,7 +55,7 @@ as <code>virDomainSaveImageGetXMLDesc()</code> to work with those files. </p> - <p>System checkpoints are created + <p>Full system state snapshots are created by <code>virDomainSnapshotCreateXML()</code> with no flags, and disk snapshots are created by the same function with the <code>VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY</code> flag; in
BTW: Existing and maybe it's just me, but when I read a conjunctive sentence with only two parts I don't expect to see ", and" or ", or" - it's just "and" or "or" without the comma....
Thanks for the careful grammar/legibility review. I'll try to fold in those suggestions (again, might not make it into v2). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org