From 3dbd45ed4a1b7a29a3af36f071153a10137c2395 Mon Sep 17 00:00:00 2001 From: Marius van Witzenburg Date: Wed, 5 Jul 2017 00:37:48 +0200 Subject: [PATCH 1/4] Better handling for spaces in datasets --- zfs_autobackup | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/zfs_autobackup b/zfs_autobackup index f4fc34c..faf87e4 100755 --- a/zfs_autobackup +++ b/zfs_autobackup @@ -140,10 +140,10 @@ test_snapshots={} """create snapshot on multiple filesystems at once (atomicly)""" def zfs_create_snapshot(ssh_to, filesystems, snapshot): - cmd=[ "zfs", "snapshot" ] + snapshots=[] for filesystem in filesystems: - cmd.append(filesystem+"@"+snapshot) + snapshots.append(filesystem+"@"+snapshot) #in testmode we dont actually make changes, so keep them in a list to simulate if args.test: @@ -153,8 +153,9 @@ def zfs_create_snapshot(ssh_to, filesystems, snapshot): test_snapshots[ssh_to][filesystem]=[] test_snapshots[ssh_to][filesystem].append(snapshot) - run(ssh_to=ssh_to, tab_split=False, cmd=cmd, test=args.test) - + run(ssh_to=ssh_to, test=args.test, input="\0".join(snapshots), cmd= + [ "xargs", "-0", "-n", "1", "zfs", "snapshot" ] + ) """get names of all snapshots for specified filesystems belonging to backup_name @@ -162,12 +163,10 @@ def zfs_create_snapshot(ssh_to, filesystems, snapshot): return[filesystem_name]=[ "snashot1", "snapshot2", ... ] """ def zfs_get_snapshots(ssh_to, filesystems, backup_name): - cmd=[ - "zfs", "list", "-d", "1", "-r", "-t" ,"snapshot", "-H", "-o", "name" - ] - cmd.extend(filesystems) - snapshots=run(ssh_to=ssh_to, tab_split=False, cmd=cmd, valid_exitcodes=[ 0,1 ]) + snapshots=run(ssh_to=ssh_to, input="\0".join(filesystems), valid_exitcodes=[ 0,1 ], cmd= + [ "xargs", "-0", "-n", "1", "zfs", "list", "-d", "1", "-r", "-t" ,"snapshot", "-H", "-o", "name" ] + ) ret={} for snapshot in snapshots: @@ -228,8 +227,8 @@ def zfs_transfer(ssh_source, source_filesystem, first_snapshot, second_snapshot, else: verbose("Tranferring "+source_filesystem+" incremental backup between snapshots "+first_snapshot+"..."+second_snapshot) source_cmd.extend([ "-i", first_snapshot ]) - - source_cmd.append(source_filesystem + "@" + second_snapshot) + # FIXME needs attention + source_cmd.append(source_filesystem.replace(' ', '\ ') + "@" + second_snapshot) # if ssh_source != "local": # #add buffer @@ -251,14 +250,15 @@ def zfs_transfer(ssh_source, source_filesystem, first_snapshot, second_snapshot, #also verbose in --verbose mode so we can see the transfer speed when its completed if args.verbose or args.debug: target_cmd.append("-v") - - target_cmd.append(target_filesystem) + # FIXME needs attention + target_cmd.append(target_filesystem.replace(' ', '\ ')) #### make sure parent on target exists parent_filesystem= "/".join(target_filesystem.split("/")[:-1]) - run(ssh_to=ssh_target, cmd=[ "zfs", "create" ,"-p", parent_filesystem ], test=args.test) - + run(ssh_to=ssh_target, test=args.test, input=parent_filesystem + "\0", cmd= + [ "xargs", "-0", "-n", "1", "zfs", "create" ,"-p" ] + ) ### execute pipe debug_txt="# "+source_cmd[0]+" '"+("' '".join(source_cmd[1:]))+"'" + " | " + target_cmd[0]+" '"+("' '".join(target_cmd[1:]))+"'" @@ -269,6 +269,7 @@ def zfs_transfer(ssh_source, source_filesystem, first_snapshot, second_snapshot, else: debug(debug_txt) + # FIXME could break on spaces source_proc=subprocess.Popen(source_cmd, env=os.environ, stdout=subprocess.PIPE) target_proc=subprocess.Popen(target_cmd, env=os.environ, stdin=source_proc.stdout) source_proc.stdout.close() # Allow p1 to receive a SIGPIPE if p2 exits. @@ -475,7 +476,10 @@ for source_filesystem in source_filesystems: if send_snapshots and args.rollback and latest_target_snapshot: #roll back any changes on target debug("Rolling back target to latest snapshot.") - run(ssh_to=args.ssh_target, test=args.test, cmd=["zfs", "rollback", target_filesystem+"@"+latest_target_snapshot ]) + + run(ssh_to=args.ssh_target, test=args.test, input=target_filesystem+"@"+latest_target_snapshot + "\0", cmd= + [ "xargs", "-0", "-n", "1", "zfs", "rollback" ] + ) for send_snapshot in send_snapshots: @@ -491,11 +495,19 @@ for source_filesystem in source_filesystems: else: if args.clear_refreservation: debug("Clearing refreservation to save space.") - run(ssh_to=args.ssh_target, test=args.test, cmd=["zfs", "set", "refreservation=none", target_filesystem ]) + + run(ssh_to=args.ssh_target, test=args.test, input=target_filesystem + "\0", cmd= + [ "xargs", "-0", "-n", "1", "zfs", "set", "refreservation=none" ] + ) + if args.clear_mountpoint: debug("Setting canmount=noauto to prevent auto-mounting in the wrong place. (ignoring errors)") - run(ssh_to=args.ssh_target, test=args.test, cmd=["zfs", "set", "canmount=noauto", target_filesystem ], valid_exitcodes= [0, 1] ) + + run(ssh_to=args.ssh_target, test=args.test, input=target_filesystem + "\0", valid_exitcodes= [0, 1], cmd= + [ "xargs", "-0", "-n", "1", "zfs", "set", "canmount=noauto" ] + ) + latest_target_snapshot=send_snapshot From 748e6d473930b60d31e3f586f5af44f6ad08d2cf Mon Sep 17 00:00:00 2001 From: Marius van Witzenburg Date: Wed, 5 Jul 2017 12:14:51 +0200 Subject: [PATCH 2/4] Handle local and remote dataset escaping differently --- zfs_autobackup | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/zfs_autobackup b/zfs_autobackup index faf87e4..dd4ac48 100755 --- a/zfs_autobackup +++ b/zfs_autobackup @@ -228,7 +228,10 @@ def zfs_transfer(ssh_source, source_filesystem, first_snapshot, second_snapshot, verbose("Tranferring "+source_filesystem+" incremental backup between snapshots "+first_snapshot+"..."+second_snapshot) source_cmd.extend([ "-i", first_snapshot ]) # FIXME needs attention - source_cmd.append(source_filesystem.replace(' ', '\ ') + "@" + second_snapshot) + if ssh_source != "local": + source_cmd.append(source_filesystem.replace(' ', '\ ') + "@" + second_snapshot) + else: + source_cmd.append(source_filesystem + "@" + second_snapshot) # if ssh_source != "local": # #add buffer @@ -251,8 +254,10 @@ def zfs_transfer(ssh_source, source_filesystem, first_snapshot, second_snapshot, if args.verbose or args.debug: target_cmd.append("-v") # FIXME needs attention - target_cmd.append(target_filesystem.replace(' ', '\ ')) - + if ssh_target != "local": + target_cmd.append(target_filesystem.replace(' ', '\ ')) + else: + target_cmd.append(target_filesystem) #### make sure parent on target exists parent_filesystem= "/".join(target_filesystem.split("/")[:-1]) From 6b5edab09da2b97bf6e574332818fa79ab41d18c Mon Sep 17 00:00:00 2001 From: Marius van Witzenburg Date: Wed, 5 Jul 2017 12:15:08 +0200 Subject: [PATCH 3/4] Escape dataset verify command --- zfs_autobackup | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/zfs_autobackup b/zfs_autobackup index dd4ac48..3449838 100755 --- a/zfs_autobackup +++ b/zfs_autobackup @@ -288,7 +288,9 @@ def zfs_transfer(ssh_source, source_filesystem, first_snapshot, second_snapshot, raise(subprocess.CalledProcessError(target_proc.returncode, target_cmd)) debug("Verifying if snapshot exists on target") - run(ssh_to=ssh_target, cmd=["zfs", "list", target_filesystem+"@"+second_snapshot ]) + run(ssh_to=ssh_target, input=target_filesystem+"@"+second_snapshot + "\0", cmd= + [ "xargs", "-0", "-n", "1", "zfs", "list" ] + ) From 3e694e09289676a1404300628447e5fd54c0eca7 Mon Sep 17 00:00:00 2001 From: Marius van Witzenburg Date: Wed, 5 Jul 2017 13:05:07 +0200 Subject: [PATCH 4/4] Cleaning up --- zfs_autobackup | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zfs_autobackup b/zfs_autobackup index 3449838..caf9b95 100755 --- a/zfs_autobackup +++ b/zfs_autobackup @@ -274,7 +274,6 @@ def zfs_transfer(ssh_source, source_filesystem, first_snapshot, second_snapshot, else: debug(debug_txt) - # FIXME could break on spaces source_proc=subprocess.Popen(source_cmd, env=os.environ, stdout=subprocess.PIPE) target_proc=subprocess.Popen(target_cmd, env=os.environ, stdin=source_proc.stdout) source_proc.stdout.close() # Allow p1 to receive a SIGPIPE if p2 exits. @@ -538,7 +537,7 @@ target_obsolete_snapshots.update(stale_target_snapshots) stale_target_destroys=[] for stale_target_filesystem in stale_target_filesystems: if stale_target_filesystem not in stale_target_snapshots: - stale_target_destroys.append(stale_target_filesystem) + stale_target_destroys.append(stale_target_filesystem) if stale_target_destroys: if args.destroy_stale: