[PATCH] Add a somewhat hackish option to limit the size of TimeMachine backups

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add a somewhat hackish option to limit the size of TimeMachine backups

Samba - samba-technical mailing list
Hi folks!

Attached is a patchs adding a new option to vfs_fruit that allows to limit the
size used by Timemachine backups.

This feature has been used in Netatalk for many years and some users want the
same feature in Samba.

How does it work? I wish you didn't ask... ;)

Timemachine backups are use a so called Sparsebundle image per client which
consists of a directory names after the client "CLIENT.sparsebundle" with the
following directory structure:

  /shares/test/test.sparsebundle/
  ├── bands
  │   ├── 1
  │   ├...
  │   └── N
  └── Info.plist
 
  1 directory, 3 files

Iirc there are a few more foles inside the top-level directory, but those are
not relevant here.

The image uses striped files in the bands/ subdirectory instead of a single
monolithic large file.

The Info.plist file is an XML file that amonst other backup metadata contains
the stripe-size of the files in the bands/ subdirectory.

A single Samba share can contain the backups of N Macs, so to calculate the
total used size of the backups we can do this:

- readdir(basedir of share) and for every element that matches regex
  "^\(.*\)\.sparsebundle$"
  - parse "\1.sparsebundle/Info.plist" and read the band-size XML key
  - count band files in "\1.sparsebundle/bands/"
  - calculate used size of all bands: band_count * band_size

This is completely hackish but has been in use by many Netatalk users for years.

Why not use quotas? I asked the same question. Answer:

Quotas are ideal where possible, but they can be problematic depending
on the filesystem being used.  Ext[234] can only apply one quota limit
per user, per filesystem, which really limits flexibility.  Btrfs quota
support is still buggy, and can cause severe performance problems for
some operations.

Why not use "dfree command" ? 1: Lower overhead. 2: Because I wanted to provide
a solution that works ootb with Samba without a dependency on a script.

The good thing: it has a test! :)

Please review & push if happy. Thanks!

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

fruit-tm-volsize.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add a somewhat hackish option to limit the size of TimeMachine backups

Samba - samba-technical mailing list
On Wed, Jan 03, 2018 at 11:04:30AM +0100, Ralph Böhme wrote:

> Hi folks!
>
> Attached is a patchs adding a new option to vfs_fruit that allows to limit the
> size used by Timemachine backups.
>
> This feature has been used in Netatalk for many years and some users want the
> same feature in Samba.
>
> How does it work? I wish you didn't ask... ;)
>
> Timemachine backups are use a so called Sparsebundle image per client which
> consists of a directory names after the client "CLIENT.sparsebundle" with the
> following directory structure:
>
>   /shares/test/test.sparsebundle/
>   ├── bands
>   │   ├── 1
>   │   ├...
>   │   └── N
>   └── Info.plist
>  
>   1 directory, 3 files
>
> Iirc there are a few more foles inside the top-level directory, but those are
> not relevant here.
>
> The image uses striped files in the bands/ subdirectory instead of a single
> monolithic large file.
>
> The Info.plist file is an XML file that amonst other backup metadata contains
> the stripe-size of the files in the bands/ subdirectory.
>
> A single Samba share can contain the backups of N Macs, so to calculate the
> total used size of the backups we can do this:
>
> - readdir(basedir of share) and for every element that matches regex
>   "^\(.*\)\.sparsebundle$"
>   - parse "\1.sparsebundle/Info.plist" and read the band-size XML key
>   - count band files in "\1.sparsebundle/bands/"
>   - calculate used size of all bands: band_count * band_size
>
> This is completely hackish but has been in use by many Netatalk users for years.
>
> Why not use quotas? I asked the same question. Answer:
>
> Quotas are ideal where possible, but they can be problematic depending
> on the filesystem being used.  Ext[234] can only apply one quota limit
> per user, per filesystem, which really limits flexibility.  Btrfs quota
> support is still buggy, and can cause severe performance problems for
> some operations.
>
> Why not use "dfree command" ? 1: Lower overhead. 2: Because I wanted to provide
> a solution that works ootb with Samba without a dependency on a script.
>
> The good thing: it has a test! :)
>
> Please review & push if happy. Thanks!

Oh my god, that is a *horrible* hack :-(. Reviewing it now...

The *ONLY* reason I'm considering this and not immediately
rejecting it is due to it's vintage Netatalk heritage, and
the fact that it only affects vfs_fruit :-). We need to
keep the Mac users happy :).

If I saw this for any other VFS module I'd immediately say
this:

https://www.thinkgeek.com/product/374d/

Should get done with the review by tomorrow...

Jeremy.

> --
> Ralph Boehme, Samba Team       https://samba.org/
> Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

