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

Re: [Libguestfs] [PATCH V4] NEW API:xfs:xfs_repair



On Thu, Aug 30, 2012 at 09:28:34AM +0800, Wanlong Gao wrote:
> Add a new api xfs_repair for repairing an XFS filesystem.
> 
> Signed-off-by: Wanlong Gao <gaowanlong cn fujitsu com>
> ---
>  daemon/xfs.c                   | 128 +++++++++++++++++++++++++++++++++++++++++
>  generator/generator_actions.ml |  25 ++++++++
>  gobject/Makefile.inc           |   6 +-
>  guestfs-release-notes.txt      |   5 ++
>  po/POTFILES                    |   1 +
>  src/MAX_PROC_NR                |   2 +-
>  6 files changed, 164 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/xfs.c b/daemon/xfs.c
> index f3058d3..a916646 100644
> --- a/daemon/xfs.c
> +++ b/daemon/xfs.c
> @@ -528,3 +528,131 @@ error:
>    free (err);
>    return -1;
>  }
> +
> +int
> +do_xfs_repair (const char *device,
> +               int forcelogzero, int dangerous,
> +               int nomodify, int noprefetch, int forcegeometry,
> +               int64_t maxmem, int64_t ihashsize,
> +               int64_t bhashsize, int64_t agstride,
> +               const char *logdev, const char *rtdev)
> +{
> +  int r;
> +  char *err = NULL;
> +  const char *argv[MAX_ARGS];
> +  char *buf = NULL;
> +  char maxmem_s[64];
> +  char ihashsize_s[70];
> +  char bhashsize_s[70];
> +  char agstride_s[74];
> +  size_t i = 0;
> +  int is_device;
> +
> +  ADD_ARG (argv, i, "xfs_repair");
> +
> +  /* Optional arguments */
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_FORCELOGZERO_BITMASK) {
> +    if (forcelogzero)
> +      ADD_ARG (argv, i, "-L");
> +  }
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_DANGEROUS_BITMASK) {
> +    if (dangerous)
> +      ADD_ARG (argv, i, "-d");
> +  }

Sorry I didn't spot this before, but is there any need to
bind this option?  The man page describes it as:

  Allow xfs_repair to repair an XFS filesystem
  mounted  read  only.

but that seems unnecessary for libguestfs.

> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_NOMODIFY_BITMASK) {
> +    if (nomodify)
> +      ADD_ARG (argv, i, "-n");
> +  }
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_NOPREFETCH_BITMASK) {
> +    if (noprefetch)
> +      ADD_ARG (argv, i, "-P");
> +  }
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_FORCEGEOMETRY_BITMASK) {
> +    if (forcegeometry) {
> +      ADD_ARG (argv, i, "-o");
> +      ADD_ARG (argv, i, "force_geometry");
> +    }
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_MAXMEM_BITMASK) {
> +    if (maxmem < 0) {
> +      reply_with_error ("maxmem must be >= 0");
> +      goto error;
> +    }
> +    snprintf(maxmem_s, sizeof maxmem_s, "%" PRIi64, maxmem);
> +    ADD_ARG (argv, i, "-m");
> +    ADD_ARG (argv, i, maxmem_s);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_IHASHSIZE_BITMASK) {
> +    if (ihashsize < 0) {
> +      reply_with_error ("ihashsize must be >= 0");
> +      goto error;
> +    }
> +    snprintf(ihashsize_s, sizeof ihashsize_s, "ihash=" "%" PRIi64, ihashsize);
> +    ADD_ARG (argv, i, "-o");
> +    ADD_ARG (argv, i, ihashsize_s);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_BHASHSIZE_BITMASK) {
> +    if (bhashsize < 0) {
> +      reply_with_error ("bhashsize must be >= 0");
> +      goto error;
> +    }
> +    snprintf(bhashsize_s, sizeof bhashsize_s, "bhash=" "%" PRIi64, bhashsize);
> +    ADD_ARG (argv, i, "-o");
> +    ADD_ARG (argv, i, bhashsize_s);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_AGSTRIDE_BITMASK) {
> +    if (agstride < 0) {
> +      reply_with_error ("agstride must be >= 0");
> +      goto error;
> +    }
> +    snprintf(agstride_s, sizeof agstride_s, "ag_stride=" "%" PRIi64, agstride);
> +    ADD_ARG (argv, i, "-o");
> +    ADD_ARG (argv, i, agstride_s);
> +  }
> +
> +
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_LOGDEV_BITMASK) {
> +    ADD_ARG (argv, i, "-l");
> +    ADD_ARG (argv, i, logdev);
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_XFS_REPAIR_RTDEV_BITMASK) {
> +    ADD_ARG (argv, i, "-r");
> +    ADD_ARG (argv, i, rtdev);
> +  }
> +
> +  is_device = STREQLEN (device, "/dev/", 5);
> +  if (!is_device) {
> +    buf = sysroot_path (device);
> +    if (buf == NULL) {
> +      reply_with_perror ("malloc");
> +      goto error;
> +    }
> +    ADD_ARG (argv, i, "-f");
> +    ADD_ARG (argv, i, buf);
> +  } else {
> +    ADD_ARG (argv, i, device);
> +  }
> +
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandrv (NULL, &err, argv);
> +  if (buf) free (buf);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", device, err);
> +    goto error;
> +  } else if (r == 1) {
> +    reply_with_error ("%s: filesystem corruption was detected, need repair", device);
> +  }

