[Libguestfs] [PATCH] xfs: add new api xfs-growfs

[Richard W.M. Jones]

On Tue, Jul 31, 2012 at 04:49:57PM +0800, Wanlong Gao wrote:
> New api xfs_growfs for expanding a XFS filesystem.
> 
> Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> ---
>  daemon/xfs.c                   | 119 +++++++++++++++++++++++++++++++++++++++++
>  generator/generator_actions.ml |  15 ++++++
>  gobject/Makefile.inc           |   6 ++-
>  po/POTFILES                    |   1 +
>  src/MAX_PROC_NR                |   2 +-
>  5 files changed, 140 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/xfs.c b/daemon/xfs.c
> index e0f0062..97da3f1 100644
> --- a/daemon/xfs.c
> +++ b/daemon/xfs.c
> @@ -348,3 +348,122 @@ error:
>      free_strings (lines);
>    return ret;
>  }
> +
> +guestfs_int_xfsinfo *
> +do_xfs_growfs (const char *path,
> +               int datasec, int logsec, int rtsec,
> +               int64_t datasize, int64_t logsize, int64_t rtsize,
> +               int64_t rtextsize, int32_t maxpct)
> +{
> +
> +#define MAX_ARGS 64

It's probably better to define this near the top of the file, instead
of here, OR instead use:

  const size_t MAX_ARGS = 64;

in the function.  Compare with daemon/btrfs.c.

> +  int r;
> +  char *buf;
> +  char *out = NULL, *err = NULL;
> +  char **lines = NULL;
> +  guestfs_int_xfsinfo *ret = NULL;
> +  const char *argv[MAX_ARGS];
> +  char datasize_s[64];
> +  char logsize_s[64];
> +  char rtsize_s[64];
> +  char rtextsize_s[64];
> +  char maxpct_s[32];
> +  size_t i = 0;
> +
> +  buf = sysroot_path (path);
> +  if (buf == NULL) {
> +    reply_with_perror ("malloc");
> +    return NULL;
> +  }
> +
> +  ADD_ARG (argv, i, "xfs_growfs");
> +
> +  /* Optional arguments */
> +  if (!(optargs_bitmask & GUESTFS_XFS_GROWFS_DATASEC_BITMASK))
> +    datasec = 0;
> +  if (!(optargs_bitmask & GUESTFS_XFS_GROWFS_LOGSEC_BITMASK))
> +    logsec = 0;
> +  if (!(optargs_bitmask & GUESTFS_XFS_GROWFS_RTSEC_BITMASK))
> +    rtsec = 0;
> +
> +  if (datasec)
> +    ADD_ARG (argv, i, "-d");
> +  if (logsec)
> +    ADD_ARG (argv, i, "-l");
> +  if (rtsec)
> +    ADD_ARG (argv, i, "-r");
> +
> +  if (optargs_bitmask & GUESTFS_XFS_GROWFS_DATASIZE_BITMASK) {
> +    if (datasize < 0) {
> +      reply_with_error ("datasize must be >= 0");
> +      goto error;
> +    }
> +    snprintf (datasize_s, sizeof datasize_s, "%" PRIi64, datasize);
> +    ADD_ARG (argv, i, "-D");
> +    ADD_ARG (argv, i, datasize_s);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_GROWFS_LOGSIZE_BITMASK) {
> +    if (logsize < 0) {
> +      reply_with_error ("logsize must be >= 0");
> +      goto error;
> +    }
> +    snprintf(logsize_s, sizeof logsize_s, "%" PRIi64, logsize);
> +    ADD_ARG (argv, i, "-L");
> +    ADD_ARG (argv, i, logsize_s);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_GROWFS_RTSIZE_BITMASK) {
> +    if (rtsize < 0) {
> +      reply_with_error ("rtsize must be >= 0");
> +      goto error;
> +    }
> +    snprintf(rtsize_s, sizeof rtsize_s, "%" PRIi64, rtsize);
> +    ADD_ARG (argv, i, "-R");
> +    ADD_ARG (argv, i, rtsize_s);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_GROWFS_RTEXTSIZE_BITMASK) {
> +    if (rtextsize < 0) {
> +      reply_with_error ("rtextsize must be >= 0");
> +      goto error;
> +    }
> +    snprintf(rtextsize_s, sizeof rtextsize_s, "%" PRIi64, rtextsize);
> +    ADD_ARG (argv, i, "-e");
> +    ADD_ARG (argv, i, rtextsize_s);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_GROWFS_MAXPCT_BITMASK) {
> +    if (maxpct < 0) {
> +      reply_with_error ("maxpct must be >= 0");
> +      goto error;
> +    }
> +    snprintf(maxpct_s, sizeof maxpct_s, "%" PRIi32, maxpct);
> +    ADD_ARG (argv, i, "-m");
> +    ADD_ARG (argv, i, maxpct_s);
> +  }
> +
> +  ADD_ARG (argv, i, buf);
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (&out, &err, argv);
> +  free (buf);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", path, err);
> +    goto error;
> +  }
> +
> +  lines = split_lines (out);
> +  if (lines == NULL)
> +    goto error;
> +
> +  ret = parse_xfs_info (lines);
> +
> +error:
> +  if (buf) free (buf);
> +  if (err) free (err);
> +  if (out) free (out);
> +  if (lines) free_strings (lines);
> +  return ret;
> +}
> diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
> index e4956f7..4d650aa 100644
> --- a/generator/generator_actions.ml
> +++ b/generator/generator_actions.ml
> @@ -9051,6 +9051,21 @@ This returns the kernel version of the appliance, where this is
>  available.  This information is only useful for debugging.  Nothing
>  in the returned structure is defined by the API." };
>  
> +  { defaults with
> +    name = "xfs_growfs";
> +    style = RStruct ("info", "xfsinfo"), [Pathname "path"], [OBool "datasec"; OBool "logsec"; OBool "rtsec"; OInt64 "datasize"; OInt64 "logsize"; OInt64 "rtsize"; OInt64 "rtextsize"; OInt "maxpct"];
> +    proc_nr = Some 343;
> +    optional = Some "xfs";
> +    tests = [];

We should test this.  See the tests associate with xfs_info and
lvresize/resize2fs for a good starting point.

> +    shortdesc = "expand a existing XFS filesystem";
> +    longdesc = "\
> +The C<path> argument is the pathname of the directory where
> +the filesystem is mounted. The filesystem must be mounted to be grown.
> +
> +The returned struct contains geometry information.  Missing
> +fields are returned as C<-1> (for numeric fields) or empty
> +string." };
> +
>  ]

In general it all looks good, but I'd really want to see this feature
being tested in the automatic tests.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw