All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC WIP 0/6] iotests: delinting work-in-progress
@ 2020-05-13 21:41 John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 1/6] iotests: type hint wip John Snow
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: John Snow @ 2020-05-13 21:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, ehabkost, qemu-block, philmd, Max Reitz, John Snow

I ran out of time, but I was briefly entertaining the idea of hitting
everything else in the iotests folder with the pylint and mypy beam.

This is just a draft of what I had at the time, in case someone gets
around to it before I do.

I forced all of the python scripts in this directory to Python3 and pass
the formatter check imposed by the pylintrc that we make iotests.py
adhere to, but there's no guarantee these scripts actually work. I was a
little rough around the edges.

qed.py and qcow2.py in particular abuse some python features that pylint
really doesn't like to see at all, and forcing these to pass a pylint
check was clearly fighting against the idioms in-use in those files,
with more than a few unsatisfactory hacks employed just to make pylint
shut up. They are not necessarily improvements to those scripts.

I created a Makefile check to hit the iotests themselves with the pylint
checker so we can identify when refactors to the testing infrastructure
invalidate tests we don't get to run that often; but didn't get around
to improving the quality of the tests themselves.

(Whether or not this is even worth our time is debatable, it depends on
how much effort it would take to bring them up to par. Maybe it's a lot,
I don't know.)

--js

John Snow (6):
  iotests: type hint wip
  Makefile: add delint WIP
  nbd-fault-injector: delint
  qed.py: delint
  qcow2.py: delint
  WIP: delint test files

 tests/qemu-iotests/Makefile              |   6 +
 tests/qemu-iotests/iotests.py            |  28 ++--
 tests/qemu-iotests/nbd-fault-injector.py |  34 +++--
 tests/qemu-iotests/pylintrc              |   1 +
 tests/qemu-iotests/qcow2.py              | 156 +++++++++++++++--------
 tests/qemu-iotests/qed.py                |  46 ++++---
 6 files changed, 173 insertions(+), 98 deletions(-)

-- 
2.21.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH RFC WIP 1/6] iotests: type hint wip
  2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
@ 2020-05-13 21:41 ` John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 2/6] Makefile: add delint WIP John Snow
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2020-05-13 21:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, ehabkost, qemu-block, philmd, Max Reitz, John Snow

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6c0e781af7..27c477c8a7 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -90,7 +90,7 @@
 luks_default_key_secret_opt = 'key-secret=keysec0'
 
 
-def qemu_img(*args):
+def qemu_img(*args) -> int:
     '''Run qemu-img and return the exit code'''
     devnull = open('/dev/null', 'r+')
     exitcode = subprocess.call(qemu_img_args + list(args),
@@ -113,24 +113,24 @@ def ordered_qmp(qmsg, conv_keys=True):
         return od
     return qmsg
 
-def qemu_img_create(*args):
-    args = list(args)
+def qemu_img_create(*args: str) -> int:
+    qargs = list(args)
 
     # default luks support
-    if '-f' in args and args[args.index('-f') + 1] == 'luks':
-        if '-o' in args:
-            i = args.index('-o')
-            if 'key-secret' not in args[i + 1]:
-                args[i + 1].append(luks_default_key_secret_opt)
-                args.insert(i + 2, '--object')
-                args.insert(i + 3, luks_default_secret_object)
+    if '-f' in qargs and qargs[qargs.index('-f') + 1] == 'luks':
+        if '-o' in qargs:
+            i = qargs.index('-o')
+            if 'key-secret' not in qargs[i + 1]:
+                qargs[i + 1].append(luks_default_key_secret_opt)
+                qargs.insert(i + 2, '--object')
+                qargs.insert(i + 3, luks_default_secret_object)
         else:
-            args = ['-o', luks_default_key_secret_opt,
-                    '--object', luks_default_secret_object] + args
+            qargs = ['-o', luks_default_key_secret_opt,
+                     '--object', luks_default_secret_object] + qargs
 
-    args.insert(0, 'create')
+    qargs.insert(0, 'create')
 
-    return qemu_img(*args)
+    return qemu_img(*qargs)
 
 def qemu_img_verbose(*args):
     '''Run qemu-img without suppressing its output and return the exit code'''
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC WIP 2/6] Makefile: add delint WIP
  2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 1/6] iotests: type hint wip John Snow
@ 2020-05-13 21:41 ` John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 3/6] nbd-fault-injector: delint John Snow
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2020-05-13 21:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, ehabkost, qemu-block, philmd, Max Reitz, John Snow

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/Makefile b/tests/qemu-iotests/Makefile
index 27380e60c1..7dbb7f0fff 100644
--- a/tests/qemu-iotests/Makefile
+++ b/tests/qemu-iotests/Makefile
@@ -1,3 +1,6 @@
+PYMODULES = $(wildcard *.py)
+
+KNOWN_GOOD = iotests.py
 
 CLEANFILES= *.out.bad *.notrun check.log check.time*
 
@@ -7,3 +10,6 @@ default:
 clean:
 	rm -f $(CLEANFILES)
 
+delint:
+	pylint $(KNOWN_GOOD)
+	pylint --disable=R,C,W $(filter-out $(KNOWN_GOOD), $(PYMODULES))
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC WIP 3/6] nbd-fault-injector: delint
  2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 1/6] iotests: type hint wip John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 2/6] Makefile: add delint WIP John Snow
@ 2020-05-13 21:41 ` John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 4/6] qed.py: delint John Snow
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2020-05-13 21:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, ehabkost, qemu-block, philmd, Max Reitz, John Snow

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/Makefile              |  2 +-
 tests/qemu-iotests/nbd-fault-injector.py | 34 ++++++++++++++----------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/tests/qemu-iotests/Makefile b/tests/qemu-iotests/Makefile
index 7dbb7f0fff..64db48342f 100644
--- a/tests/qemu-iotests/Makefile
+++ b/tests/qemu-iotests/Makefile
@@ -1,6 +1,6 @@
 PYMODULES = $(wildcard *.py)
 
-KNOWN_GOOD = iotests.py
+KNOWN_GOOD = iotests.py nbd-fault-injector.py
 
 CLEANFILES= *.out.bad *.notrun check.log check.time*
 
diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
index 588d62aebf..0de6bc643e 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -47,10 +47,7 @@
 import socket
 import struct
 import collections
-if sys.version_info.major >= 3:
-    import configparser
-else:
-    import ConfigParser as configparser
+import configparser
 
 FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
 
@@ -71,7 +68,9 @@
 export_tuple = collections.namedtuple('Export', 'reserved magic opt len')
 export_struct = struct.Struct('>IQII')
 neg2_struct = struct.Struct('>QH124x')
-request_tuple = collections.namedtuple('Request', 'magic type handle from_ len')
+request_tuple = collections.namedtuple(
+    'Request', 'magic type handle from_ len'
+)
 request_struct = struct.Struct('>IIQQI')
 reply_struct = struct.Struct('>IIQ')
 
@@ -84,13 +83,13 @@ def recvall(sock, bufsize):
     chunks = []
     while received < bufsize:
         chunk = sock.recv(bufsize - received)
-        if len(chunk) == 0:
+        if not chunk:
             raise Exception('unexpected disconnect')
         chunks.append(chunk)
         received += len(chunk)
     return b''.join(chunks)
 
-class Rule(object):
+class Rule:
     def __init__(self, name, event, io, when):
         self.name = name
         self.event = event
@@ -104,7 +103,7 @@ def match(self, event, io):
             return False
         return True
 
-class FaultInjectionSocket(object):
+class FaultInjectionSocket:
     def __init__(self, sock, rules):
         self.sock = sock
         self.rules = rules
@@ -150,7 +149,7 @@ def negotiate_export(conn):
     export = export_tuple._make(export_struct.unpack(buf))
     assert export.magic == NBD_OPTS_MAGIC
     assert export.opt == NBD_OPT_EXPORT_NAME
-    name = conn.recv(export.len, event='export-name')
+    _name = conn.recv(export.len, event='export-name')
 
     # Send negotiation part 2
     buf = neg2_struct.pack(FAKE_DISK_SIZE, 0)
@@ -200,7 +199,8 @@ def parse_inject_error(name, options):
     if 'event' not in options:
         err('missing \"event\" option in %s' % name)
     event = options['event']
-    if event not in ('neg-classic', 'neg1', 'export', 'neg2', 'request', 'reply', 'data'):
+    if event not in ('neg-classic', 'neg1', 'export',
+                     'neg2', 'request', 'reply', 'data'):
         err('invalid \"event\" option value \"%s\" in %s' % (event, name))
     io = options.get('io', 'readwrite')
     if io not in ('read', 'write', 'readwrite'):
@@ -229,8 +229,8 @@ def parse_config(config):
 
 def load_rules(filename):
     config = configparser.RawConfigParser()
-    with open(filename, 'rt') as f:
-        config.readfp(f, filename)
+    with open(filename, 'rt') as infile:
+        config.read_file(infile)
     return parse_config(config)
 
 def open_socket(path):
@@ -252,8 +252,14 @@ def open_socket(path):
     return sock
 
 def usage(args):
-    sys.stderr.write('usage: %s [--classic-negotiation] <tcp-port>|<unix-path> <config-file>\n' % args[0])
-    sys.stderr.write('Run an fault injector NBD server with rules defined in a config file.\n')
+    name = args[0]
+    sys.stderr.write(
+        f'usage: {name} [--classic-negotiation] '
+        '<tcp-port>|<unix-path> <config-file>\n'
+    )
+    sys.stderr.write(
+        'Run a fault injector NBD server with '
+        'rules defined in a config file.\n')
     sys.exit(1)
 
 def main(args):
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC WIP 4/6] qed.py: delint
  2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
                   ` (2 preceding siblings ...)
  2020-05-13 21:41 ` [PATCH RFC WIP 3/6] nbd-fault-injector: delint John Snow
@ 2020-05-13 21:41 ` John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 5/6] qcow2.py: delint John Snow
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2020-05-13 21:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, ehabkost, qemu-block, philmd, Max Reitz, John Snow

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/Makefile |  2 +-
 tests/qemu-iotests/qed.py   | 46 ++++++++++++++++++++++++-------------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/tests/qemu-iotests/Makefile b/tests/qemu-iotests/Makefile
