[PATCH v2 0/3]: Snapper support

Jeremy Allison jra at samba.org
Fri Jun 27 14:54:49 MDT 2014


On Fri, Jun 27, 2014 at 03:31:34PM +0200, David Disseldorp wrote:
> Snapper[1] is a snapshot management tool for Linux, that offers support
> for the creation and deletion of snapshots on btrfs and ext4
> thin-provisioned LVM volumes.
> 
> This patch-set adds and documents a new vfs_snapper module, which
> exposes snapshots managed by snapper to Windows clients, via the
> FSCTL_SRV_ENUM_SNAPS/FSCTL_GET_SHADOW_COPY_DATA ioctls and the
> @GMT/TWrp path tokens.
> 
> Demonstration video: http://youtu.be/RZq84ONWu1I
> 
> Changes since v1 (Thanks Jeremy):
> - Added missing previous file versions patch
> - Removed unused snapper_snap_path_to_id()

LGTM, pushed.

Things to clean up in a future patch (i.e.
these things bugged me, but not enough to
ask for another go around :-):

+static void snapper_conf_array_free(int32_t num_confs,
+                                   struct snapper_conf *confs)
+{
+       int i;
+
+       for (i = 0; i < num_confs; i++) {
+               talloc_free(confs[i].attrs);
+       }
+       talloc_free(confs);
+}

confs[i].attrs should be a talloc child of confs.

and

+static void snapper_snap_array_free(int32_t num_snaps,
+                                   struct snapper_snap *snaps)
+{
+       int i;
+
+       for (i = 0; i < num_snaps; i++) {
+               talloc_free(snaps[i].user_data);
+       }
+       talloc_free(snaps);
+}

snaps[i].user_data should be a talloc child of snaps.

That way, the free functions just become talloc_free(parent).

Plus I think you should look at the changes just
pushed to master by Volker and Christof, as I think
they may also be relevent in this module too.

Cheers,

	Jeremy.


More information about the samba-technical mailing list