> From 812a036e50c82cd021d3e5543aaf5acb407098c0 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <[hidden email]>
> Date: Fri, 3 Nov 2017 10:56:29 +0100
> Subject: [PATCH 1/2] vfs_fruit: add "time machine max size" option
>
> This can be used to configure a per client filesystem size limit on
> TimeMachine shares.
>
> It's a nasty hack but it was reportedly working well in Netatalk where
> it's taken from.
>
> Signed-off-by: Ralph Boehme <[hidden email]>
> ---
>  docs-xml/manpages/vfs_fruit.8.xml |  18 ++
>  source3/modules/vfs_fruit.c       | 421 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 439 insertions(+)
>
> diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml
> index fcaf1737ce8..7f6a0e7c022 100644
> --- a/docs-xml/manpages/vfs_fruit.8.xml
> +++ b/docs-xml/manpages/vfs_fruit.8.xml
> @@ -242,6 +242,24 @@
>    </varlistentry>
>  
>    <varlistentry>
> +    <term>fruit:time machine max size = SIZE [K|M|G|T|P]</term>
> +    <listitem>
> +      <para>Useful for Time Machine: limits the reported disksize, thus
> +      preventing Time Machine from using the whole real disk space for
> +      backup. The option takes a number plus an optional unit.</para>
> +      <para><emphasis>IMPORTANT</emphasis>: This is an approximated
> +      calculation that only takes into account the contents of Time
> +      Machine sparsebundle images. Therefor you <emphasis>MUST
> +      NOT</emphasis> use this volume to store other content when using
> +      this option, because it would NOT be accounted.</para>
> +      <para>The calculation works by reading the band size from the
> +      Info.plist XML file of the sparsebundle, reading the bands/
> +      directory counting the number of band files, and then multiplying
> +      one with the other.</para>
> +    </listitem>
> +  </varlistentry>
> +
> +  <varlistentry>
>      <term>fruit:metadata = [ stream | netatalk ]</term>
>      <listitem>
>        <para>Controls where the OS X metadata stream is stored:</para>
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 67dd571c627..e7b4e220a54 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -141,6 +141,7 @@ struct fruit_config_data {
>   bool aapl_zero_file_id;
>   const char *model;
>   bool time_machine;
> + size_t time_machine_max_size;
>  
>   /*
>   * Additional options, all enabled by default,
> @@ -1885,6 +1886,7 @@ static int init_fruit_config(vfs_handle_struct *handle)
>  {
>   struct fruit_config_data *config;
>   int enumval;
> + const char *tm_size_str = NULL;
>  
>   config = talloc_zero(handle->conn, struct fruit_config_data);
>   if (!config) {
> @@ -1983,6 +1985,14 @@ static int init_fruit_config(vfs_handle_struct *handle)
>   config->model = lp_parm_const_string(
>   -1, FRUIT_PARAM_TYPE_NAME, "model", "MacSamba");
>  
> + tm_size_str = lp_parm_const_string(
> + SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME,
> + "time machine max size", NULL);
> + if (tm_size_str != NULL) {
> + config->time_machine_max_size =
> + (size_t)conv_str_size(tm_size_str);
> + }
> +
>   SMB_VFS_HANDLE_SET_DATA(handle, config,
>   NULL, struct fruit_config_data,
>   return -1);
> @@ -6003,8 +6013,419 @@ static NTSTATUS fruit_offload_write_recv(struct vfs_handle_struct *handle,
>   return NT_STATUS_OK;
>  }
>  
> +static char *fruit_get_bandsize_line(char **lines, int numlines)
> +{
> + static regex_t re;
> + static bool re_initialized = false;
> + int i;
> + int ret;
> +
> + if (!re_initialized) {
> + ret = regcomp(&re, "^[[:blank:]]*<key>band-size</key>$", 0);
> + if (ret != 0) {
> + return NULL;
> + }
> + re_initialized = true;
> + }
> +
> + for (i = 0; i < numlines; i++) {
> + regmatch_t matches[1];
> +
> + ret = regexec(&re, lines[i], 1, matches, 0);
> + if (ret == 0) {
> + /*
> + * Check if the match was on the last line, sa we want
> + * the subsequent line.
> + */
> + if (i + 1 == numlines) {
> + return NULL;
> + }
> + return lines[i + 1];
> + }
> + if (ret != REG_NOMATCH) {
> + return NULL;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static bool fruit_get_bandsize_from_line(char *line, size_t *band_size)
> +{
> + static regex_t re;
> + static bool re_initialized = false;
> + regmatch_t matches[2];
> + uint64_t uint64;
> + int ret;
> + bool ok;
> +
> + if (!re_initialized) {
> + ret = regcomp(&re,
> +      "^[[:blank:]]*"
> +      "<integer>\\([[:digit:]]*\\)</integer>$",
> +      0);
> + if (ret != 0) {
> + return false;
> + }
> + re_initialized = true;
> + }
> +
> + ret = regexec(&re, line, 2, matches, 0);
> + if (ret != 0) {
> + DBG_ERR("regex failed [%s]\n", line);
> + return false;
> + }
> +
> + line[matches[1].rm_eo] = '\0';
> +
> + ok = conv_str_u64(&line[matches[1].rm_so], &uint64);
> + if (!ok) {
> + return false;
> + }
> + *band_size = (size_t)uint64;
> + return true;
> +}
> +
> +/*
> + * This reads and parses an Info.plist from a TM sparsebundle looking for the
> + * "band-size" key and value.
> + */
> +static bool fruit_get_bandsize(vfs_handle_struct *handle,
> +       const char *dir,
> +       size_t *band_size)
> +{
> +#define INFO_PLIST_MAX_SIZE 64*1024
> + char *plist = NULL;
> + struct smb_filename *smb_fname = NULL;
> + files_struct *fsp = NULL;
> + uint8_t *file_data = NULL;
> + char **lines = NULL;
> + char *band_size_line = NULL;
> + size_t plist_file_size;
> + ssize_t nread;
> + int numlines;
> + int ret;
> + bool ok = false;
> + NTSTATUS status;
> +
> + plist = talloc_asprintf(talloc_tos(),
> + "%s/%s/Info.plist",
> + handle->conn->connectpath,
> + dir);
> + if (plist == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + smb_fname = synthetic_smb_fname(talloc_tos(), plist, NULL, NULL, 0);
> + if (smb_fname == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + ret = SMB_VFS_NEXT_LSTAT(handle, smb_fname);
> + if (ret != 0) {
> + DBG_INFO("Ignoring Sparsebundle without Info.plist [%s]\n", dir);
> + ok = true;
> + goto out;
> + }
> +
> + plist_file_size = smb_fname->st.st_ex_size;
> +
> + if (plist_file_size > INFO_PLIST_MAX_SIZE) {
> + DBG_INFO("%s is too large, ignoring\n", plist);
> + ok = true;
> + goto out;
> + }
> +
> + status = SMB_VFS_NEXT_CREATE_FILE(
> + handle, /* conn */
> + NULL, /* req */
> + 0, /* root_dir_fid */
> + smb_fname, /* fname */
> + FILE_GENERIC_READ, /* access_mask */
> + FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
> + FILE_OPEN, /* create_disposition */
> + 0, /* create_options */
> + 0, /* file_attributes */
> + INTERNAL_OPEN_ONLY, /* oplock_request */
> + NULL, /* lease */
> + 0, /* allocation_size */
> + 0, /* private_flags */
> + NULL, /* sd */
> + NULL, /* ea_list */
> + &fsp, /* result */
> + NULL, /* psbuf */
> + NULL, NULL); /* create context */
> + if (!NT_STATUS_IS_OK(status)) {
> + DBG_INFO("Opening [%s] failed [%s]\n",
> + smb_fname_str_dbg(smb_fname), nt_errstr(status));
> + ok = false;
> + goto out;
> + }
> +
> + file_data = talloc_array(talloc_tos(), uint8_t, plist_file_size);
> + if (file_data == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + nread = SMB_VFS_NEXT_PREAD(handle, fsp, file_data, plist_file_size, 0);
> + if (nread != plist_file_size) {
> + DBG_ERR("Short read on [%s]: %zu/%zd\n",
> + fsp_str_dbg(fsp), nread, plist_file_size);
> + ok = false;
> + goto out;
> +
> + }
> +
> + status = close_file(NULL, fsp, NORMAL_CLOSE);
> + fsp = NULL;
> + if (!NT_STATUS_IS_OK(status)) {
> + DBG_ERR("close_file failed: %s\n", nt_errstr(status));
> + ok = false;
> + goto out;
> + }
> +
> + lines = file_lines_parse((char *)file_data,
> + plist_file_size,
> + &numlines,
> + talloc_tos());
> + if (lines == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + band_size_line = fruit_get_bandsize_line(lines, numlines);
> + if (band_size_line == NULL) {
> + DBG_ERR("Didn't find band-size key in [%s]\n",
> + smb_fname_str_dbg(smb_fname));
> + ok = false;
> + goto out;
> + }
> +
> + ok = fruit_get_bandsize_from_line(band_size_line, band_size);
> + if (!ok) {
> + DBG_ERR("fruit_get_bandsize_from_line failed\n");
> + goto out;
> + }
> +
> + DBG_DEBUG("Parsed band-size [%zu] for [%s]\n", *band_size, plist);
> +
> +out:
> + if (fsp != NULL) {
> + status = close_file(NULL, fsp, NORMAL_CLOSE);
> + if (!NT_STATUS_IS_OK(status)) {
> + DBG_ERR("close_file failed: %s\n", nt_errstr(status));
> + }
> + fsp = NULL;
> + }
> + TALLOC_FREE(plist);
> + TALLOC_FREE(smb_fname);
> + TALLOC_FREE(file_data);
> + TALLOC_FREE(lines);
> + return ok;
> +}
> +
> +struct fruit_disk_free_state {
> + size_t total_size;
> +};
> +
> +static bool fruit_get_num_bands(vfs_handle_struct *handle,
> + char *bundle,
> + size_t *_nbands)
> +{
> + char *path = NULL;
> + struct smb_filename *bands_dir = NULL;
> + DIR *d = NULL;
> + struct dirent *e = NULL;
> + size_t nbands;
> + int ret;
> +
> + path = talloc_asprintf(talloc_tos(),
> +       "%s/%s/bands",
> +       handle->conn->connectpath,
> +       bundle);
> + if (path == NULL) {
> + return false;
> + }
> +
> + bands_dir = synthetic_smb_fname(talloc_tos(),
> + path,
> + NULL,
> + NULL,
> + 0);
> + TALLOC_FREE(path);
> + if (bands_dir == NULL) {
> + return false;
> + }
> +
> +
> + d = SMB_VFS_NEXT_OPENDIR(handle, bands_dir, NULL, 0);
> + if (d == NULL) {
> + TALLOC_FREE(bands_dir);
> + return false;
> + }
> +
> + nbands = 0;
> +
> + for (e = SMB_VFS_NEXT_READDIR(handle, d, NULL);
> +     e != NULL;
> +     e = SMB_VFS_NEXT_READDIR(handle, d, NULL))
> + {
> + if (ISDOT(e->d_name) || ISDOTDOT(e->d_name)) {
> + continue;
> + }
> + nbands++;
> + }
> +
> + ret = SMB_VFS_NEXT_CLOSEDIR(handle, d);
> + if (ret != 0) {
> + TALLOC_FREE(bands_dir);
> + return false;
> + }
> +
> + DBG_DEBUG("%zu bands in [%s]\n", nbands, smb_fname_str_dbg(bands_dir));
> +
> + TALLOC_FREE(bands_dir);
> +
> + *_nbands = nbands;
> + return true;
> +}
> +
> +static bool fruit_tmsize_do_dirent(vfs_handle_struct *handle,
> +   struct fruit_disk_free_state *state,
> +   struct dirent *e)
> +{
> + bool ok;
> + char *p = NULL;
> + size_t sparsebundle_strlen = strlen("sparsebundle");
> + size_t bandsize;
> + size_t nbands;
> + double tm_size;
> +
> + p = strstr(e->d_name, "sparsebundle");
> + if (p == NULL) {
> + return true;
> + }
> +
> + if (p[sparsebundle_strlen] != '\0') {
> + return true;
> + }
> +
> + DBG_DEBUG("Processing sparsebundle [%s]\n", e->d_name);
> +
> + ok = fruit_get_bandsize(handle, e->d_name, &bandsize);
> + if (!ok) {
> + /*
> + * Beware of race conditions: this may be an uninitialued
> + * Info.plist that a client is just creating. We don't want let
> + * this to trigger complete failure.
> + */
> + DBG_ERR("Processing sparsebundle [%s] failed\n", e->d_name);
> + return true;
> + }
> +
> + ok = fruit_get_num_bands(handle, e->d_name, &nbands);
> + if (!ok) {
> + /*
> + * Beware of race conditions: this may be an uninitialued
> + * Info.plist that a client is just creating. We don't want let
> + * this to trigger complete failure.
> + */
> + DBG_ERR("Processing sparsebundle [%s] failed\n", e->d_name);
> + return true;
> + }
> +
> + tm_size = bandsize * nbands;
> + if (tm_size > UINT64_MAX) {
> + DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
> + bandsize, nbands);
> + return false;
> + }
> +
> + if (state->total_size + tm_size < state->total_size) {
> + DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
> + bandsize, nbands);
> + return false;
> + }
> +
> + state->total_size += tm_size;
> +
> + DBG_DEBUG("[%s] tm_size [%.0f] total_size [%zu]\n",
> +  e->d_name, tm_size, state->total_size);
> +
> + return true;
> +}
> +
> +/**
> + * Calculate used size of a TimeMachine volume
> + *
> + * This assumes that the volume is used only for TimeMachine.
> + *
> + * - readdir(basedir of share), then
> + * - for every element that matches regex "^\(.*\)\.sparsebundle$" :
> + * - parse "\1.sparsebundle/Info.plist" and read the band-size XML key
> + * - count band files in "\1.sparsebundle/bands/"
> + * - calculate used size of all bands: band_count * band_size
> + **/
> +static uint64_t fruit_disk_free(vfs_handle_struct *handle,
> + const struct smb_filename *smb_fname,
> + uint64_t *bsize,
> + uint64_t *dfree,
> + uint64_t *dsize)
> +{
> + struct fruit_config_data *config = NULL;
> + struct fruit_disk_free_state state = {0};
> + DIR *d = NULL;
> + struct dirent *e = NULL;
> + int ret;
> + bool ok;
> +
> + SMB_VFS_HANDLE_GET_DATA(handle, config,
> + struct fruit_config_data,
> + return UINT64_MAX);
> +
> + if (!config->time_machine ||
> +    config->time_machine_max_size == 0)
> + {
> + return SMB_VFS_NEXT_DISK_FREE(handle,
> +      smb_fname,
> +      bsize,
> +      dfree,
> +      dsize);
> + }
> +
> + d = SMB_VFS_NEXT_OPENDIR(handle, smb_fname, NULL, 0);
> + if (d == NULL) {
> + return UINT64_MAX;
> + }
> +
> + for (e = SMB_VFS_NEXT_READDIR(handle, d, NULL);
> +     e != NULL;
> +     e = SMB_VFS_NEXT_READDIR(handle, d, NULL))
> + {
> + ok = fruit_tmsize_do_dirent(handle, &state, e);
> + if (!ok) {
> + SMB_VFS_NEXT_CLOSEDIR(handle, d);
> + return UINT64_MAX;
> + }
> + }
> +
> + ret = SMB_VFS_NEXT_CLOSEDIR(handle, d);
> + if (ret != 0) {
> + return UINT64_MAX;
> + }
> +
> + *bsize = 512;
> + *dsize = config->time_machine_max_size / 512;
> + *dfree = *dsize - (state.total_size / 512);
> + return *dfree / 2;
> +}
> +
>  static struct vfs_fn_pointers vfs_fruit_fns = {
>   .connect_fn = fruit_connect,
> + .disk_free_fn = fruit_disk_free,
>  
>   /* File operations */
>   .chmod_fn = fruit_chmod,
> --
> 2.13.6
>
>
> From 090b7d80362be4fa1382d98b120dd83aeac81ab7 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <[hidden email]>
> Date: Tue, 2 Jan 2018 19:09:04 +0100
> Subject: [PATCH 2/2] s4/torture: test vfs_fruit "fruit:time machine max size"
>  option
>
> Signed-off-by: Ralph Boehme <[hidden email]>
> ---
>  selftest/target/Samba3.pm   |  1 +
>  source4/torture/vfs/fruit.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
>
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index c9888c2dca3..c483dfb1144 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -1934,6 +1934,7 @@ sub provision($$$$$$$$$)
>   fruit:locking = netatalk
>   fruit:encoding = native
>   fruit:veto_appledouble = no
> + fruit:time machine max size = 10G
>  
>  [vfs_fruit_metadata_stream]
>   path = $shrdir
> diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
> index 04f04e2cd56..0975b86b585 100644
> --- a/source4/torture/vfs/fruit.c
> +++ b/source4/torture/vfs/fruit.c
> @@ -4344,6 +4344,94 @@ done:
>   return ok;
>  }
>  
> +static bool test_timemachine_volsize(struct torture_context *tctx,
> +     struct smb2_tree *tree)
> +{
> + TALLOC_CTX *mem_ctx = talloc_new(tctx);
> + struct smb2_handle h = {{0}};
> + union smb_fsinfo fsinfo;
> + NTSTATUS status;
> + bool ok = true;
> + const char *info_plist =
> + "<dict>\n"
> + "        <key>band-size</key>\n"
> + "        <integer>8192</integer>\n"
> + "</dict>\n";
> +
> + smb2_deltree(tree, "test.sparsebundle");
> +
> + ok = enable_aapl(tctx, tree);
> + torture_assert_goto(tctx, ok, ok, done, "enable_aapl failed");
> +
> + status = smb2_util_mkdir(tree, "test.sparsebundle");
> + torture_assert_ntstatus_ok_goto(tctx, status, ok, done,
> + "smb2_util_mkdir\n");
> +
> + ok = write_stream(tree, __location__, tctx, mem_ctx,
> +  "test.sparsebundle/Info.plist", NULL,
> +   0, strlen(info_plist), info_plist);
> + torture_assert_goto(tctx, ok, ok, done, "write_stream failed\n");
> +
> + status = smb2_util_mkdir(tree, "test.sparsebundle/bands");
> + torture_assert_ntstatus_ok_goto(tctx, status, ok, done,
> + "smb2_util_mkdir\n");
> +
> + ok = torture_setup_file(tctx, tree, "test.sparsebundle/bands/1", false);
> + torture_assert_goto(tctx, ok, ok, done, "torture_setup_file failed\n");
> +
> + ok = torture_setup_file(tctx, tree, "test.sparsebundle/bands/2", false);
> + torture_assert_goto(tctx, ok, ok, done, "torture_setup_file failed\n");
> +
> + status = smb2_util_roothandle(tree, &h);
> + torture_assert_ntstatus_ok(tctx, status, "Unable to create root handle");
> +
> + ZERO_STRUCT(fsinfo);
> + fsinfo.generic.level = RAW_QFS_SIZE_INFORMATION;
> + fsinfo.generic.handle = h;
> +
> + status = smb2_getinfo_fs(tree, tree, &fsinfo);
> + torture_assert_ntstatus_ok(tctx, status, "smb2_getinfo_fs failed");
> +
> + torture_comment(tctx, "sectors_per_unit: %" PRIu32"\n"
> + "bytes_per_sector: %" PRIu32"\n"
> + "total_alloc_units: %" PRIu64"\n"
> + "avail_alloc_units: %" PRIu64"\n",
> + fsinfo.size_info.out.sectors_per_unit,
> + fsinfo.size_info.out.bytes_per_sector,
> + fsinfo.size_info.out.total_alloc_units,
> + fsinfo.size_info.out.avail_alloc_units);
> +
> + /*
> + * Let me explain the numbers:
> + *
> + * - the share is set to "fruit:time machine max size = 20G"
> + * - we've faked a bandsize of 8 K in the Info.plist file
> + * - we've created two bands files
> + * - one allocation unit is made of two sectors with 512 B each
> + * => we've consumed 16 allocation units
> + */
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.sectors_per_unit == 2,
> +    ok, done, "Bad total_alloc_units");
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.bytes_per_sector == 512,
> +    ok, done, "Bad total_alloc_units");
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.total_alloc_units == 20971520,
> +    ok, done, "Bad total_alloc_units");
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.avail_alloc_units == 20971504,
> +    ok, done, "Bad total_alloc_units");
> +
> +done:
> + if (!smb2_util_handle_empty(h)) {
> + smb2_util_close(tree, h);
> + }
> + smb2_deltree(tree, "test.sparsebundle");
> + talloc_free(mem_ctx);
> + return ok;
> +}
> +
>  /*
>   * Note: This test depends on "vfs objects = catia fruit streams_xattr".  For
>   * some tests torture must be run on the host it tests and takes an additional
> @@ -4384,6 +4472,7 @@ struct torture_suite *torture_vfs_fruit(TALLOC_CTX *ctx)
>   torture_suite_add_1smb2_test(suite, "creating rsrc with read-only access", test_rfork_create_ro);
>   torture_suite_add_1smb2_test(suite, "copy-chunk streams", test_copy_chunk_streams);
>   torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion", test_adouble_conversion);
> + torture_suite_add_1smb2_test(suite, "Timemachine-volsize", test_timemachine_volsize);
>  
>   return suite;
>  }
> --
> 2.13.6
>


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add a somewhat hackish option to limit the size of TimeMachine backups

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wed, Jan 03, 2018 at 11:04:30AM +0100, Ralph Böhme wrote:
>
> The good thing: it has a test! :)
>
> Please review & push if happy. Thanks!

Looks pretty good.

There are just a couple of comments inline. Can you take a look
and get back to me ?

Jeremy.

> --
> Ralph Boehme, Samba Team       https://samba.org/
> Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

> From 812a036e50c82cd021d3e5543aaf5acb407098c0 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <[hidden email]>
> Date: Fri, 3 Nov 2017 10:56:29 +0100
> Subject: [PATCH 1/2] vfs_fruit: add "time machine max size" option
>
> This can be used to configure a per client filesystem size limit on
> TimeMachine shares.
>
> It's a nasty hack but it was reportedly working well in Netatalk where
> it's taken from.
>
> Signed-off-by: Ralph Boehme <[hidden email]>
> ---
>  docs-xml/manpages/vfs_fruit.8.xml |  18 ++
>  source3/modules/vfs_fruit.c       | 421 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 439 insertions(+)
>
> diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml
> index fcaf1737ce8..7f6a0e7c022 100644
> --- a/docs-xml/manpages/vfs_fruit.8.xml
> +++ b/docs-xml/manpages/vfs_fruit.8.xml
> @@ -242,6 +242,24 @@
>    </varlistentry>
>  
>    <varlistentry>
> +    <term>fruit:time machine max size = SIZE [K|M|G|T|P]</term>
> +    <listitem>
> +      <para>Useful for Time Machine: limits the reported disksize, thus
> +      preventing Time Machine from using the whole real disk space for
> +      backup. The option takes a number plus an optional unit.</para>
> +      <para><emphasis>IMPORTANT</emphasis>: This is an approximated
> +      calculation that only takes into account the contents of Time
> +      Machine sparsebundle images. Therefor you <emphasis>MUST
> +      NOT</emphasis> use this volume to store other content when using
> +      this option, because it would NOT be accounted.</para>
> +      <para>The calculation works by reading the band size from the
> +      Info.plist XML file of the sparsebundle, reading the bands/
> +      directory counting the number of band files, and then multiplying
> +      one with the other.</para>
> +    </listitem>
> +  </varlistentry>
> +
> +  <varlistentry>
>      <term>fruit:metadata = [ stream | netatalk ]</term>
>      <listitem>
>        <para>Controls where the OS X metadata stream is stored:</para>
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 67dd571c627..e7b4e220a54 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -141,6 +141,7 @@ struct fruit_config_data {
>   bool aapl_zero_file_id;
>   const char *model;
>   bool time_machine;
> + size_t time_machine_max_size;
>  
>   /*
>   * Additional options, all enabled by default,
> @@ -1885,6 +1886,7 @@ static int init_fruit_config(vfs_handle_struct *handle)
>  {
>   struct fruit_config_data *config;
>   int enumval;
> + const char *tm_size_str = NULL;
>  
>   config = talloc_zero(handle->conn, struct fruit_config_data);
>   if (!config) {
> @@ -1983,6 +1985,14 @@ static int init_fruit_config(vfs_handle_struct *handle)
>   config->model = lp_parm_const_string(
>   -1, FRUIT_PARAM_TYPE_NAME, "model", "MacSamba");
>  
> + tm_size_str = lp_parm_const_string(
> + SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME,
> + "time machine max size", NULL);
> + if (tm_size_str != NULL) {
> + config->time_machine_max_size =
> + (size_t)conv_str_size(tm_size_str);
> + }
> +
>   SMB_VFS_HANDLE_SET_DATA(handle, config,
>   NULL, struct fruit_config_data,
>   return -1);
> @@ -6003,8 +6013,419 @@ static NTSTATUS fruit_offload_write_recv(struct vfs_handle_struct *handle,
>   return NT_STATUS_OK;
>  }
>  
> +static char *fruit_get_bandsize_line(char **lines, int numlines)
> +{
> + static regex_t re;
> + static bool re_initialized = false;
> + int i;
> + int ret;
> +
> + if (!re_initialized) {
> + ret = regcomp(&re, "^[[:blank:]]*<key>band-size</key>$", 0);
> + if (ret != 0) {
> + return NULL;
> + }
> + re_initialized = true;
> + }
> +
> + for (i = 0; i < numlines; i++) {
> + regmatch_t matches[1];
> +
> + ret = regexec(&re, lines[i], 1, matches, 0);
> + if (ret == 0) {
> + /*
> + * Check if the match was on the last line, sa we want
> + * the subsequent line.
> + */
> + if (i + 1 == numlines) {
> + return NULL;
> + }
> + return lines[i + 1];
> + }
> + if (ret != REG_NOMATCH) {
> + return NULL;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static bool fruit_get_bandsize_from_line(char *line, size_t *band_size)
> +{
> + static regex_t re;
> + static bool re_initialized = false;
> + regmatch_t matches[2];
> + uint64_t uint64;

                 ^^^^^^
                 'uint64' is a *horrible* confusing name
for a variable. It looks like a type definition.
Please change it to something meaningful.

> + int ret;
> + bool ok;
> +
> + if (!re_initialized) {
> + ret = regcomp(&re,
> +      "^[[:blank:]]*"
> +      "<integer>\\([[:digit:]]*\\)</integer>$",
> +      0);
> + if (ret != 0) {
> + return false;
> + }
> + re_initialized = true;
> + }
> +
> + ret = regexec(&re, line, 2, matches, 0);
> + if (ret != 0) {
> + DBG_ERR("regex failed [%s]\n", line);
> + return false;
> + }
> +
> + line[matches[1].rm_eo] = '\0';
> +
> + ok = conv_str_u64(&line[matches[1].rm_so], &uint64);
> + if (!ok) {
> + return false;
> + }
> + *band_size = (size_t)uint64;
> + return true;
> +}
> +
> +/*
> + * This reads and parses an Info.plist from a TM sparsebundle looking for the
> + * "band-size" key and value.
> + */
> +static bool fruit_get_bandsize(vfs_handle_struct *handle,
> +       const char *dir,
> +       size_t *band_size)
> +{
> +#define INFO_PLIST_MAX_SIZE 64*1024
> + char *plist = NULL;
> + struct smb_filename *smb_fname = NULL;
> + files_struct *fsp = NULL;
> + uint8_t *file_data = NULL;
> + char **lines = NULL;
> + char *band_size_line = NULL;
> + size_t plist_file_size;
> + ssize_t nread;
> + int numlines;
> + int ret;
> + bool ok = false;
> + NTSTATUS status;
> +
> + plist = talloc_asprintf(talloc_tos(),
> + "%s/%s/Info.plist",
> + handle->conn->connectpath,
> + dir);
> + if (plist == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + smb_fname = synthetic_smb_fname(talloc_tos(), plist, NULL, NULL, 0);
> + if (smb_fname == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + ret = SMB_VFS_NEXT_LSTAT(handle, smb_fname);
> + if (ret != 0) {
> + DBG_INFO("Ignoring Sparsebundle without Info.plist [%s]\n", dir);
> + ok = true;
> + goto out;
> + }
> +
> + plist_file_size = smb_fname->st.st_ex_size;
> +
> + if (plist_file_size > INFO_PLIST_MAX_SIZE) {
> + DBG_INFO("%s is too large, ignoring\n", plist);
> + ok = true;
> + goto out;
> + }
> +
> + status = SMB_VFS_NEXT_CREATE_FILE(
> + handle, /* conn */
> + NULL, /* req */
> + 0, /* root_dir_fid */
> + smb_fname, /* fname */
> + FILE_GENERIC_READ, /* access_mask */
> + FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
> + FILE_OPEN, /* create_disposition */
> + 0, /* create_options */
> + 0, /* file_attributes */
> + INTERNAL_OPEN_ONLY, /* oplock_request */
> + NULL, /* lease */
> + 0, /* allocation_size */
> + 0, /* private_flags */
> + NULL, /* sd */
> + NULL, /* ea_list */
> + &fsp, /* result */
> + NULL, /* psbuf */
> + NULL, NULL); /* create context */
> + if (!NT_STATUS_IS_OK(status)) {
> + DBG_INFO("Opening [%s] failed [%s]\n",
> + smb_fname_str_dbg(smb_fname), nt_errstr(status));
> + ok = false;
> + goto out;
> + }
> +
> + file_data = talloc_array(talloc_tos(), uint8_t, plist_file_size);
> + if (file_data == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + nread = SMB_VFS_NEXT_PREAD(handle, fsp, file_data, plist_file_size, 0);
> + if (nread != plist_file_size) {
> + DBG_ERR("Short read on [%s]: %zu/%zd\n",
> + fsp_str_dbg(fsp), nread, plist_file_size);
> + ok = false;
> + goto out;
> +
> + }
> +
> + status = close_file(NULL, fsp, NORMAL_CLOSE);
> + fsp = NULL;
> + if (!NT_STATUS_IS_OK(status)) {
> + DBG_ERR("close_file failed: %s\n", nt_errstr(status));
> + ok = false;
> + goto out;
> + }
> +
> + lines = file_lines_parse((char *)file_data,
> + plist_file_size,
> + &numlines,
> + talloc_tos());
> + if (lines == NULL) {
> + ok = false;
> + goto out;
> + }
> +
> + band_size_line = fruit_get_bandsize_line(lines, numlines);
> + if (band_size_line == NULL) {
> + DBG_ERR("Didn't find band-size key in [%s]\n",
> + smb_fname_str_dbg(smb_fname));
> + ok = false;
> + goto out;
> + }
> +
> + ok = fruit_get_bandsize_from_line(band_size_line, band_size);
> + if (!ok) {
> + DBG_ERR("fruit_get_bandsize_from_line failed\n");
> + goto out;
> + }
> +
> + DBG_DEBUG("Parsed band-size [%zu] for [%s]\n", *band_size, plist);
> +
> +out:
> + if (fsp != NULL) {
> + status = close_file(NULL, fsp, NORMAL_CLOSE);
> + if (!NT_STATUS_IS_OK(status)) {
> + DBG_ERR("close_file failed: %s\n", nt_errstr(status));
> + }
> + fsp = NULL;
> + }
> + TALLOC_FREE(plist);
> + TALLOC_FREE(smb_fname);
> + TALLOC_FREE(file_data);
> + TALLOC_FREE(lines);
> + return ok;
> +}
> +
> +struct fruit_disk_free_state {
> + size_t total_size;
> +};
> +
> +static bool fruit_get_num_bands(vfs_handle_struct *handle,
> + char *bundle,
> + size_t *_nbands)
> +{
> + char *path = NULL;
> + struct smb_filename *bands_dir = NULL;
> + DIR *d = NULL;
> + struct dirent *e = NULL;
> + size_t nbands;
> + int ret;
> +
> + path = talloc_asprintf(talloc_tos(),
> +       "%s/%s/bands",
> +       handle->conn->connectpath,
> +       bundle);
> + if (path == NULL) {
> + return false;
> + }
> +
> + bands_dir = synthetic_smb_fname(talloc_tos(),
> + path,
> + NULL,
> + NULL,
> + 0);
> + TALLOC_FREE(path);
> + if (bands_dir == NULL) {
> + return false;
> + }
> +
> +
> + d = SMB_VFS_NEXT_OPENDIR(handle, bands_dir, NULL, 0);
> + if (d == NULL) {
> + TALLOC_FREE(bands_dir);
> + return false;
> + }
> +
> + nbands = 0;
> +
> + for (e = SMB_VFS_NEXT_READDIR(handle, d, NULL);
> +     e != NULL;
> +     e = SMB_VFS_NEXT_READDIR(handle, d, NULL))
> + {
> + if (ISDOT(e->d_name) || ISDOTDOT(e->d_name)) {
> + continue;
> + }
> + nbands++;
> + }
> +
> + ret = SMB_VFS_NEXT_CLOSEDIR(handle, d);
> + if (ret != 0) {
> + TALLOC_FREE(bands_dir);
> + return false;
> + }
> +
> + DBG_DEBUG("%zu bands in [%s]\n", nbands, smb_fname_str_dbg(bands_dir));
> +
> + TALLOC_FREE(bands_dir);
> +
> + *_nbands = nbands;
> + return true;
> +}
> +
> +static bool fruit_tmsize_do_dirent(vfs_handle_struct *handle,
> +   struct fruit_disk_free_state *state,
> +   struct dirent *e)
> +{
> + bool ok;
> + char *p = NULL;
> + size_t sparsebundle_strlen = strlen("sparsebundle");
> + size_t bandsize;
> + size_t nbands;
> + double tm_size;
> +
> + p = strstr(e->d_name, "sparsebundle");
> + if (p == NULL) {
> + return true;
> + }
> +
> + if (p[sparsebundle_strlen] != '\0') {
> + return true;
> + }
> +
> + DBG_DEBUG("Processing sparsebundle [%s]\n", e->d_name);
> +
> + ok = fruit_get_bandsize(handle, e->d_name, &bandsize);
> + if (!ok) {
> + /*
> + * Beware of race conditions: this may be an uninitialued
> + * Info.plist that a client is just creating. We don't want let
> + * this to trigger complete failure.
> + */
> + DBG_ERR("Processing sparsebundle [%s] failed\n", e->d_name);
> + return true;
> + }
> +
> + ok = fruit_get_num_bands(handle, e->d_name, &nbands);
> + if (!ok) {
> + /*
> + * Beware of race conditions: this may be an uninitialued
> + * Info.plist that a client is just creating. We don't want let
> + * this to trigger complete failure.
> + */
> + DBG_ERR("Processing sparsebundle [%s] failed\n", e->d_name);
> + return true;
> + }
> +
> + tm_size = bandsize * nbands;
> + if (tm_size > UINT64_MAX) {
> + DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
> + bandsize, nbands);
> + return false;
> + }
> +
> + if (state->total_size + tm_size < state->total_size) {
> + DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
> + bandsize, nbands);
> + return false;
> + }
> +
> + state->total_size += tm_size;
> +
> + DBG_DEBUG("[%s] tm_size [%.0f] total_size [%zu]\n",
> +  e->d_name, tm_size, state->total_size);
> +
> + return true;
> +}
> +
> +/**
> + * Calculate used size of a TimeMachine volume
> + *
> + * This assumes that the volume is used only for TimeMachine.
> + *
> + * - readdir(basedir of share), then
> + * - for every element that matches regex "^\(.*\)\.sparsebundle$" :
> + * - parse "\1.sparsebundle/Info.plist" and read the band-size XML key
> + * - count band files in "\1.sparsebundle/bands/"
> + * - calculate used size of all bands: band_count * band_size
> + **/
> +static uint64_t fruit_disk_free(vfs_handle_struct *handle,
> + const struct smb_filename *smb_fname,
> + uint64_t *bsize,
> + uint64_t *dfree,
> + uint64_t *dsize)
> +{
> + struct fruit_config_data *config = NULL;
> + struct fruit_disk_free_state state = {0};
> + DIR *d = NULL;
> + struct dirent *e = NULL;
> + int ret;
> + bool ok;
> +
> + SMB_VFS_HANDLE_GET_DATA(handle, config,
> + struct fruit_config_data,
> + return UINT64_MAX);
> +
> + if (!config->time_machine ||
> +    config->time_machine_max_size == 0)
> + {
> + return SMB_VFS_NEXT_DISK_FREE(handle,
> +      smb_fname,
> +      bsize,
> +      dfree,
> +      dsize);
> + }
> +
> + d = SMB_VFS_NEXT_OPENDIR(handle, smb_fname, NULL, 0);
> + if (d == NULL) {
> + return UINT64_MAX;
> + }
> +
> + for (e = SMB_VFS_NEXT_READDIR(handle, d, NULL);
> +     e != NULL;
> +     e = SMB_VFS_NEXT_READDIR(handle, d, NULL))
> + {
> + ok = fruit_tmsize_do_dirent(handle, &state, e);
> + if (!ok) {
> + SMB_VFS_NEXT_CLOSEDIR(handle, d);
> + return UINT64_MAX;
> + }
> + }
> +
> + ret = SMB_VFS_NEXT_CLOSEDIR(handle, d);
> + if (ret != 0) {
> + return UINT64_MAX;
> + }
> +
> + *bsize = 512;
> + *dsize = config->time_machine_max_size / 512;
> + *dfree = *dsize - (state.total_size / 512);
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                  Can this go negative ? What would
                  happen if it did ?

> + return *dfree / 2;
> +}
> +
>  static struct vfs_fn_pointers vfs_fruit_fns = {
>   .connect_fn = fruit_connect,
> + .disk_free_fn = fruit_disk_free,
>  
>   /* File operations */
>   .chmod_fn = fruit_chmod,
> --
> 2.13.6
>
>
> From 090b7d80362be4fa1382d98b120dd83aeac81ab7 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <[hidden email]>
> Date: Tue, 2 Jan 2018 19:09:04 +0100
> Subject: [PATCH 2/2] s4/torture: test vfs_fruit "fruit:time machine max size"
>  option
>
> Signed-off-by: Ralph Boehme <[hidden email]>
> ---
>  selftest/target/Samba3.pm   |  1 +
>  source4/torture/vfs/fruit.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
>
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index c9888c2dca3..c483dfb1144 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -1934,6 +1934,7 @@ sub provision($$$$$$$$$)
>   fruit:locking = netatalk
>   fruit:encoding = native
>   fruit:veto_appledouble = no
> + fruit:time machine max size = 10G
>  
>  [vfs_fruit_metadata_stream]
>   path = $shrdir
> diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
> index 04f04e2cd56..0975b86b585 100644
> --- a/source4/torture/vfs/fruit.c
> +++ b/source4/torture/vfs/fruit.c
> @@ -4344,6 +4344,94 @@ done:
>   return ok;
>  }
>  
> +static bool test_timemachine_volsize(struct torture_context *tctx,
> +     struct smb2_tree *tree)
> +{
> + TALLOC_CTX *mem_ctx = talloc_new(tctx);
> + struct smb2_handle h = {{0}};
> + union smb_fsinfo fsinfo;
> + NTSTATUS status;
> + bool ok = true;
> + const char *info_plist =
> + "<dict>\n"
> + "        <key>band-size</key>\n"
> + "        <integer>8192</integer>\n"
> + "</dict>\n";
> +
> + smb2_deltree(tree, "test.sparsebundle");
> +
> + ok = enable_aapl(tctx, tree);
> + torture_assert_goto(tctx, ok, ok, done, "enable_aapl failed");
> +
> + status = smb2_util_mkdir(tree, "test.sparsebundle");
> + torture_assert_ntstatus_ok_goto(tctx, status, ok, done,
> + "smb2_util_mkdir\n");
> +
> + ok = write_stream(tree, __location__, tctx, mem_ctx,
> +  "test.sparsebundle/Info.plist", NULL,
> +   0, strlen(info_plist), info_plist);
> + torture_assert_goto(tctx, ok, ok, done, "write_stream failed\n");
> +
> + status = smb2_util_mkdir(tree, "test.sparsebundle/bands");
> + torture_assert_ntstatus_ok_goto(tctx, status, ok, done,
> + "smb2_util_mkdir\n");
> +
> + ok = torture_setup_file(tctx, tree, "test.sparsebundle/bands/1", false);
> + torture_assert_goto(tctx, ok, ok, done, "torture_setup_file failed\n");
> +
> + ok = torture_setup_file(tctx, tree, "test.sparsebundle/bands/2", false);
> + torture_assert_goto(tctx, ok, ok, done, "torture_setup_file failed\n");
> +
> + status = smb2_util_roothandle(tree, &h);
> + torture_assert_ntstatus_ok(tctx, status, "Unable to create root handle");
> +
> + ZERO_STRUCT(fsinfo);
> + fsinfo.generic.level = RAW_QFS_SIZE_INFORMATION;
> + fsinfo.generic.handle = h;
> +
> + status = smb2_getinfo_fs(tree, tree, &fsinfo);
> + torture_assert_ntstatus_ok(tctx, status, "smb2_getinfo_fs failed");
> +
> + torture_comment(tctx, "sectors_per_unit: %" PRIu32"\n"
> + "bytes_per_sector: %" PRIu32"\n"
> + "total_alloc_units: %" PRIu64"\n"
> + "avail_alloc_units: %" PRIu64"\n",
> + fsinfo.size_info.out.sectors_per_unit,
> + fsinfo.size_info.out.bytes_per_sector,
> + fsinfo.size_info.out.total_alloc_units,
> + fsinfo.size_info.out.avail_alloc_units);
> +
> + /*
> + * Let me explain the numbers:
> + *
> + * - the share is set to "fruit:time machine max size = 20G"
> + * - we've faked a bandsize of 8 K in the Info.plist file
> + * - we've created two bands files
> + * - one allocation unit is made of two sectors with 512 B each
> + * => we've consumed 16 allocation units
> + */
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.sectors_per_unit == 2,
> +    ok, done, "Bad total_alloc_units");
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.bytes_per_sector == 512,
> +    ok, done, "Bad total_alloc_units");
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.total_alloc_units == 20971520,
> +    ok, done, "Bad total_alloc_units");
> +
> + torture_assert_goto(tctx, fsinfo.size_info.out.avail_alloc_units == 20971504,
> +    ok, done, "Bad total_alloc_units");
> +
> +done:
> + if (!smb2_util_handle_empty(h)) {
> + smb2_util_close(tree, h);
> + }
> + smb2_deltree(tree, "test.sparsebundle");
> + talloc_free(mem_ctx);
> + return ok;
> +}
> +
>  /*
>   * Note: This test depends on "vfs objects = catia fruit streams_xattr".  For
>   * some tests torture must be run on the host it tests and takes an additional
> @@ -4384,6 +4472,7 @@ struct torture_suite *torture_vfs_fruit(TALLOC_CTX *ctx)
>   torture_suite_add_1smb2_test(suite, "creating rsrc with read-only access", test_rfork_create_ro);
>   torture_suite_add_1smb2_test(suite, "copy-chunk streams", test_copy_chunk_streams);
>   torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion", test_adouble_conversion);
> + torture_suite_add_1smb2_test(suite, "Timemachine-volsize", test_timemachine_volsize);
>  
>   return suite;
>  }
> --
> 2.13.6
>


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add a somewhat hackish option to limit the size of TimeMachine backups

Samba - samba-technical mailing list
On Thu, Jan 04, 2018 at 05:08:29PM -0800, Jeremy Allison wrote:

> On Wed, Jan 03, 2018 at 11:04:30AM +0100, Ralph Böhme wrote:
> >
> > The good thing: it has a test! :)
> >
> > Please review & push if happy. Thanks!
>
> Looks pretty good.
>
> There are just a couple of comments inline. Can you take a look
> and get back to me ?
>
> > + uint64_t uint64;
>
>                  ^^^^^^
>                  'uint64' is a *horrible* confusing name
> for a variable. It looks like a type definition.
> Please change it to something meaningful.
Fixed.

> > + *dfree = *dsize - (state.total_size / 512);
>                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  Can this go negative ? What would
>  happen if it did ?

You bet it can! Thanks for catching this. Fixed.

I also noticed that the test failed in make test. I didn't notice this as I kept
running it directly from smbtorture against a local server and share where it
worked. While at it, I moved the test to it's own test-suite so I can run it
against a dedicated new share that has the necessary options set up.

Thanks!
-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

fruit-tm-volsize.patch (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add a somewhat hackish option to limit the size of TimeMachine backups

Samba - samba-technical mailing list
On Fri, Jan 05, 2018 at 11:57:42AM +0100, Ralph Böhme via samba-technical wrote:

> On Thu, Jan 04, 2018 at 05:08:29PM -0800, Jeremy Allison wrote:
> > On Wed, Jan 03, 2018 at 11:04:30AM +0100, Ralph Böhme wrote:
> > >
> > > The good thing: it has a test! :)
> > >
> > > Please review & push if happy. Thanks!
> >
> > Looks pretty good.
> >
> > There are just a couple of comments inline. Can you take a look
> > and get back to me ?
> >
> > > + uint64_t uint64;
> >
> >                  ^^^^^^
> >                  'uint64' is a *horrible* confusing name
> > for a variable. It looks like a type definition.
> > Please change it to something meaningful.
>
> Fixed.
>
> > > + *dfree = *dsize - (state.total_size / 512);
> >                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >  Can this go negative ? What would
> >  happen if it did ?
>
> You bet it can! Thanks for catching this. Fixed.
>
> I also noticed that the test failed in make test. I didn't notice this as I kept
> running it directly from smbtorture against a local server and share where it
> worked. While at it, I moved the test to it's own test-suite so I can run it
> against a dedicated new share that has the necessary options set up.
>
> Thanks!

LGTM. Thanks for the fixes ! RB+ and pushed.

Jeremy.