[Libguestfs] [PATCH nbdinfo v2 3/3] copy: Print debug information with human sizes

[Richard W.M. Jones]

On Mon, Sep 20, 2021 at 06:32:29PM +0200, Laszlo Ersek wrote:
> On 09/20/21 13:04, Richard W.M. Jones wrote:
> > ---
> >  copy/main.c          | 6 +++++-
> >  copy/test-verbose.sh | 4 ++--
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/copy/main.c b/copy/main.c
> > index 70534b5a..15a64544 100644
> > --- a/copy/main.c
> > +++ b/copy/main.c
> > @@ -39,6 +39,7 @@
> >  #include <libnbd.h>
> >  
> >  #include "ispowerof2.h"
> > +#include "human-size.h"
> >  #include "version.h"
> >  #include "nbdcopy.h"
> >  
> > @@ -508,8 +509,11 @@ open_local (const char *filename, direction d)
> >  static void
> >  print_rw (struct rw *rw, const char *prefix, FILE *fp)
> >  {
> > +  char buf[HUMAN_SIZE_LONGEST];
> > +
> >    fprintf (fp, "%s: %s \"%s\"\n", prefix, rw->ops->ops_name, rw->name);
> > -  fprintf (fp, "%s: size=%" PRIi64 "\n", prefix, rw->size);
> > +  fprintf (fp, "%s: size=%" PRIi64 " (%s)\n",
> > +           prefix, rw->size, human_size (buf, rw->size, NULL));
> >  }
> >  
> >  /* Default implementation of rw->ops->get_extents for backends which
> 
> Hopefully rw->size is never negative here...

About this and your similar comment on patch 2, the size is returned
by this API:

  https://libguestfs.org/nbd_get_size.3.html

This function returns int64_t because -1 indicates an error, but in
non-error cases the size is always between 0 and INT64_MAX, so we use
int64_t to store the size in programs just to avoid an awkward
conversion.  libnbd and nbdkit both assume and support only export
sizes in this range, indeed we even test it:

  https://gitlab.com/nbdkit/nbdkit/-/blob/master/tests/test-memory-largest.sh

qemu/qemu-nbd doesn't actually support the full range of sizes, and
they've even {changed|broken} [delete as appropriate] what they
support in recent memory, see:

  https://gitlab.com/nbdkit/nbdkit/-/commit/c3ec8c951e39a0f921252c162c236f23c588d2bd

The NBD protocol in theory does not limit export sizes, any unsigned
64 bit size could be used, but I doubt the kernel could handle it
correctly.

> > diff --git a/copy/test-verbose.sh b/copy/test-verbose.sh
> > index afd57580..4cc67d37 100755
> > --- a/copy/test-verbose.sh
> > +++ b/copy/test-verbose.sh
> > @@ -28,11 +28,11 @@ requires nbdkit --version
> >  file=test-verbose.out
> >  cleanup_fn rm -f $file
> >  
> > -$VG nbdcopy -v -- [ nbdkit null ] null: 2>$file
> > +$VG nbdcopy -v -- [ nbdkit memory 1M ] null: 2>$file
> 
> (1) I don't understand this change. Why do we replace "null" with
> "memory 1M"?

I'm afraid I already pushed this change so I cannot update the commit
message, but you're right below that it's to make the output more
interesting.

In fact I had to change it again (to size=1M) because Debian 10 is
shipping some ancient nbdkit that doesn't support the abbreviated "1M"
(without magic size=) form.

Thanks,

Rich.

> (Side question that I've been meaning to ask: what is this "$VG" magic?)
> 
> >  
> >  cat $file
> >  
> >  # Check some known strings appear in the output.
> >  grep '^nbdcopy: src: nbd_ops' $file
> > -grep '^nbdcopy: src: size=0' $file
> > +grep '^nbdcopy: src: size=1048576 (1M)' $file
> >  grep '^nbdcopy: dst: null_ops' $file
> > 
> 
> Ah, the test case is modified at once so it generate more interesting
> output. Can you note that in the commit message please?
> 
> Acked-by: Laszlo Ersek <lersek at redhat.com>
> 
> Thanks!
> Laszlo

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html