jmsperu commented on PR #12898: URL: https://github.com/apache/cloudstack/pull/12898#issuecomment-6018997135
@abh1sar thanks for the thorough review. All seven inline points are answered in their threads; the fixes are in f82d33f80b (management server), 1f596ed58f (nasbackup.sh) and d32f5707ab (restore wrapper). Local results: NASBackupProviderTest 29/29, LibvirtRestoreBackupCommandWrapperTest 24/24, LibvirtTakeBackupCommandWrapperTest 5/5, checkstyle clean. I also ran the script changes through a PATH-shimmed harness with real qemu-img, comparing old and new, for compression + encryption, `-r` with and without support, `ionice` gating, and `qemu-img check` exit codes 0, 2 and 3. One item is still open: confirming on a real host that `blockjob --bandwidth` applies to a push-mode backup job (see that thread). On your question about rotating the passphrase: yes, today a restore only gets the zone's current `nas.backup.encryption.passphrase`, so changing it makes backups taken with the old one unrestorable until the old value is set back. You were right to raise it. For now the setting's description says so explicitly (f82d33f80b), and a restore that cannot open the image fails rather than copying it. Proper rotation would mean keeping the passphrase per backup (for example, a reference stored in backup details that keeps old passphrases readable). I would rather do that as a follow-up than grow this PR. Would that work for you? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