index 64db48342f..5a3a1e8092 100644
--- a/tests/qemu-iotests/Makefile
+++ b/tests/qemu-iotests/Makefile
@@ -1,6 +1,6 @@
 PYMODULES = $(wildcard *.py)
 
-KNOWN_GOOD = iotests.py nbd-fault-injector.py
+KNOWN_GOOD = iotests.py nbd-fault-injector.py qed.py
 
 CLEANFILES= *.out.bad *.notrun check.log check.time*
 
diff --git a/tests/qemu-iotests/qed.py b/tests/qemu-iotests/qed.py
index d6bec96069..9e016cc2a2 100755
--- a/tests/qemu-iotests/qed.py
+++ b/tests/qemu-iotests/qed.py
@@ -13,7 +13,6 @@
 import sys
 import struct
 import random
-import optparse
 
 # This can be used as a module
 __all__ = ['QED_F_NEED_CHECK', 'QED']
@@ -47,7 +46,7 @@ def unpack_table_elem(s):
 def pack_table_elem(elem):
     return struct.pack(table_elem_fmt, elem)
 
-class QED(object):
+class QED:
     def __init__(self, f):
         self.f = f
 
@@ -74,19 +73,22 @@ def store_header(self):
     def read_table(self, offset):
         size = self.header['table_size'] * self.header['cluster_size']
         s = self.raw_pread(offset, size)
