| Message ID | 1594676203-436999-9-git-send-email-andrey.shinkevich@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: Dump QCOW2 dirty bitmaps metadata | expand |
14.07.2020 00:36, Andrey Shinkevich wrote: > Add the command key to the qcow2.py arguments list to dump QCOW2 > metadata in JSON format. Here is the suggested way to do that. The > implementation of the dump in JSON format is in the patch that follows. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > tests/qemu-iotests/qcow2.py | 19 +++++++++++++++---- > tests/qemu-iotests/qcow2_format.py | 16 ++++++++-------- > 2 files changed, 23 insertions(+), 12 deletions(-) > > diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py > index 0910e6a..7402279 100755 > --- a/tests/qemu-iotests/qcow2.py > +++ b/tests/qemu-iotests/qcow2.py > @@ -26,16 +26,19 @@ from qcow2_format import ( > ) > > > +dump_json = False > + > + > def cmd_dump_header(fd): > h = QcowHeader(fd) > - h.dump() > + h.dump(dump_json) > print() > - h.dump_extensions() > + h.dump_extensions(dump_json) > > > def cmd_dump_header_exts(fd): > h = QcowHeader(fd) > - h.dump_extensions() > + h.dump_extensions(dump_json) > > > def cmd_set_header(fd, name, value): > @@ -134,6 +137,11 @@ cmds = [ > > > def main(filename, cmd, args): > + global dump_json > + dump_json = '-j' in sys.argv > + if dump_json: > + sys.argv.remove('-j') > + args.remove('-j') I'd prefer to access sys.argv in one place (in "if __name__ ..."). > fd = open(filename, "r+b") > try: > for name, handler, num_args, desc in cmds: > @@ -151,11 +159,14 @@ def main(filename, cmd, args): > > > def usage(): > - print("Usage: %s <file> <cmd> [<arg>, ...]" % sys.argv[0]) > + print("Usage: %s <file> <cmd> [<arg>, ...] [<key>, ...]" % sys.argv[0]) > print("") > print("Supported commands:") > for name, handler, num_args, desc in cmds: > print(" %-20s - %s" % (name, desc)) > + print("") > + print("Supported keys:") > + print(" %-20s - %s" % ('-j', 'Dump in JSON format')) > > > if __name__ == '__main__': > diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py > index 2c78d46..e0e14b5 100644 > --- a/tests/qemu-iotests/qcow2_format.py > +++ b/tests/qemu-iotests/qcow2_format.py > @@ -109,7 +109,7 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): > self.__dict__ = dict((field[2], values[i]) > for i, field in enumerate(self.fields)) > > - def dump(self): > + def dump(self, dump_json=None): > for f in self.fields: > value = self.__dict__[f[2]] > if isinstance(f[1], str): > @@ -145,8 +145,8 @@ class Qcow2BitmapExt(Qcow2Struct): > [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) > for _ in range(self.nb_bitmaps)] > > - def dump(self): > - super().dump() > + def dump(self, dump_json=None): strange to make None the default value for boolean. Why not just False? Also, why not call it just "json"? We are already in "dump" context, no needs to add prefix to the name. > + super().dump(dump_json) > for entry in self.bitmap_directory: > print() > entry.dump() How will it work? You are interleaving json dump and non-json? Looking at this, I think that json and non-json dumps has more differences than similarities, and it probably simpler to make a separate function dump_json.. But I'm absolutely against an option, if it will be done consistently. > @@ -190,7 +190,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): > self.bitmap_table = Qcow2BitmapTable(raw_table=table, > cluster_size=self.cluster_size) > > - def dump(self): > + def dump(self, dump_json=None): > print(f'{"Bitmap name":<25} {self.name}') > super(Qcow2BitmapDirEntry, self).dump() > self.bitmap_table.dump() Here the new option just not passed to the nested dump() calls.. > @@ -291,13 +291,13 @@ class QcowHeaderExtension(Qcow2Struct): > data_str = '<binary>' > self.data_str = data_str > > - def dump(self): > + def dump(self, dump_json=None): > super().dump() > > if self.obj is None: > print(f'{"data":<25} {self.data_str}') > else: > - self.obj.dump() > + self.obj.dump(dump_json) > > @classmethod > def create(cls, magic, data): > @@ -396,8 +396,8 @@ class QcowHeader(Qcow2Struct): > buf = buf[0:header_bytes-1] > fd.write(buf) > > - def dump_extensions(self): > + def dump_extensions(self, dump_json=None): > for ex in self.extensions: > print('Header extension:') > - ex.dump() > + ex.dump(dump_json) > print() >
On 16.07.2020 13:20, Vladimir Sementsov-Ogievskiy wrote: > 14.07.2020 00:36, Andrey Shinkevich wrote: >> Add the command key to the qcow2.py arguments list to dump QCOW2 >> metadata in JSON format. Here is the suggested way to do that. The >> implementation of the dump in JSON format is in the patch that follows. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> --- >> tests/qemu-iotests/qcow2.py | 19 +++++++++++++++---- >> tests/qemu-iotests/qcow2_format.py | 16 ++++++++-------- >> 2 files changed, 23 insertions(+), 12 deletions(-) >> >> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py >> index 0910e6a..7402279 100755 ... >> @@ -134,6 +137,11 @@ cmds = [ >> def main(filename, cmd, args): >> + global dump_json >> + dump_json = '-j' in sys.argv >> + if dump_json: >> + sys.argv.remove('-j') >> + args.remove('-j') > > I'd prefer to access sys.argv in one place (in "if __name__ ..."). > In case we move the block global dump_json dump_json = '-j' in sys.argv if dump_json: sys.argv.remove('-j') from main() to the body of "if __name__ == '__main__'" we get the following error - "SyntaxError: name 'dump_json' is assigned to before global declaration" Andrey >> fd = open(filename, "r+b") >> try: >> for name, handler, num_args, desc in cmds: >> @@ -151,11 +159,14 @@ def main(filename, cmd, args): >> def usage(): >> - print("Usage: %s <file> <cmd> [<arg>, ...]" % sys.argv[0]) >> + print("Usage: %s <file> <cmd> [<arg>, ...] [<key>, ...]" % >> sys.argv[0]) >> print("") >> print("Supported commands:") >> for name, handler, num_args, desc in cmds: >> print(" %-20s - %s" % (name, desc)) >> + print("") >> + print("Supported keys:") >> + print(" %-20s - %s" % ('-j', 'Dump in JSON format')) >> if __name__ == '__main__': >> diff --git a/tests/qemu-iotests/qcow2_format.py >> b/tests/qemu-iotests/qcow2_format.py >> index 2c78d46..e0e14b5 100644 >> --- a/tests/qemu-iotests/qcow2_format.py >> +++ b/tests/qemu-iotests/qcow2_format.py >> @@ -109,7 +109,7 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): >> self.__dict__ = dict((field[2], values[i]) >> for i, field in enumerate(self.fields)) >> - def dump(self): >> + def dump(self, dump_json=None): >> for f in self.fields: >> value = self.__dict__[f[2]] >> if isinstance(f[1], str): >> @@ -145,8 +145,8 @@ class Qcow2BitmapExt(Qcow2Struct): >> [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) >> for _ in range(self.nb_bitmaps)] >> - def dump(self): >> - super().dump() >> + def dump(self, dump_json=None): > > strange to make None the default value for boolean. Why not just False? > > Also, why not call it just "json"? We are already in "dump" context, > no needs to add prefix to the name. > >> + super().dump(dump_json) >> for entry in self.bitmap_directory: >> print() >> entry.dump() > > How will it work? You are interleaving json dump and non-json? > > Looking at this, I think that json and non-json dumps has more > differences than similarities, and it probably simpler to make a > separate function dump_json.. But I'm absolutely against an option, if > it will be done consistently. > >> @@ -190,7 +190,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): >> self.bitmap_table = Qcow2BitmapTable(raw_table=table, >> cluster_size=self.cluster_size) >> - def dump(self): >> + def dump(self, dump_json=None): >> print(f'{"Bitmap name":<25} {self.name}') >> super(Qcow2BitmapDirEntry, self).dump() >> self.bitmap_table.dump() > > Here the new option just not passed to the nested dump() calls.. > >> @@ -291,13 +291,13 @@ class QcowHeaderExtension(Qcow2Struct): >> data_str = '<binary>' >> self.data_str = data_str >> - def dump(self): >> + def dump(self, dump_json=None): >> super().dump() >> if self.obj is None: >> print(f'{"data":<25} {self.data_str}') >> else: >> - self.obj.dump() >> + self.obj.dump(dump_json) >> @classmethod >> def create(cls, magic, data): >> @@ -396,8 +396,8 @@ class QcowHeader(Qcow2Struct): >> buf = buf[0:header_bytes-1] >> fd.write(buf) >> - def dump_extensions(self): >> + def dump_extensions(self, dump_json=None): >> for ex in self.extensions: >> print('Header extension:') >> - ex.dump() >> + ex.dump(dump_json) >> print() >> > >
16.07.2020 17:36, Andrey Shinkevich wrote: > On 16.07.2020 13:20, Vladimir Sementsov-Ogievskiy wrote: >> 14.07.2020 00:36, Andrey Shinkevich wrote: >>> Add the command key to the qcow2.py arguments list to dump QCOW2 >>> metadata in JSON format. Here is the suggested way to do that. The >>> implementation of the dump in JSON format is in the patch that follows. >>> >>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >>> --- >>> tests/qemu-iotests/qcow2.py | 19 +++++++++++++++---- >>> tests/qemu-iotests/qcow2_format.py | 16 ++++++++-------- >>> 2 files changed, 23 insertions(+), 12 deletions(-) >>> >>> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py >>> index 0910e6a..7402279 100755 > ... >>> @@ -134,6 +137,11 @@ cmds = [ >>> def main(filename, cmd, args): >>> + global dump_json >>> + dump_json = '-j' in sys.argv >>> + if dump_json: >>> + sys.argv.remove('-j') >>> + args.remove('-j') >> >> I'd prefer to access sys.argv in one place (in "if __name__ ..."). >> > > In case we move the block > > global dump_json > dump_json = '-j' in sys.argv > > if dump_json: > sys.argv.remove('-j') > > from main() to the body of "if __name__ == '__main__'" > > we get the following error - > > "SyntaxError: name 'dump_json' is assigned to before global declaration" you don't need "global" declaration outside of a function
On 16.07.2020 17:38, Vladimir Sementsov-Ogievskiy wrote: > 16.07.2020 17:36, Andrey Shinkevich wrote: >> On 16.07.2020 13:20, Vladimir Sementsov-Ogievskiy wrote: >>> 14.07.2020 00:36, Andrey Shinkevich wrote: >>>> Add the command key to the qcow2.py arguments list to dump QCOW2 >>>> metadata in JSON format. Here is the suggested way to do that. The >>>> implementation of the dump in JSON format is in the patch that >>>> follows. >>>> >>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >>>> --- >>>> tests/qemu-iotests/qcow2.py | 19 +++++++++++++++---- >>>> tests/qemu-iotests/qcow2_format.py | 16 ++++++++-------- >>>> 2 files changed, 23 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py >>>> index 0910e6a..7402279 100755 >> ... >>>> @@ -134,6 +137,11 @@ cmds = [ >>>> def main(filename, cmd, args): >>>> + global dump_json >>>> + dump_json = '-j' in sys.argv >>>> + if dump_json: >>>> + sys.argv.remove('-j') >>>> + args.remove('-j') >>> >>> I'd prefer to access sys.argv in one place (in "if __name__ ..."). >>> >> >> In case we move the block >> >> global dump_json >> dump_json = '-j' in sys.argv >> >> if dump_json: >> sys.argv.remove('-j') >> >> from main() to the body of "if __name__ == '__main__'" >> >> we get the following error - >> >> "SyntaxError: name 'dump_json' is assigned to before global declaration" > > you don't need "global" declaration outside of a function > > OK, thanks. However, if we want to parse more keys in future, will we do all that parsing in the body of "if __name__ == '__main__'"? Andrey
16.07.2020 17:44, Andrey Shinkevich wrote: > On 16.07.2020 17:38, Vladimir Sementsov-Ogievskiy wrote: >> 16.07.2020 17:36, Andrey Shinkevich wrote: >>> On 16.07.2020 13:20, Vladimir Sementsov-Ogievskiy wrote: >>>> 14.07.2020 00:36, Andrey Shinkevich wrote: >>>>> Add the command key to the qcow2.py arguments list to dump QCOW2 >>>>> metadata in JSON format. Here is the suggested way to do that. The >>>>> implementation of the dump in JSON format is in the patch that follows. >>>>> >>>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >>>>> --- >>>>> tests/qemu-iotests/qcow2.py | 19 +++++++++++++++---- >>>>> tests/qemu-iotests/qcow2_format.py | 16 ++++++++-------- >>>>> 2 files changed, 23 insertions(+), 12 deletions(-) >>>>> >>>>> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py >>>>> index 0910e6a..7402279 100755 >>> ... >>>>> @@ -134,6 +137,11 @@ cmds = [ >>>>> def main(filename, cmd, args): >>>>> + global dump_json >>>>> + dump_json = '-j' in sys.argv >>>>> + if dump_json: >>>>> + sys.argv.remove('-j') >>>>> + args.remove('-j') >>>> >>>> I'd prefer to access sys.argv in one place (in "if __name__ ..."). >>>> >>> >>> In case we move the block >>> >>> global dump_json >>> dump_json = '-j' in sys.argv >>> >>> if dump_json: >>> sys.argv.remove('-j') >>> >>> from main() to the body of "if __name__ == '__main__'" >>> >>> we get the following error - >>> >>> "SyntaxError: name 'dump_json' is assigned to before global declaration" >> >> you don't need "global" declaration outside of a function >> >> > > OK, thanks. However, if we want to parse more keys in future, will we do all that parsing in the body of "if __name__ == '__main__'"? > If we are going to parse more key, we should move to ArgumentParser first. It may be in body of "if" or in a function..
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py index 0910e6a..7402279 100755 --- a/tests/qemu-iotests/qcow2.py +++ b/tests/qemu-iotests/qcow2.py @@ -26,16 +26,19 @@ from qcow2_format import ( ) +dump_json = False + + def cmd_dump_header(fd): h = QcowHeader(fd) - h.dump() + h.dump(dump_json) print() - h.dump_extensions() + h.dump_extensions(dump_json) def cmd_dump_header_exts(fd): h = QcowHeader(fd) - h.dump_extensions() + h.dump_extensions(dump_json) def cmd_set_header(fd, name, value): @@ -134,6 +137,11 @@ cmds = [ def main(filename, cmd, args): + global dump_json + dump_json = '-j' in sys.argv + if dump_json: + sys.argv.remove('-j') + args.remove('-j') fd = open(filename, "r+b") try: for name, handler, num_args, desc in cmds: @@ -151,11 +159,14 @@ def main(filename, cmd, args): def usage(): - print("Usage: %s <file> <cmd> [<arg>, ...]" % sys.argv[0]) + print("Usage: %s <file> <cmd> [<arg>, ...] [<key>, ...]" % sys.argv[0]) print("") print("Supported commands:") for name, handler, num_args, desc in cmds: print(" %-20s - %s" % (name, desc)) + print("") + print("Supported keys:") + print(" %-20s - %s" % ('-j', 'Dump in JSON format')) if __name__ == '__main__': diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py index 2c78d46..e0e14b5 100644 --- a/tests/qemu-iotests/qcow2_format.py +++ b/tests/qemu-iotests/qcow2_format.py @@ -109,7 +109,7 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): self.__dict__ = dict((field[2], values[i]) for i, field in enumerate(self.fields)) - def dump(self): + def dump(self, dump_json=None): for f in self.fields: value = self.__dict__[f[2]] if isinstance(f[1], str): @@ -145,8 +145,8 @@ class Qcow2BitmapExt(Qcow2Struct): [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) for _ in range(self.nb_bitmaps)] - def dump(self): - super().dump() + def dump(self, dump_json=None): + super().dump(dump_json) for entry in self.bitmap_directory: print() entry.dump() @@ -190,7 +190,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): self.bitmap_table = Qcow2BitmapTable(raw_table=table, cluster_size=self.cluster_size) - def dump(self): + def dump(self, dump_json=None): print(f'{"Bitmap name":<25} {self.name}') super(Qcow2BitmapDirEntry, self).dump() self.bitmap_table.dump() @@ -291,13 +291,13 @@ class QcowHeaderExtension(Qcow2Struct): data_str = '<binary>' self.data_str = data_str - def dump(self): + def dump(self, dump_json=None): super().dump() if self.obj is None: print(f'{"data":<25} {self.data_str}') else: - self.obj.dump() + self.obj.dump(dump_json) @classmethod def create(cls, magic, data): @@ -396,8 +396,8 @@ class QcowHeader(Qcow2Struct): buf = buf[0:header_bytes-1] fd.write(buf) - def dump_extensions(self): + def dump_extensions(self, dump_json=None): for ex in self.extensions: print('Header extension:') - ex.dump() + ex.dump(dump_json) print()
Add the command key to the qcow2.py arguments list to dump QCOW2 metadata in JSON format. Here is the suggested way to do that. The implementation of the dump in JSON format is in the patch that follows. Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> --- tests/qemu-iotests/qcow2.py | 19 +++++++++++++++---- tests/qemu-iotests/qcow2_format.py | 16 ++++++++-------- 2 files changed, 23 insertions(+), 12 deletions(-)