Outstanding issues

[baodrate]

Issues with the script, prior to this fork:

• every call to snapper is incredibly slow on systems with quota enabled
  • see Long time waiting for snapper list openSUSE/snapper#629
  • snapper list will initiate a btrfs quota rescan at the top
• many calls over ssh, specifically this execution of btrfs subvolume show in a loop:
  https://github.com/qubidt/snap-sync/blob/000cc2bbac5563527f498fb5a2b70d6cc3f6d0f6/bin/snap-sync#L229-L233
  • cumulatively, this is pretty slow, taking nearly half a minute to iterate over the ~dozens of mounted subvolumes on my system
  • can likely leverage btrfs subvolume list to get the subvolume IDs in one call. and since we already have btrfs (btrfs-progs) as a dependency, this introduced no new dependencies
• script is completely broken w/ any paths that need escaping
  • e.g. whitespace, quotes, other special characters that get send as part of a command line that gets parsed by the whatever shell is on the other side of ssh
  • this one is difficult because we can't predict the user's shell. (my root uses zsh, more common are bash and posix-compatible shells. possibly fish??). we can't depend on bash-specific escaping rules
  • best solution is to avoid sending paths as part of an ssh command altogether. possible alternatives:
    • send a script that gets executed on the remote side, ansible style
    • use some other identifiers that don't need special parsing (e.g. subvolume UUIDs). This will need some cleverness to avoid working directly with mount paths
• everything is one long procedural script with a lot of global variables (and arrays)
  • this makes everything difficult to maintain and refactor. individual parts of the script need to be untangled and broken out into functions
• snap-sync stores data in the sender's snapper config to find the appropriate receiver location for future incremental updates
  • we might be able to avoid this. btrfs subvolume list -u -R will print the UUID of a subvolume as well as the sender's sent subvolume
  • have to be careful about how this feature is transitioned-to/deprecated
  • this dependency on the stored userdata is probably why snap-sync requires the target be specified by btrfs filesystem + subvolume ID + relative path
    • we could avoid all this, ideally, the user should be able to specify simpily the target path to backup to, and snap-sync will do checks to make sure it's a valid btrfs filesystem (perhaps prompting to user to confirm the filesystem/container-subvolume)
    • not 100% sure about this, but it seems like this like is wrong:

      -u, --UUID <UUID>        UUID of the mounted BTRFS subvolume to back up to
      

      the uuid seems to specify the BTRFS filesystem that is targeted
• consider trying to make this script more portable. the dependency on bash doesn't seem that useful
  • the only major feature we depend on is arrays which are used to pass data between different parts off the code, this is problematic in any case and are more a product of the current code layout than
  • special care is required when handling escapes
    • as said before, snap-sync is currently broken for paths that need escaping
    • we depend on findmnt which has options that escape the paths, but unfortunately they do hex-based escaping (e.g. `\x8F') which are a GNU extension and aren't supported by POSIX (as per POSIX.1_2017)
  • this is probably low-priority/unnecessary considering BTRFS hard-depends on Linux. Although Linux does not imply bash/GNU, it probably isn't too bad to require bash on the sender side (as is the status quo), very unlikely a system supports btrfs but doesn't provide bash)

summary:

bugs/improvements:

• avoid quota rescan when calling snapper (fixed w/ d557676)
• fix handling of paths with spaces/quotes/other special characters
• reduce ssh calls
• refactor script or maintainability

new feature proposals:

• use btrfs subvolume list -R to avoid having to store it in the snapper userdata
• replace uuid, subvolid, and backupdir with mounted-path
• make script portable sh