Re: [Libguestfs] [PATCH 2/3] New API: guestfs_blockdev_setra: Adjust readahead for filesystems and devices.

Pino Toscano
2014-11-24T18:20:23Z

On Sunday 23 November 2014 22:16:40 Richard W.M. Jones
wrote:
> This adds a binding for 'blockdev --setra', allowing you to adjust
the
> readahead parameter for filesystems and devices.
> ---
>  daemon/blockdev.c    | 30 ++++++++++++++++++++----------
>  generator/actions.ml | 14 ++++++++++++++
>  2 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/daemon/blockdev.c b/daemon/blockdev.c
> index 8a7b1a8..6e8821d 100644
> --- a/daemon/blockdev.c
> +++ b/daemon/blockdev.c
> @@ -62,7 +62,7 @@ call_blockdev (const char *device, const char
> *switc, int extraarg, int prints) */
>    udev_settle ();
>
> -  if (extraarg > 0) {
> +  if (extraarg >= 0) {
>      snprintf (buf, sizeof buf, "%d", extraarg);
>      argv[2] = buf;
>      argv[3] = device;
> @@ -89,33 +89,43 @@ call_blockdev (const char *device, const char
> *switc, int extraarg, int prints) }
>
>  int
> +do_blockdev_setra (const char *device, int sectors)
> +{
> +  if (sectors < 0) {
> +    reply_with_error ("readahead sectors must be >= 0");
> +    return -1;
> +  }
> +  return (int) call_blockdev (device, "--setra", sectors, 0);
> +}
> +
> +int
>  do_blockdev_setro (const char *device)
>  {
> -  return (int) call_blockdev (device, "--setro", 0, 0);
> +  return (int) call_blockdev (device, "--setro", -1, 0);
>  }
>
>  int
>  do_blockdev_setrw (const char *device)
>  {
> -  return (int) call_blockdev (device, "--setrw", 0, 0);
> +  return (int) call_blockdev (device, "--setrw", -1, 0);
>  }
>
>  int
>  do_blockdev_getro (const char *device)
>  {
> -  return (int) call_blockdev (device, "--getro", 0, 1);
> +  return (int) call_blockdev (device, "--getro", -1, 1);
>  }
>
>  int
>  do_blockdev_getss (const char *device)
>  {
> -  return (int) call_blockdev (device, "--getss", 0, 1);
> +  return (int) call_blockdev (device, "--getss", -1, 1);
>  }
>
>  int
>  do_blockdev_getbsz (const char *device)
>  {
> -  return (int) call_blockdev (device, "--getbsz", 0, 1);
> +  return (int) call_blockdev (device, "--getbsz", -1, 1);
>  }
>
>  int
> @@ -128,23 +138,23 @@ do_blockdev_setbsz (const char *device, int
> blocksize) int64_t
>  do_blockdev_getsz (const char *device)
>  {
> -  return call_blockdev (device, "--getsz", 0, 1);
> +  return call_blockdev (device, "--getsz", -1, 1);
>  }
>
>  int64_t
>  do_blockdev_getsize64 (const char *device)
>  {
> -  return call_blockdev (device, "--getsize64", 0, 1);
> +  return call_blockdev (device, "--getsize64", -1, 1);
>  }
>
>  int
>  do_blockdev_flushbufs (const char *device)
>  {
> -  return call_blockdev (device, "--flushbufs", 0, 0);
> +  return call_blockdev (device, "--flushbufs", -1, 0);
>  }
>
>  int
>  do_blockdev_rereadpt (const char *device)
>  {
> -  return call_blockdev (device, "--rereadpt", 0, 0);
> +  return call_blockdev (device, "--rereadpt", -1, 0);
>  }
> diff --git a/generator/actions.ml b/generator/actions.ml
> index fe492e6..baa7679 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -12003,6 +12003,20 @@ This is the same as the C<lstat(2)> system
> call." }; longdesc = "\
>  This is the internal call which implements
C<guestfs_lstatnslist>."
> };
>
> +  { defaults with
> +    name = "blockdev_setra";
> +    style = RErr, [Device "device"; Int "sectors"],
[];
> +    proc_nr = Some 424;
> +    tests = [
> +      InitEmpty, Always, TestRun (
> +        [["blockdev_setra"; "/dev/sda";
"1024" ]]), []
> +    ];
> +    shortdesc = "set readahead";
> +    longdesc = "\
> +Set readahead (in 512-byte sectors) for the device.
> +
> +This uses the L<blockdev(8)> command." };
> +
>  ]
>
>  (* Non-API meta-commands available only in guestfish.

Just a couple of notes:
- I would have done the small internal behaviour change of the
  extraarg parameter of call_blockdev in an own commit, so it isn't
  tangled with the API addition
- should there be some get counterpart, like blockdev_getra?
- don't forget to commit MAX_PROC_NR together with this

Otherwise, LGTM.

--
Pino Toscano