subject:"\[Libguestfs\] \[PATCH libnbd 5\/5\] copy\: Allow human sizes for \-\-queue\-size, \-\-request\-size, \-\-sparse"

Re: [Libguestfs] [PATCH libnbd 5/5] copy: Allow human sizes for --queue-size, --request-size, --sparse

2023-09-05 Thread Richard W.M. Jones

On Mon, Sep 04, 2023 at 10:13:45AM +0200, Laszlo Ersek wrote:
> On 9/3/23 17:23, Richard W.M. Jones wrote:
> > Allow these options to be specified using human sizes, for example
> > this now works:
> > 
> >   nbdcopy --request-size=32M ...
> > ---
> >  copy/copy-sparse-allocated.sh|  2 +-
> >  copy/copy-sparse-no-extents.sh   |  2 +-
> >  copy/copy-sparse-request-size.sh |  2 +-
> >  copy/main.c  | 37 
> >  copy/nbdcopy.h   |  2 +-
> >  5 files changed, 27 insertions(+), 18 deletions(-)
> > 
> > diff --git a/copy/copy-sparse-allocated.sh b/copy/copy-sparse-allocated.sh
> > index c65ddea79f..e1fe9cf463 100755
> > --- a/copy/copy-sparse-allocated.sh
> > +++ b/copy/copy-sparse-allocated.sh
> > @@ -31,7 +31,7 @@ requires nbdkit eval --version
> >  out=copy-sparse-allocated.out
> >  cleanup_fn rm -f $out
> >  
> > -$VG nbdcopy --allocated --request-size=32768 -- \
> > +$VG nbdcopy --allocated --request-size=32K -- \
> >  [ nbdkit --exit-with-parent data data='
> >   1
> >   @1073741823 1
> > diff --git a/copy/copy-sparse-no-extents.sh b/copy/copy-sparse-no-extents.sh
> > index cff356978b..9368c564e9 100755
> > --- a/copy/copy-sparse-no-extents.sh
> > +++ b/copy/copy-sparse-no-extents.sh
> > @@ -39,7 +39,7 @@ requires nbdkit eval --version
> >  out=copy-sparse-no-extents.out
> >  cleanup_fn rm -f $out
> >  
> > -$VG nbdcopy --request-size=33554432 --no-extents -S 0 -- \
> > +$VG nbdcopy --request-size=32M --no-extents -S 0 -- \
> >  [ nbdkit --exit-with-parent data data='
> >   1
> >   @1073741823 1
> > diff --git a/copy/copy-sparse-request-size.sh 
> > b/copy/copy-sparse-request-size.sh
> > index dc8caeafd1..dd28695f68 100755
> > --- a/copy/copy-sparse-request-size.sh
> > +++ b/copy/copy-sparse-request-size.sh
> > @@ -39,7 +39,7 @@ requires nbdkit eval --version
> >  out=copy-sparse-request-size.out
> >  cleanup_fn rm -f $out
> >  
> > -$VG nbdcopy --no-extents -S 0 --request-size=1048576 -- \
> > +$VG nbdcopy --no-extents -S 0 --request-size=1M -- \
> >  [ nbdkit --exit-with-parent data data='
> >   1
> >   @33554431 1
> > diff --git a/copy/main.c b/copy/main.c
> > index 6928a4acde..47b1ea8be0 100644
> > --- a/copy/main.c
> > +++ b/copy/main.c
> > @@ -141,6 +141,8 @@ main (int argc, char *argv[])
> >};
> >int c;
> >size_t i;
> > +  int64_t i64;
> > +  const char *error, *pstr;
> >  
> >/* Set prog to basename argv[0]. */
> >prog = strrchr (argv[0], '/');
> > @@ -210,26 +212,32 @@ main (int argc, char *argv[])
> >break;
> >  
> >  case QUEUE_SIZE_OPTION:
> > -  if (sscanf (optarg, "%u", _size) != 1) {
> > -fprintf (stderr, "%s: --queue-size: could not parse: %s\n",
> > - prog, optarg);
> > +  i64 = human_size_parse (optarg, , );
> > +  if (i64 == -1) {
> > +fprintf (stderr, "%s: --queue-size: %s: %s\n", prog, error, pstr);
> >  exit (EXIT_FAILURE);
> >}
> > +  if (i64 > UINT_MAX) {
> > +fprintf (stderr, "%s: --queue-size is too large\n", prog);
> 
> (1) Print "optarg" (or even format back "i64") here?
> 
> > +exit (EXIT_FAILURE);
> > +  }
> > +  queue_size = i64;
> >break;
> >  
> >  case REQUEST_SIZE_OPTION:
> > -  if (sscanf (optarg, "%u", _size) != 1) {
> > -fprintf (stderr, "%s: --request-size: could not parse: %s\n",
> > - prog, optarg);
> > +  i64 = human_size_parse (optarg, , );
> > +  if (i64 == -1) {
> > +fprintf (stderr, "%s: --request-size: %s: %s\n", prog, error, 
> > pstr);
> >  exit (EXIT_FAILURE);
> >}
> > -  if (request_size < MIN_REQUEST_SIZE || request_size > 
> > MAX_REQUEST_SIZE ||
> > -  !is_power_of_2 (request_size)) {
> > +  if (i64 < MIN_REQUEST_SIZE || i64 > MAX_REQUEST_SIZE ||
> > +  !is_power_of_2 (i64)) {
> >  fprintf (stderr,
> >  "%s: --request-size: must be a power of 2 within %d-%d\n",
> >   prog, MIN_REQUEST_SIZE, MAX_REQUEST_SIZE);
> 
> (2) Same comment as (1).
> 
> (Albeit not as much justified as at (1). At (1), the patch *stops*
> printing the out-of-range "optarg", while at (2), the patch *continues
> not to print* the out-of-range "optarg".)
> 
> >  exit (EXIT_FAILURE);
> >}
> > +  request_size = i64;
> >break;
> 
> (3) I'll come back to this later...
> 
> >  
> >  case 'R':
> > @@ -241,17 +249,18 @@ main (int argc, char *argv[])
> >break;
> >  
> >  case 'S':
> > -  if (sscanf (optarg, "%u", _size) != 1) {
> > -fprintf (stderr, "%s: --sparse: could not parse: %s\n",
> > - prog, optarg);
> > +  i64 = human_size_parse (optarg, , );
> > +  if (i64 == -1) {
> > +fprintf (stderr, "%s: --sparse: %s: %s\n", prog, error, pstr);
> >  exit (EXIT_FAILURE);
> >}
> > -

Re: [Libguestfs] [PATCH libnbd 5/5] copy: Allow human sizes for --queue-size, --request-size, --sparse

2023-09-04 Thread Laszlo Ersek

On 9/3/23 17:23, Richard W.M. Jones wrote:
> Allow these options to be specified using human sizes, for example
> this now works:
> 
>   nbdcopy --request-size=32M ...
> ---
>  copy/copy-sparse-allocated.sh|  2 +-
>  copy/copy-sparse-no-extents.sh   |  2 +-
>  copy/copy-sparse-request-size.sh |  2 +-
>  copy/main.c  | 37 
>  copy/nbdcopy.h   |  2 +-
>  5 files changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/copy/copy-sparse-allocated.sh b/copy/copy-sparse-allocated.sh
> index c65ddea79f..e1fe9cf463 100755
> --- a/copy/copy-sparse-allocated.sh
> +++ b/copy/copy-sparse-allocated.sh
> @@ -31,7 +31,7 @@ requires nbdkit eval --version
>  out=copy-sparse-allocated.out
>  cleanup_fn rm -f $out
>  
> -$VG nbdcopy --allocated --request-size=32768 -- \
> +$VG nbdcopy --allocated --request-size=32K -- \
>  [ nbdkit --exit-with-parent data data='
>   1
>   @1073741823 1
> diff --git a/copy/copy-sparse-no-extents.sh b/copy/copy-sparse-no-extents.sh
> index cff356978b..9368c564e9 100755
> --- a/copy/copy-sparse-no-extents.sh
> +++ b/copy/copy-sparse-no-extents.sh
> @@ -39,7 +39,7 @@ requires nbdkit eval --version
>  out=copy-sparse-no-extents.out
>  cleanup_fn rm -f $out
>  
> -$VG nbdcopy --request-size=33554432 --no-extents -S 0 -- \
> +$VG nbdcopy --request-size=32M --no-extents -S 0 -- \
>  [ nbdkit --exit-with-parent data data='
>   1
>   @1073741823 1
> diff --git a/copy/copy-sparse-request-size.sh 
> b/copy/copy-sparse-request-size.sh
> index dc8caeafd1..dd28695f68 100755
> --- a/copy/copy-sparse-request-size.sh
> +++ b/copy/copy-sparse-request-size.sh
> @@ -39,7 +39,7 @@ requires nbdkit eval --version
>  out=copy-sparse-request-size.out
>  cleanup_fn rm -f $out
>  
> -$VG nbdcopy --no-extents -S 0 --request-size=1048576 -- \
> +$VG nbdcopy --no-extents -S 0 --request-size=1M -- \
>  [ nbdkit --exit-with-parent data data='
>   1
>   @33554431 1
> diff --git a/copy/main.c b/copy/main.c
> index 6928a4acde..47b1ea8be0 100644
> --- a/copy/main.c
> +++ b/copy/main.c
> @@ -141,6 +141,8 @@ main (int argc, char *argv[])
>};
>int c;
>size_t i;
> +  int64_t i64;
> +  const char *error, *pstr;
>  
>/* Set prog to basename argv[0]. */
>prog = strrchr (argv[0], '/');
> @@ -210,26 +212,32 @@ main (int argc, char *argv[])
>break;
>  
>  case QUEUE_SIZE_OPTION:
> -  if (sscanf (optarg, "%u", _size) != 1) {
> -fprintf (stderr, "%s: --queue-size: could not parse: %s\n",
> - prog, optarg);
> +  i64 = human_size_parse (optarg, , );
> +  if (i64 == -1) {
> +fprintf (stderr, "%s: --queue-size: %s: %s\n", prog, error, pstr);
>  exit (EXIT_FAILURE);
>}
> +  if (i64 > UINT_MAX) {
> +fprintf (stderr, "%s: --queue-size is too large\n", prog);

(1) Print "optarg" (or even format back "i64") here?

> +exit (EXIT_FAILURE);
> +  }
> +  queue_size = i64;
>break;
>  
>  case REQUEST_SIZE_OPTION:
> -  if (sscanf (optarg, "%u", _size) != 1) {
> -fprintf (stderr, "%s: --request-size: could not parse: %s\n",
> - prog, optarg);
> +  i64 = human_size_parse (optarg, , );
> +  if (i64 == -1) {
> +fprintf (stderr, "%s: --request-size: %s: %s\n", prog, error, pstr);
>  exit (EXIT_FAILURE);
>}
> -  if (request_size < MIN_REQUEST_SIZE || request_size > MAX_REQUEST_SIZE 
> ||
> -  !is_power_of_2 (request_size)) {
> +  if (i64 < MIN_REQUEST_SIZE || i64 > MAX_REQUEST_SIZE ||
> +  !is_power_of_2 (i64)) {
>  fprintf (stderr,
>  "%s: --request-size: must be a power of 2 within %d-%d\n",
>   prog, MIN_REQUEST_SIZE, MAX_REQUEST_SIZE);

(2) Same comment as (1).

(Albeit not as much justified as at (1). At (1), the patch *stops*
printing the out-of-range "optarg", while at (2), the patch *continues
not to print* the out-of-range "optarg".)

>  exit (EXIT_FAILURE);
>}
> +  request_size = i64;
>break;

(3) I'll come back to this later...

>  
>  case 'R':
> @@ -241,17 +249,18 @@ main (int argc, char *argv[])
>break;
>  
>  case 'S':
> -  if (sscanf (optarg, "%u", _size) != 1) {
> -fprintf (stderr, "%s: --sparse: could not parse: %s\n",
> - prog, optarg);
> +  i64 = human_size_parse (optarg, , );
> +  if (i64 == -1) {
> +fprintf (stderr, "%s: --sparse: %s: %s\n", prog, error, pstr);
>  exit (EXIT_FAILURE);
>}
> -  if (sparse_size != 0 &&
> -  (sparse_size < 512 || !is_power_of_2 (sparse_size))) {
> -fprintf (stderr, "%s: --sparse: must be a power of 2 and >= 512\n",
> +  if (i64 != 0 &&
> +  (i64 < 512 || i64 > UINT_MAX || !is_power_of_2 (i64))) {
> +fprintf (stderr, "%s: --sparse: must be a power of 2,

[Libguestfs] [PATCH libnbd 5/5] copy: Allow human sizes for --queue-size, --request-size, --sparse

2023-09-03 Thread Richard W.M. Jones

Allow these options to be specified using human sizes, for example
this now works:

  nbdcopy --request-size=32M ...
---
 copy/copy-sparse-allocated.sh|  2 +-
 copy/copy-sparse-no-extents.sh   |  2 +-
 copy/copy-sparse-request-size.sh |  2 +-
 copy/main.c  | 37 
 copy/nbdcopy.h   |  2 +-
 5 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/copy/copy-sparse-allocated.sh b/copy/copy-sparse-allocated.sh
index c65ddea79f..e1fe9cf463 100755
--- a/copy/copy-sparse-allocated.sh
+++ b/copy/copy-sparse-allocated.sh
@@ -31,7 +31,7 @@ requires nbdkit eval --version
 out=copy-sparse-allocated.out
 cleanup_fn rm -f $out
 
-$VG nbdcopy --allocated --request-size=32768 -- \
+$VG nbdcopy --allocated --request-size=32K -- \
 [ nbdkit --exit-with-parent data data='
  1
  @1073741823 1
diff --git a/copy/copy-sparse-no-extents.sh b/copy/copy-sparse-no-extents.sh
index cff356978b..9368c564e9 100755
--- a/copy/copy-sparse-no-extents.sh
+++ b/copy/copy-sparse-no-extents.sh
@@ -39,7 +39,7 @@ requires nbdkit eval --version
 out=copy-sparse-no-extents.out
 cleanup_fn rm -f $out
 
-$VG nbdcopy --request-size=33554432 --no-extents -S 0 -- \
+$VG nbdcopy --request-size=32M --no-extents -S 0 -- \
 [ nbdkit --exit-with-parent data data='
  1
  @1073741823 1
diff --git a/copy/copy-sparse-request-size.sh b/copy/copy-sparse-request-size.sh
index dc8caeafd1..dd28695f68 100755
--- a/copy/copy-sparse-request-size.sh
+++ b/copy/copy-sparse-request-size.sh
@@ -39,7 +39,7 @@ requires nbdkit eval --version
 out=copy-sparse-request-size.out
 cleanup_fn rm -f $out
 
-$VG nbdcopy --no-extents -S 0 --request-size=1048576 -- \
+$VG nbdcopy --no-extents -S 0 --request-size=1M -- \
 [ nbdkit --exit-with-parent data data='
  1
  @33554431 1
diff --git a/copy/main.c b/copy/main.c
index 6928a4acde..47b1ea8be0 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -141,6 +141,8 @@ main (int argc, char *argv[])
   };
   int c;
   size_t i;
+  int64_t i64;
+  const char *error, *pstr;
 
   /* Set prog to basename argv[0]. */
   prog = strrchr (argv[0], '/');
@@ -210,26 +212,32 @@ main (int argc, char *argv[])
   break;
 
 case QUEUE_SIZE_OPTION:
-  if (sscanf (optarg, "%u", _size) != 1) {
-fprintf (stderr, "%s: --queue-size: could not parse: %s\n",
- prog, optarg);
+  i64 = human_size_parse (optarg, , );
+  if (i64 == -1) {
+fprintf (stderr, "%s: --queue-size: %s: %s\n", prog, error, pstr);
 exit (EXIT_FAILURE);
   }
+  if (i64 > UINT_MAX) {
+fprintf (stderr, "%s: --queue-size is too large\n", prog);
+exit (EXIT_FAILURE);
+  }
+  queue_size = i64;
   break;
 
 case REQUEST_SIZE_OPTION:
-  if (sscanf (optarg, "%u", _size) != 1) {
-fprintf (stderr, "%s: --request-size: could not parse: %s\n",
- prog, optarg);
+  i64 = human_size_parse (optarg, , );
+  if (i64 == -1) {
+fprintf (stderr, "%s: --request-size: %s: %s\n", prog, error, pstr);
 exit (EXIT_FAILURE);
   }
-  if (request_size < MIN_REQUEST_SIZE || request_size > MAX_REQUEST_SIZE ||
-  !is_power_of_2 (request_size)) {
+  if (i64 < MIN_REQUEST_SIZE || i64 > MAX_REQUEST_SIZE ||
+  !is_power_of_2 (i64)) {
 fprintf (stderr,
 "%s: --request-size: must be a power of 2 within %d-%d\n",
  prog, MIN_REQUEST_SIZE, MAX_REQUEST_SIZE);
 exit (EXIT_FAILURE);
   }
+  request_size = i64;
   break;
 
 case 'R':
@@ -241,17 +249,18 @@ main (int argc, char *argv[])
   break;
 
 case 'S':
-  if (sscanf (optarg, "%u", _size) != 1) {
-fprintf (stderr, "%s: --sparse: could not parse: %s\n",
- prog, optarg);
+  i64 = human_size_parse (optarg, , );
+  if (i64 == -1) {
+fprintf (stderr, "%s: --sparse: %s: %s\n", prog, error, pstr);
 exit (EXIT_FAILURE);
   }
-  if (sparse_size != 0 &&
-  (sparse_size < 512 || !is_power_of_2 (sparse_size))) {
-fprintf (stderr, "%s: --sparse: must be a power of 2 and >= 512\n",
+  if (i64 != 0 &&
+  (i64 < 512 || i64 > UINT_MAX || !is_power_of_2 (i64))) {
+fprintf (stderr, "%s: --sparse: must be a power of 2, between 512 and 
UINT_MAX\n",
  prog);
 exit (EXIT_FAILURE);
   }
+  sparse_size = i64;
   break;
 
 case 'T':
diff --git a/copy/nbdcopy.h b/copy/nbdcopy.h
index 465b7052e7..ade53d1a05 100644
--- a/copy/nbdcopy.h
+++ b/copy/nbdcopy.h
@@ -28,7 +28,7 @@
 #include "vector.h"
 
 #define MIN_REQUEST_SIZE 4096
-#define MAX_REQUEST_SIZE (32 * 1024 * 1024)
+#define MAX_REQUEST_SIZE (32 * 1024 * 1024) /* must be <= UNSIGNED_MAX */
 
 /* This must be a multiple of MAX_REQUEST_SIZE.  Larger is better up
  * to a point, but it reduces the effectiveness of threads

Re: [Libguestfs] [PATCH libnbd 5/5] copy: Allow human sizes for --queue-size, --request-size, --sparse

Re: [Libguestfs] [PATCH libnbd 5/5] copy: Allow human sizes for --queue-size, --request-size, --sparse

[Libguestfs] [PATCH libnbd 5/5] copy: Allow human sizes for --queue-size, --request-size, --sparse

3 matches

Site Navigation

Mail list logo

Footer information