-        table = [unpack_table_elem(s[i:i + table_elem_size]) for i in xrange(0, size, table_elem_size)]
+        table = [unpack_table_elem(s[i:i + table_elem_size])
+                 for i in range(0, size, table_elem_size)]
         return table
 
     def load_l1_table(self):
         self.l1_table = self.read_table(self.header['l1_table_offset'])
-        self.table_nelems = self.header['table_size'] * self.header['cluster_size'] // table_elem_size
+        self.table_nelems = (self.header['table_size']
+                             * self.header['cluster_size'] // table_elem_size)
 
     def write_table(self, offset, table):
         s = ''.join(pack_table_elem(x) for x in table)
         self.raw_pwrite(offset, s)
 
 def random_table_item(table):
-    vals = [(index, offset) for index, offset in enumerate(table) if offset != 0]
+    vals = [(index, offset) for index, offset
+            in enumerate(table) if offset != 0]
     if not vals:
         err('cannot pick random item because table is empty')
     return random.choice(vals)
@@ -103,7 +105,8 @@ def corrupt_table_duplicate(table):
 def corrupt_table_invalidate(qed, table):
     '''Corrupt a table by introducing an invalid offset'''
     index, _ = random_table_item(table)
-    table[index] = qed.filesize + random.randint(0, 100 * 1024 * 1024 * 1024 * 1024)
+    table[index] = (qed.filesize
+                    + random.randint(0, 100 * 1024 * 1024 * 1024 * 1024))
 
 def cmd_show(qed, *args):
     '''show [header|l1|l2 <offset>]- Show header or l1/l2 tables'''
@@ -144,7 +147,11 @@ def cmd_invalidate(qed, table_level):
     qed.write_table(offset, table)
 
 def cmd_need_check(qed, *args):
-    '''need-check [on|off] - Test, set, or clear the QED_F_NEED_CHECK header bit'''
+    """
+    need-check [on|off]
+
+    Test, set, or clear the QED_F_NEED_CHECK header bit
+    """
     if not args:
         print(bool(qed.header['features'] & QED_F_NEED_CHECK))
         return
@@ -165,7 +172,7 @@ def cmd_zero_cluster(qed, pos, *args):
             err('expected one argument')
         n = int(args[0])
 
-    for i in xrange(n):
+    for _ in range(n):
         l1_index = pos // qed.header['cluster_size'] // len(qed.l1_table)
         if qed.l1_table[l1_index] == 0:
             err('no l2 table allocated')
@@ -179,7 +186,11 @@ def cmd_zero_cluster(qed, pos, *args):
         pos += qed.header['cluster_size']
 
 def cmd_copy_metadata(qed, outfile):
-    '''copy-metadata <outfile> - Copy metadata only (for scrubbing corrupted images)'''
+    """
+    copy-metadata <outfile>
+
+    Copy metadata only (for scrubbing corrupted images)
+    """
     out = open(outfile, 'wb')
 
     # Match file size
@@ -213,23 +224,26 @@ def usage():
     print('Supported commands:')
     for cmd in sorted(x for x in globals() if x.startswith('cmd_')):
         print(globals()[cmd].__doc__)
-    sys.exit(1)
+    return 1
 
 def main():
     if len(sys.argv) < 3:
-        usage()
-    filename, cmd = sys.argv[1:3]
+        return usage()
+    filename = sys.argv[1]
+    cmd = sys.argv[2]
 
     cmd = 'cmd_' + cmd.replace('-', '_')
     if cmd not in globals():
-        usage()
+        return usage()
 
     qed = QED(open(filename, 'r+b'))
     try:
         globals()[cmd](qed, *sys.argv[3:])
-    except TypeError as e:
+    except TypeError:
         sys.stderr.write(globals()[cmd].__doc__ + '\n')
-        sys.exit(1)
+        return 1
+
+    return 0
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC WIP 5/6] qcow2.py: delint
  2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
                   ` (3 preceding siblings ...)
  2020-05-13 21:41 ` [PATCH RFC WIP 4/6] qed.py: delint John Snow