I think it's probably better to return 'r' (ie. the function
would become RInt instead of RErr).  In C, this would mean that
guestfs_xfs_repair would return 0, 1 or -1.

> +  free (err);
> +  return 0;
> +
> +error:
> +  if (err) free (err);
> +  return -1;
> +}
> diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
> index 6290333..f5eef60 100644
> --- a/generator/generator_actions.ml
> +++ b/generator/generator_actions.ml
> @@ -9682,6 +9682,31 @@ C<key> is the name, C<t> is the type, and C<val> is the data.
>  
>  This is a wrapper around the L<hivex(3)> call of the same name." };
>  
> +  { defaults with
> +    name = "xfs_repair";
> +    style = RErr, [Dev_or_Path "device"], [OBool "forcelogzero"; OBool "dangerous"; OBool "nomodify"; OBool "noprefetch"; OBool "forcegeometry"; OInt64 "maxmem"; OInt64 "ihashsize"; OInt64 "bhashsize"; OInt64 "agstride"; OString "logdev"; OString "rtdev"];
> +    proc_nr = Some 366;
> +    optional = Some "xfs";
> +    tests = [
> +      InitEmpty, IfAvailable "xfs", TestRun (
> +        [["part_disk"; "/dev/sda"; "mbr"];
> +         ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""];
> +         ["xfs_repair"; "/dev/sda1"; ""; ""; "true"; ""; ""; ""; ""; ""; ""; "NOARG"; "NOARG"]
> +      ])
> +    ];
> +    shortdesc = "repair an XFS filesystem";
> +    longdesc = "\
> +Repair corrupt or damaged XFS filesystem on C<device>.
> +
> +The filesystem is specified using the C<device> argument which should be
> +the device name of the disk partition or volume containing the filesystem.
> +If given the name of a block device, C<xfs_repair> will attempt to find
> +the raw device associated with the specified block device and will use
> +the raw device instead.
> +
> +Regardless, the filesystem to be repaired must be unmounted, otherwise,
> +the resulting filesystem may be inconsistent or corrupt." };
> +
>  ]
>  
>  (* Non-API meta-commands available only in guestfish.
> diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc
> index 26d1c3c..ba41b48 100644
> --- a/gobject/Makefile.inc
> +++ b/gobject/Makefile.inc
> @@ -77,7 +77,8 @@ guestfs_gobject_headers= \
>    include/guestfs-gobject/optargs-rsync_in.h \
>    include/guestfs-gobject/optargs-rsync_out.h \
>    include/guestfs-gobject/optargs-xfs_admin.h \
> -  include/guestfs-gobject/optargs-hivex_open.h
> +  include/guestfs-gobject/optargs-hivex_open.h \
> +  include/guestfs-gobject/optargs-xfs_repair.h
>  
>  guestfs_gobject_sources= \
>    src/session.c \
> @@ -136,4 +137,5 @@ guestfs_gobject_sources= \
>    src/optargs-rsync_in.c \
>    src/optargs-rsync_out.c \
>    src/optargs-xfs_admin.c \
> -  src/optargs-hivex_open.c
> +  src/optargs-hivex_open.c \
> +  src/optargs-xfs_repair.c
> diff --git a/guestfs-release-notes.txt b/guestfs-release-notes.txt
> index ed9de41..19b8a59 100644
> --- a/guestfs-release-notes.txt
> +++ b/guestfs-release-notes.txt
> @@ -114,6 +114,7 @@ RELEASE NOTES FOR LIBGUESTFS 1.20
>  
>      For further information, see
>      https://bugzilla.redhat.com/show_bug.cgi?id=788642
> +    <https://bugzilla.redhat.com/show_bug.cgi?id=788642>
>  
>   New APIs
>  
> @@ -1633,10 +1634,14 @@ BUGS
>      To get a list of bugs against libguestfs, use this link:
>      https://bugzilla.redhat.com/buglist.cgi?component=libguestfs&product=Vi
>      rtualization+Tools
> +    <https://bugzilla.redhat.com/buglist.cgi?component=libguestfs&product=V
> +    irtualization+Tools>
>  
>      To report a new bug against libguestfs, use this link:
>      https://bugzilla.redhat.com/enter_bug.cgi?component=libguestfs&product=
>      Virtualization+Tools
> +    <https://bugzilla.redhat.com/enter_bug.cgi?component=libguestfs&product
> +    =Virtualization+Tools>
>  
>      When reporting a bug, please supply:
>  
> diff --git a/po/POTFILES b/po/POTFILES
> index 13f20c1..19cb0b5 100644
> --- a/po/POTFILES
> +++ b/po/POTFILES
> @@ -172,6 +172,7 @@ gobject/src/optargs-umount.c
>  gobject/src/optargs-umount_local.c
>  gobject/src/optargs-xfs_admin.c
>  gobject/src/optargs-xfs_growfs.c
> +gobject/src/optargs-xfs_repair.c
>  gobject/src/session.c
>  gobject/src/struct-application.c
>  gobject/src/struct-btrfssubvolume.c
> diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
> index 4753102..4203007 100644
> --- a/src/MAX_PROC_NR
> +++ b/src/MAX_PROC_NR
> @@ -1 +1 @@
> -365
> +366
> -- 
> 1.7.12

Apart from those two small things, it all looks fine to me.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/


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