[pve-devel] [pve-zsync] Discussion/Feedback: Extending pve-zsync to handle intervening snapshots

[Bruce Wainer]

 Hello,

I am considering making an improvement to pve-zsync and submitting it as a
patch, but I would like some feedback before doing so.

Problem:
pve-zsync can't be used multiple times between the same source and
destination pool. For example, these two commands are allowed:
pve-zsync create --source 192.168.10.18:101 --name 101-daily --dest
192.168.10.14:HDDs/replica-daily
pve-zsync create --source 192.168.10.18:101 --name 101-weekly --dest
192.168.10.14:HDDs/replica-weekly
But these two are not:
pve-zsync create --source 192.168.10.18:101 --name 101-daily --dest
192.168.10.14:HDDs/replica
pve-zsync create --source 192.168.10.18:101 --name 101-weekly --dest
192.168.10.14:HDDs/replica

The first set of commands work together because the destination volumes are
different. However, this means that the destination host will have
duplicate copies of the source. This is not ideal. The second set of
commands will not work because the last_snap used for the zfs send/recv
only takes into account the current job.

Proposal
Flip the source and destination in snapshot_get() and snapshot_exists(),
because if the last snapshot on the destination doesn't exist on the
source, the sync is going to fail anyway (ZFS will error with "cannot
receive new filesystem stream: destination has snapshots").
1. Add a new command option "use_last_dest_snapshot" (add to help, to cron
generation, etc). If this is set, the new logic will be used.
2. Add "sub snapshot_get_dest" which is similar to snapshot_get() except it
performs its checks on the destination, not the source. Also, last_snap
would be the last snapshot of all, not just the last one related to the
current job.
3. Add "sub snapshot_exists_source" which is the same as snapshot_exists()
except it performs its checks on the source, not the destination. I would
also improve the error message if the snapshot doesn't exist on the source.
4. In "sub send_image", if " use_last_dest_snapshot " is set then the new
functions would be called, otherwise the old functions would be called.

This will be my first time submitting changes to a project, but I believe I
am capable of writing code that meets your code standards. I am aware of
the style guide and will submit the CLA.

Please let me know what you think about my proposed changes.

Thank you,
Bruce Wainer