* [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