[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 2/6] btrfs: add optional parameter `ro' to btrfs_subvolume_snapshot



On Friday 21 November 2014 13:17:56 Hu Tao wrote:
> Parameter `ro' is for creating readonly btrfs snapshot.
> 
> Signed-off-by: Hu Tao <hutao cn fujitsu com>
> ---
>  daemon/btrfs.c       | 11 ++++++++++-
>  generator/actions.ml |  4 ++--
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 7a4d43d..b630bce 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -208,7 +208,7 @@ do_mkfs_btrfs (char *const *devices,
>  }
> 
>  int
> -do_btrfs_subvolume_snapshot (const char *source, const char *dest)
> +do_btrfs_subvolume_snapshot (const char *source, const char *dest,
> int ro) {
>    const size_t MAX_ARGS = 64;
>    const char *argv[MAX_ARGS];
> @@ -231,6 +231,15 @@ do_btrfs_subvolume_snapshot (const char *source,
> const char *dest) ADD_ARG (argv, i, str_btrfs);
>    ADD_ARG (argv, i, "subvolume");
>    ADD_ARG (argv, i, "snapshot");
> +
> +  /* Optional arguments. */
> +  if (!(optargs_bitmask &
> GUESTFS_BTRFS_SUBVOLUME_SNAPSHOT_RO_BITMASK)) +    ro = 0;
> +
> +  if (ro) {
> +    ADD_ARG (argv, i, "-r");
> +  }
> +

I would make this bit simplier:

 if ((optargs_bitmask & GUESTFS_BTRFS_SUBVOLUME_SNAPSHOT_RO_BITMASK)
     && ro)
   ADD_ARG (argv, i, "-r");

Also, seen in the other patches as well, you don't need { ... } brackets 
for blocks of just one instructions.


> diff --git a/generator/actions.ml b/generator/actions.ml
> index fe492e6..850e58d 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -10217,7 +10217,7 @@ See C<guestfs_get_e2generation>." };
> 
>    { defaults with
>      name = "btrfs_subvolume_snapshot";
> -    style = RErr, [Pathname "source"; Pathname "dest"], [];
> +    style = RErr, [Pathname "source"; Pathname "dest"], [OBool "ro"];
> proc_nr = Some 322;
>      optional = Some "btrfs"; camel_name = "BTRFSSubvolumeSnapshot";
>      tests = [

You must set once_had_no_optargs = true for this action, as it currently 
has no optional arguments; see generator/README.

-- 
Pino Toscano


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]