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

On Tue, Dec 21, 2021 at 11:21 PM Richard W.M. Jones <rjones(a)redhat.com&gt; 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.

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.