@ 2020-05-13 21:41 ` John Snow
  2020-05-13 21:41 ` [PATCH RFC WIP 6/6] WIP: delint test files John Snow
  2020-05-14  3:15 ` [PATCH RFC WIP 0/6] iotests: delinting work-in-progress no-reply
  6 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2020-05-13 21:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, ehabkost, qemu-block, philmd, Max Reitz, John Snow

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/Makefile |   5 +-
 tests/qemu-iotests/pylintrc |   1 +
 tests/qemu-iotests/qcow2.py | 156 +++++++++++++++++++++++-------------
 3 files changed, 104 insertions(+), 58 deletions(-)

diff --git a/tests/qemu-iotests/Makefile b/tests/qemu-iotests/Makefile
index 5a3a1e8092..77da9fd96d 100644
--- a/tests/qemu-iotests/Makefile
+++ b/tests/qemu-iotests/Makefile
@@ -1,7 +1,5 @@
 PYMODULES = $(wildcard *.py)
 
-KNOWN_GOOD = iotests.py nbd-fault-injector.py qed.py
-
 CLEANFILES= *.out.bad *.notrun check.log check.time*
 
 # no default target
@@ -11,5 +9,4 @@ clean:
 	rm -f $(CLEANFILES)
 
 delint:
-	pylint $(KNOWN_GOOD)
-	pylint --disable=R,C,W $(filter-out $(KNOWN_GOOD), $(PYMODULES))
+	pylint $(PYMODULES)
diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index 5481afe528..a334e242b6 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -17,6 +17,7 @@ disable=invalid-name,
         too-many-lines,
         too-many-locals,
         too-many-public-methods,
+        too-many-instance-attributes,
         # These are temporary, and should be removed:
         missing-docstring,
 
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 94a07b2f6f..2840f4b661 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -11,14 +11,15 @@ def __init__(self, magic, length, data):
             padding = 8 - (length % 8)
             data += b"\0" * padding
 
-        self.magic  = magic
+        self.magic = magic
         self.length = length
-        self.data   = data
+        self.data = data
 
     @classmethod
     def create(cls, magic, data):
         return QcowHeaderExtension(magic, len(data), data)
 
+
 class QcowHeader:
 
     uint32_t = 'I'
@@ -26,30 +27,41 @@ class QcowHeader:
 
     fields = [
         # Version 2 header fields
-        [ uint32_t, '%#x',  'magic' ],
-        [ uint32_t, '%d',   'version' ],
-        [ uint64_t, '%#x',  'backing_file_offset' ],
-        [ uint32_t, '%#x',  'backing_file_size' ],
-        [ uint32_t, '%d',   'cluster_bits' ],
-        [ uint64_t, '%d',   'size' ],
-        [ uint32_t, '%d',   'crypt_method' ],
-        [ uint32_t, '%d',   'l1_size' ],
-        [ uint64_t, '%#x',  'l1_table_offset' ],
-        [ uint64_t, '%#x',  'refcount_table_offset' ],
-        [ uint32_t, '%d',   'refcount_table_clusters' ],
-        [ uint32_t, '%d',   'nb_snapshots' ],
-        [ uint64_t, '%#x',  'snapshot_offset' ],
+        # pylint: disable=bad-whitespace
+        [uint32_t, '%#x',  'magic'],
+        [uint32_t, '%d',   'version'],
+        [uint64_t, '%#x',  'backing_file_offset'],
+        [uint32_t, '%#x',  'backing_file_size'],
+        [uint32_t, '%d',   'cluster_bits'],
+        [uint64_t, '%d',   'size'],
+        [uint32_t, '%d',   'crypt_method'],
+        [uint32_t, '%d',   'l1_size'],
+        [uint64_t, '%#x',  'l1_table_offset'],
+        [uint64_t, '%#x',  'refcount_table_offset'],
+        [uint32_t, '%d',   'refcount_table_clusters'],
+        [uint32_t, '%d',   'nb_snapshots'],
+        [uint64_t, '%#x',  'snapshot_offset'],
 
         # Version 3 header fields
-        [ uint64_t, 'mask', 'incompatible_features' ],
-        [ uint64_t, 'mask', 'compatible_features' ],
-        [ uint64_t, 'mask', 'autoclear_features' ],
-        [ uint32_t, '%d',   'refcount_order' ],
-        [ uint32_t, '%d',   'header_length' ],
-    ];
+        [uint64_t, 'mask', 'incompatible_features'],
+        [uint64_t, 'mask', 'compatible_features'],
+        [uint64_t, 'mask', 'autoclear_features'],
+        [uint32_t, '%d',   'refcount_order'],
+        [uint32_t, '%d',   'header_length'],
+    ]
 
     fmt = '>' + ''.join(field[0] for field in fields)
 
+    @property
+    def backing_file_offset(self):
+        # Pylint struggles to verify dynamic properties.
+        # Dataclasses in 3.7 would make this easy.
+        return self.__dict__['backing_file_offset']
+
+    @backing_file_offset.setter
+    def backing_file_offset(self, val):
+        self.__dict__['backing_file_offset'] = val
+
     def __init__(self, fd):
 
         buf_size = struct.calcsize(QcowHeader.fmt)
@@ -59,22 +71,22 @@ def __init__(self, fd):
 
         header = struct.unpack(QcowHeader.fmt, buf)
         self.__dict__ = dict((field[2], header[i])
-            for i, field in enumerate(QcowHeader.fields))
+                             for i, field in enumerate(QcowHeader.fields))
 
         self.set_defaults()
-        self.cluster_size = 1 << self.cluster_bits
+        self.cluster_size = 1 << self.__dict__['cluster_bits']
 
         fd.seek(self.header_length)
         self.load_extensions(fd)
 
         if self.backing_file_offset:
             fd.seek(self.backing_file_offset)
-            self.backing_file = fd.read(self.backing_file_size)
+            self.backing_file = fd.read(self.__dict__['backing_file_size'])
         else:
             self.backing_file = None
 
     def set_defaults(self):
-        if self.version == 2:
+        if self.__dict__['version'] == 2:
             self.incompatible_features = 0
             self.compatible_features = 0
             self.autoclear_features = 0
@@ -93,10 +105,10 @@ def load_extensions(self, fd):
             (magic, length) = struct.unpack('>II', fd.read(8))
             if magic == 0:
                 break
-            else:
-                padded = (length + 7) & ~7
-                data = fd.read(padded)
-                self.extensions.append(QcowHeaderExtension(magic, length, data))
+            padded = (length + 7) & ~7
+            data = fd.read(padded)
+            extension = QcowHeaderExtension(magic, length, data)
+            self.extensions.append(extension)
 
     def update_extensions(self, fd):
 
@@ -108,7 +120,7 @@ def update_extensions(self, fd):
             fd.write(buf)
             fd.write(ex.data)
 
-        if self.backing_file != None:
+        if self.backing_file is None:
             self.backing_file_offset = fd.tell()
             fd.write(self.backing_file)
 
@@ -170,13 +182,13 @@ def cmd_dump_header_exts(fd):
 def cmd_set_header(fd, name, value):
     try:
         value = int(value, 0)
-    except:
-        print("'%s' is not a valid number" % value)
+    except ValueError:
+        print(f"'{value}' is not a valid number")
         sys.exit(1)
 
     fields = (field[2] for field in QcowHeader.fields)
     if not name in fields:
-        print("'%s' is not a known header field" % name)
+        print("'{name}' is not a known header field")
         sys.exit(1)
 
     h = QcowHeader(fd)
@@ -186,12 +198,13 @@ def cmd_set_header(fd, name, value):
 def cmd_add_header_ext(fd, magic, data):
     try:
         magic = int(magic, 0)
-    except:
-        print("'%s' is not a valid magic number" % magic)
+    except ValueError:
+        print(f"'{magic}' is not a valid magic number")
         sys.exit(1)
 
     h = QcowHeader(fd)
-    h.extensions.append(QcowHeaderExtension.create(magic, data.encode('ascii')))
+    ext = QcowHeaderExtension.create(magic, data.encode('ascii'))
+    h.extensions.append(ext)
     h.update(fd)
 
 def cmd_add_header_ext_stdio(fd, magic):
@@ -201,8 +214,8 @@ def cmd_add_header_ext_stdio(fd, magic):
 def cmd_del_header_ext(fd, magic):
     try:
         magic = int(magic, 0)
-    except:
-        print("'%s' is not a valid magic number" % magic)
+    except ValueError:
+        print(f"'{magic}' is not a valid magic number")
         sys.exit(1)
 
     h = QcowHeader(fd)
@@ -224,8 +237,8 @@ def cmd_set_feature_bit(fd, group, bit):
         bit = int(bit, 0)
         if bit < 0 or bit >= 64:
             raise ValueError
-    except:
-        print("'%s' is not a valid bit number in range [0, 64)" % bit)
+    except ValueError:
+        print(f"'{bit}' is not a valid bit number in range [0, 64)")
         sys.exit(1)
 
     h = QcowHeader(fd)
@@ -236,33 +249,68 @@ def cmd_set_feature_bit(fd, group, bit):
     elif group == 'autoclear':
         h.autoclear_features |= 1 << bit
     else:
-        print("'%s' is not a valid group, try 'incompatible', 'compatible', or 'autoclear'" % group)
+        print(f"'{group}' is not a valid group, "
+              "try 'incompatible', 'compatible', or 'autoclear'")
         sys.exit(1)
 
     h.update(fd)
 
 cmds = [
-    [ 'dump-header',          cmd_dump_header,          0, 'Dump image header and header extensions' ],
-    [ 'dump-header-exts',     cmd_dump_header_exts,     0, 'Dump image header extensions' ],
-    [ 'set-header',           cmd_set_header,           2, 'Set a field in the header'],
-    [ 'add-header-ext',       cmd_add_header_ext,       2, 'Add a header extension' ],
-    [ 'add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin' ],
-    [ 'del-header-ext',       cmd_del_header_ext,       1, 'Delete a header extension' ],
-    [ 'set-feature-bit',      cmd_set_feature_bit,      2, 'Set a feature bit'],
+    [
+        'dump-header',
+        cmd_dump_header,
+        0,
+        'Dump image header and header extensions'
+    ],
+    [
+        'dump-header-exts',
+        cmd_dump_header_exts,
+        0,
+        'Dump image header extensions'
+    ],
+    [
+        'set-header',
+        cmd_set_header,
+        2,
+        'Set a field in the header'
+    ],
+    [
+        'add-header-ext',
+        cmd_add_header_ext,
+        2,
+        'Add a header extension'
+    ],
+    [
+        'add-header-ext-stdio',
+        cmd_add_header_ext_stdio,
+        1,
+        'Add a header extension, data from stdin'
+    ],
+    [
+        'del-header-ext',
+        cmd_del_header_ext,
+        1,
+        'Delete a header extension'
+    ],
+    [
+        'set-feature-bit',
+        cmd_set_feature_bit,
+        2,
+        'Set a feature bit'
+    ],
 ]
 
 def main(filename, cmd, args):
     fd = open(filename, "r+b")
     try:
-        for name, handler, num_args, desc in cmds:
+        for name, handler, num_args, _desc in cmds:
             if name != cmd:
                 continue
-            elif len(args) != num_args:
+            if len(args) != num_args:
                 usage()
                 return
-            else:
-                handler(fd, *args)
-                return
+            handler(fd, *args)
+            return
         print("Unknown command '%s'" % cmd)
     finally:
         fd.close()
@@ -271,7 +319,7 @@ def usage():
     print("Usage: %s <file> <cmd> [<arg>, ...]" % sys.argv[0])
     print("")
     print("Supported commands:")
-    for name, handler, num_args, desc in cmds:
+    for name, _handler, _num_args, desc in cmds:
         print("    %-20s - %s" % (name, desc))
 
 if __name__ == '__main__':
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC WIP 6/6] WIP: delint test files
  2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
                   ` (4 preceding siblings ...)
  2020-05-13 21:41 ` [PATCH RFC WIP 5/6] qcow2.py: delint John Snow
@ 2020-05-13 21:41 ` John Snow
  2020-05-14  3:15 ` [PATCH RFC WIP 0/6] iotests: delinting work-in-progress no-reply
  6 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2020-05-13 21:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, ehabkost, qemu-block, philmd, Max Reitz, John Snow

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/Makefile b/tests/qemu-iotests/Makefile
index 77da9fd96d..fd85eb5bae 100644
--- a/tests/qemu-iotests/Makefile
+++ b/tests/qemu-iotests/Makefile
@@ -1,4 +1,6 @@
 PYMODULES = $(wildcard *.py)
+PYTESTS := $(shell grep -rl '^\#!/usr/bin/env python3' *)
+PYTESTS := $(filter-out $(PYMODULES), $(PYTESTS))
 
 CLEANFILES= *.out.bad *.notrun check.log check.time*
 
@@ -10,3 +12,4 @@ clean:
 
 delint:
 	pylint $(PYMODULES)
+	pylint --disable=R,C,W $(PYTESTS)
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC WIP 0/6] iotests: delinting work-in-progress
  2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
                   ` (5 preceding siblings ...)
  2020-05-13 21:41 ` [PATCH RFC WIP 6/6] WIP: delint test files John Snow
@ 2020-05-14  3:15 ` no-reply
  6 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2020-05-14  3:15 UTC (permalink / raw)
  To: jsnow; +Cc: kwolf, ehabkost, qemu-block, jsnow, qemu-devel, mreitz, philmd

