Nir Soffer
2021-Dec-21 22:21 UTC
[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 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. 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. Nir
Richard W.M. Jones
2021-Dec-21 22:25 UTC
[Libguestfs] [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/