Libguestfs - Dec 2021 - [PATCH nbdkit 1/2] tests/test-python-plugin.py: Allow test to use large disks

On Wed, Dec 22, 2021 at 12:21:25AM +0200, Nir Soffer
wrote:
> On Tue, Dec 21, 2021 at 11:21 PM Richard W.M. Jones <rjones at
redhat.com> wrote:
> >
> > The Python test harness uses a plugin which always created a fully
> > allocated disk backed by an in-memory bytearray.  This prevented us
> > from testing very large disks (since we could run out of memory
> > easily).
> >
> > Add a feature allowing large all-zero disks to be tested.  The disk is
> > not allocated and non-zero writes will fail, but everything else
> > works.
> > ---
> >  tests/test-python-plugin.py | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/test-python-plugin.py b/tests/test-python-plugin.py
> > index 70d545db..a30b7f64 100644
> > --- a/tests/test-python-plugin.py
> > +++ b/tests/test-python-plugin.py
> > @@ -51,13 +51,17 @@ def config_complete():
> >
> >
> >  def open(readonly):
> > +    if cfg.get('no_disk', False):
> 
> get() default is None, so the idiomatic way is:
> 
>     if cfg.get("no_disk"):
> 
> > +        disk = None
> > +    else:
> > +        disk = bytearray(cfg.get('size', 0))
> >      return {
> > -        'disk': bytearray(cfg.get('size', 0))
> > +        'disk': disk
> >      }
> >
> >
> >  def get_size(h):
> > -    return len(h['disk'])
> > +    return cfg.get('size', 0)
> >
> >
> >  def is_rotational(h):
> > @@ -123,6 +127,7 @@ def pwrite(h, buf, offset, flags):
> >      actual_fua = bool(flags & nbdkit.FLAG_FUA)
> >      assert expect_fua == actual_fua
> >      end = offset + len(buf)
> > +    assert h['disk'] is not None
> >      h['disk'][offset:end] = buf
> >
> >
> > @@ -134,7 +139,8 @@ def trim(h, count, offset, flags):
> >      expect_fua = cfg.get('trim_expect_fua', False)
> >      actual_fua = bool(flags & nbdkit.FLAG_FUA)
> >      assert expect_fua == actual_fua
> > -    h['disk'][offset:offset+count] = bytearray(count)
> > +    if h['disk'] is not None:
> > +        h['disk'][offset:offset+count] = bytearray(count)
> >
> >
> >  def zero(h, count, offset, flags):
> > @@ -147,7 +153,8 @@ def zero(h, count, offset, flags):
> >      expect_fast_zero = cfg.get('zero_expect_fast_zero',
False)
> >      actual_fast_zero = bool(flags & nbdkit.FLAG_FAST_ZERO)
> >      assert expect_fast_zero == actual_fast_zero
> > -    h['disk'][offset:offset+count] = bytearray(count)
> > +    if h['disk'] is not None:
> > +        h['disk'][offset:offset+count] = bytearray(count)
> >
> >
> >  def cache(h, count, offset, flags):
> > --
> > 2.32.0
> >
> 
> Using double negative:
> 
>    {"no_disk": False, ...}
> 
> is little confusing, maybe the default should be:
> 
>    {"create_disk": True, ...}
> 
> And test that don't want to create disk will use:
> 
>     {"create_disk": False, ...}
> 
> Otherwise this looks useful.
Yes, that's a good idea.
> It would be help also if we can test extents argument, currently the
> plugin ignores the argument so we did not detect the issue with the
> wrong format string.
OK, I'll take a look.

Rich.

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

On Tue, Dec 21, 2021 at 10:25:35PM +0000, Richard W.M. Jones
wrote:
> On Wed, Dec 22, 2021 at 12:21:25AM +0200, Nir Soffer wrote:
> > It would be help also if we can test extents argument, currently the
> > plugin ignores the argument so we did not detect the issue with the
> > wrong format string.
> 
> OK, I'll take a look.
I see - the plugin function is defined just to return the extents sent
in (for testing):

  def extents(h, count, offset, flags):
      return cfg.get('extents', [])

This means we don't test the parameters.  However to test those
realistically I suppose we would really need to support sparse disks
in this test plugin, which seems quite tricky.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org