Patchew URL: https://patchew.org/QEMU/20200513214130.15375-1-jsnow@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

Not run: 259
Failures: 031 036 054 061 114
Failed 5 of 118 iotests
make: *** [check-tests/check-block.sh] Error 1
make: *** Waiting for unfinished jobs....
  TEST    check-qtest-aarch64: tests/qtest/qos-test
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=a364a3916b304854887e4b7475175383', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-n73ihxu6/src/docker-src.2020-05-13-23.00.06.31673:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=a364a3916b304854887e4b7475175383
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-n73ihxu6/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    15m47.213s
user    0m5.361s


The full log is available at
http://patchew.org/logs/20200513214130.15375-1-jsnow@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-05-14  3:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 21:41 [PATCH RFC WIP 0/6] iotests: delinting work-in-progress John Snow
2020-05-13 21:41 ` [PATCH RFC WIP 1/6] iotests: type hint wip John Snow
2020-05-13 21:41 ` [PATCH RFC WIP 2/6] Makefile: add delint WIP John Snow
2020-05-13 21:41 ` [PATCH RFC WIP 3/6] nbd-fault-injector: delint John Snow
2020-05-13 21:41 ` [PATCH RFC WIP 4/6] qed.py: delint John Snow
2020-05-13 21:41 ` [PATCH RFC WIP 5/6] qcow2.py: delint John Snow
2020-05-13 21:41 ` [PATCH RFC WIP 6/6] WIP: delint test files John Snow
2020-05-14  3:15 ` [PATCH RFC WIP 0/6] iotests: delinting work-in-progress no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.