Hello, I think that's a good idea.
However, I am not sure whether the additional flag described in point 1 is necessary, since old jobs already have an independent dest dataset. So the new logic should not conflict it. I think it would be great to rewrite the "snapshot_get" and "snapshot_exists" functions to fit this use case, In my opinion it makes little sense to copy the function and only change dest and source and give the function a new name. Regards, Wolfgang > On 06/15/2020 9:50 PM Bruce Wainer <brwai...@gmail.com> wrote: > > > 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 > _______________________________________________ > pve-devel mailing list > pve-devel@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
