search

jimsalterjrs / sanoid

These are policy-driven snapshot management and replication tools which use OpenZFS for underlying next-gen storage. (Btrfs support plans are shelved unless and until btrfs becomes reliable.)
http://www.openoid.net/products/
GNU General Public License v3.0
3.13k stars 308 forks source link

Sanoid/Syncoid completly fails in Solaris due to zfs get call - fix included. #777

Open brendon0 opened 2 years ago

brendon0 commented 2 years ago

In Solaris the "zfs get" function call fails as there is no "-t" option in Solaris, however this exists in Linux / OpenZFS

This is referenced in line 808 of the 2.1.0 Sanoid code:

open FH, "$zfs get -Hrpt snapshot creation |";

Outputs: root@SolarisZFS:/opt/csw/bin# zfs get -Hrpt snapshot creation invalid option 't' For more info, run: zfs help get

This however works fine in Linux as the "zfs get" function does have a "-t" option.

Outputs: root@ubuntuzfs:/etc/sanoid# zfs get -Hrpt snapshot creation flash@test creation 1665337414 - flash/smb/documents@autosnap_2022-10-09_17:45:27_monthly creation 1665337527 - flash/smb/documents@autosnap_2022-10-09_17:45:27_daily creation 1665337527 -

On further investigation, the "zfs get -t snapshot" is basically just doing a "zfs get all" and filtering by snapshots.

An alternative method of doing this is using grep and filtering by the "@" symbol. This will make this function call compatible with both Linux and Solaris with a single line of code change.

"zfs get -Hrp creation | grep @"

Output: root@SolarisZFS:/opt/csw/bin# zfs get -Hrp creation | grep @ flash/smb/documents@autosnap_2022-10-10_17:06:42_monthly creation 1665436002 - flash/smb/documents@autosnap_2022-10-10_17:06:42_daily creation 1665436002 -

I am proposing the following code change to ensure compatibility with both Solaris and Linux/OpenZFS derivatives of ZFS. Change line 808 of the code to the following:

open FH, "$zfs get -Hrp creation | grep @ |";

This has been tested as working in both my Solaris and Linux installs.

Also I believe this fix will work in Syncoid as well as it also uses the "-t" filter however I have not had time to test it.

jimsalterjrs commented 2 years ago

I appreciate the fix, but I'm not happy with piping through grep. This introduces new and exciting ways for things to fail on different distros when grep may or may not be in the PATH, etc.

If we're going to filter that way, I'd strongly prefer just getting -Hrp creation, then in Perl just destroying elements that don't contain an @. That way we don't cause further headaches down the road. This can be accomplished with a simple check along the lines of if ($str =~ /\@/) { do stuff; } or the inverse.

brendon0 commented 2 years ago

Jim, Totally get it. I dont write PERL tho, so I just put a quick hack in so I can get my solaris host running. This is why I didn't do a pull andjust created a ticket.

I am digging into the Syncoid code right now and it looks like someone tried to fix already for the solaris missing feature. There is a whole fallback function....

Line 1575: sub getsnapsfallback() {     # fallback (solaris for example doesn't support the -t option)

I am not running Syncoid so I cant really test this, however maybe this code should make its way over to the Sanoid side? Like I said I dont write PERL...

Happy to test any code changes on Solaris... I have a OmniOS host as well I can test with.