| Message ID | 1594676203-436999-10-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: > As __dict__ is being extended with class members we do not want to > print, make a light copy of the initial __dict__ and extend the copy > by adding lists we have to print in the JSON output. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > tests/qemu-iotests/qcow2_format.py | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py > index e0e14b5..83c3482 100644 > --- a/tests/qemu-iotests/qcow2_format.py > +++ b/tests/qemu-iotests/qcow2_format.py > @@ -109,6 +109,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): > self.__dict__ = dict((field[2], values[i]) > for i, field in enumerate(self.fields)) > > + self.fields_dict = self.__dict__.copy() No, I don't like that. Keeping two copies of all the data is bad idea. If you want to select some fields, add a method (dump_json() ?) Which will collect the fields you want in a dict and return that new dict. But don't store object attributes twice. > + > def dump(self, dump_json=None): > for f in self.fields: > value = self.__dict__[f[2]] > @@ -144,6 +146,7 @@ class Qcow2BitmapExt(Qcow2Struct): > self.bitmap_directory = \ > [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) > for _ in range(self.nb_bitmaps)] > + self.fields_dict.update(bitmap_directory=self.bitmap_directory) > > def dump(self, dump_json=None): > super().dump(dump_json) > @@ -189,6 +192,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): > table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))] > self.bitmap_table = Qcow2BitmapTable(raw_table=table, > cluster_size=self.cluster_size) > + self.fields_dict.update(bitmap_table=self.bitmap_table) > > def dump(self, dump_json=None): > print(f'{"Bitmap name":<25} {self.name}') >
On 16.07.2020 13:24, Vladimir Sementsov-Ogievskiy wrote: > 14.07.2020 00:36, Andrey Shinkevich wrote: >> As __dict__ is being extended with class members we do not want to >> print, make a light copy of the initial __dict__ and extend the copy >> by adding lists we have to print in the JSON output. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> --- >> tests/qemu-iotests/qcow2_format.py | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/tests/qemu-iotests/qcow2_format.py >> b/tests/qemu-iotests/qcow2_format.py >> index e0e14b5..83c3482 100644 >> --- a/tests/qemu-iotests/qcow2_format.py >> +++ b/tests/qemu-iotests/qcow2_format.py >> @@ -109,6 +109,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): >> self.__dict__ = dict((field[2], values[i]) >> for i, field in enumerate(self.fields)) >> + self.fields_dict = self.__dict__.copy() > > No, I don't like that. Keeping two copies of all the data is bad idea. > If you want to select some fields, add a method (dump_json() ?) Which > will collect the fields you want in a dict and return that new dict. > But don't store object attributes twice. > Not really. It makes a light copy that stores only references to the desired fields. Andrey >> + >> def dump(self, dump_json=None): >> for f in self.fields: >> value = self.__dict__[f[2]] >> @@ -144,6 +146,7 @@ class Qcow2BitmapExt(Qcow2Struct): >> self.bitmap_directory = \ >> [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) >> for _ in range(self.nb_bitmaps)] >> + self.fields_dict.update(bitmap_directory=self.bitmap_directory) >> def dump(self, dump_json=None): >> super().dump(dump_json) >> @@ -189,6 +192,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): >> table = [e[0] for e in struct.iter_unpack('>Q', >> fd.read(table_size))] >> self.bitmap_table = Qcow2BitmapTable(raw_table=table, >> cluster_size=self.cluster_size) >> + self.fields_dict.update(bitmap_table=self.bitmap_table) >> def dump(self, dump_json=None): >> print(f'{"Bitmap name":<25} {self.name}') >> > >
16.07.2020 18:34, Andrey Shinkevich wrote: > On 16.07.2020 13:24, Vladimir Sementsov-Ogievskiy wrote: >> 14.07.2020 00:36, Andrey Shinkevich wrote: >>> As __dict__ is being extended with class members we do not want to >>> print, make a light copy of the initial __dict__ and extend the copy >>> by adding lists we have to print in the JSON output. >>> >>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >>> --- >>> tests/qemu-iotests/qcow2_format.py | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py >>> index e0e14b5..83c3482 100644 >>> --- a/tests/qemu-iotests/qcow2_format.py >>> +++ b/tests/qemu-iotests/qcow2_format.py >>> @@ -109,6 +109,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): >>> self.__dict__ = dict((field[2], values[i]) >>> for i, field in enumerate(self.fields)) >>> + self.fields_dict = self.__dict__.copy() >> >> No, I don't like that. Keeping two copies of all the data is bad idea. If you want to select some fields, add a method (dump_json() ?) Which will collect the fields you want in a dict and return that new dict. But don't store object attributes twice. >> > > Not really. It makes a light copy that stores only references to the desired fields. > I'm not about memory consumption, I'm about the design. Keeping two representations of same thing is always painful to maintain. > >>> + >>> def dump(self, dump_json=None): >>> for f in self.fields: >>> value = self.__dict__[f[2]] >>> @@ -144,6 +146,7 @@ class Qcow2BitmapExt(Qcow2Struct): >>> self.bitmap_directory = \ >>> [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) >>> for _ in range(self.nb_bitmaps)] >>> + self.fields_dict.update(bitmap_directory=self.bitmap_directory) >>> def dump(self, dump_json=None): >>> super().dump(dump_json) >>> @@ -189,6 +192,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): >>> table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))] >>> self.bitmap_table = Qcow2BitmapTable(raw_table=table, >>> cluster_size=self.cluster_size) >>> + self.fields_dict.update(bitmap_table=self.bitmap_table) >>> def dump(self, dump_json=None): >>> print(f'{"Bitmap name":<25} {self.name}') >>> >> >>
On 16.07.2020 18:40, Vladimir Sementsov-Ogievskiy wrote: > 16.07.2020 18:34, Andrey Shinkevich wrote: >> On 16.07.2020 13:24, Vladimir Sementsov-Ogievskiy wrote: >>> 14.07.2020 00:36, Andrey Shinkevich wrote: >>>> As __dict__ is being extended with class members we do not want to >>>> print, make a light copy of the initial __dict__ and extend the copy >>>> by adding lists we have to print in the JSON output. >>>> >>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >>>> --- >>>> tests/qemu-iotests/qcow2_format.py | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/tests/qemu-iotests/qcow2_format.py >>>> b/tests/qemu-iotests/qcow2_format.py >>>> index e0e14b5..83c3482 100644 >>>> --- a/tests/qemu-iotests/qcow2_format.py >>>> +++ b/tests/qemu-iotests/qcow2_format.py >>>> @@ -109,6 +109,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): >>>> self.__dict__ = dict((field[2], values[i]) >>>> for i, field in enumerate(self.fields)) >>>> + self.fields_dict = self.__dict__.copy() >>> >>> No, I don't like that. Keeping two copies of all the data is bad >>> idea. If you want to select some fields, add a method (dump_json() >>> ?) Which will collect the fields you want in a dict and return that >>> new dict. But don't store object attributes twice. That is what I did in the versions before but it looks clumsy to me. Every single class lists almost all the items of the __dict__ again in the additional method. Andrey >>> >> >> Not really. It makes a light copy that stores only references to the >> desired fields. >> > > I'm not about memory consumption, I'm about the design. Keeping two > representations of same thing is always painful to maintain. > >> >>>> + >>>> def dump(self, dump_json=None): >>>> for f in self.fields: >>>> value = self.__dict__[f[2]] >>>> @@ -144,6 +146,7 @@ class Qcow2BitmapExt(Qcow2Struct): >>>> self.bitmap_directory = \ >>>> [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) >>>> for _ in range(self.nb_bitmaps)] >>>> + self.fields_dict.update(bitmap_directory=self.bitmap_directory) >>>> def dump(self, dump_json=None): >>>> super().dump(dump_json) >>>> @@ -189,6 +192,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): >>>> table = [e[0] for e in struct.iter_unpack('>Q', >>>> fd.read(table_size))] >>>> self.bitmap_table = Qcow2BitmapTable(raw_table=table, >>>> cluster_size=self.cluster_size) >>>> + self.fields_dict.update(bitmap_table=self.bitmap_table) >>>> def dump(self, dump_json=None): >>>> print(f'{"Bitmap name":<25} {self.name}') >>>> >>> >>> > >
16.07.2020 18:52, Andrey Shinkevich wrote: > On 16.07.2020 18:40, Vladimir Sementsov-Ogievskiy wrote: >> 16.07.2020 18:34, Andrey Shinkevich wrote: >>> On 16.07.2020 13:24, Vladimir Sementsov-Ogievskiy wrote: >>>> 14.07.2020 00:36, Andrey Shinkevich wrote: >>>>> As __dict__ is being extended with class members we do not want to >>>>> print, make a light copy of the initial __dict__ and extend the copy >>>>> by adding lists we have to print in the JSON output. >>>>> >>>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >>>>> --- >>>>> tests/qemu-iotests/qcow2_format.py | 4 ++++ >>>>> 1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py >>>>> index e0e14b5..83c3482 100644 >>>>> --- a/tests/qemu-iotests/qcow2_format.py >>>>> +++ b/tests/qemu-iotests/qcow2_format.py >>>>> @@ -109,6 +109,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): >>>>> self.__dict__ = dict((field[2], values[i]) >>>>> for i, field in enumerate(self.fields)) >>>>> + self.fields_dict = self.__dict__.copy() >>>> >>>> No, I don't like that. Keeping two copies of all the data is bad idea. If you want to select some fields, add a method (dump_json() ?) Which will collect the fields you want in a dict and return that new dict. But don't store object attributes twice. > > > That is what I did in the versions before but it looks clumsy to me. Every single class lists almost all the items of the __dict__ again in the additional method. > Most of them should be listed automatically by base class method, which will iterate through .fields > >>>> >>> >>> Not really. It makes a light copy that stores only references to the desired fields. >>> >> >> I'm not about memory consumption, I'm about the design. Keeping two representations of same thing is always painful to maintain. >> >>> >>>>> + >>>>> def dump(self, dump_json=None): >>>>> for f in self.fields: >>>>> value = self.__dict__[f[2]] >>>>> @@ -144,6 +146,7 @@ class Qcow2BitmapExt(Qcow2Struct): >>>>> self.bitmap_directory = \ >>>>> [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) >>>>> for _ in range(self.nb_bitmaps)] >>>>> + self.fields_dict.update(bitmap_directory=self.bitmap_directory) >>>>> def dump(self, dump_json=None): >>>>> super().dump(dump_json) >>>>> @@ -189,6 +192,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): >>>>> table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))] >>>>> self.bitmap_table = Qcow2BitmapTable(raw_table=table, >>>>> cluster_size=self.cluster_size) >>>>> + self.fields_dict.update(bitmap_table=self.bitmap_table) >>>>> def dump(self, dump_json=None): >>>>> print(f'{"Bitmap name":<25} {self.name}') >>>>> >>>> >>>> >> >>
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py index e0e14b5..83c3482 100644 --- a/tests/qemu-iotests/qcow2_format.py +++ b/tests/qemu-iotests/qcow2_format.py @@ -109,6 +109,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta): self.__dict__ = dict((field[2], values[i]) for i, field in enumerate(self.fields)) + self.fields_dict = self.__dict__.copy() + def dump(self, dump_json=None): for f in self.fields: value = self.__dict__[f[2]] @@ -144,6 +146,7 @@ class Qcow2BitmapExt(Qcow2Struct): self.bitmap_directory = \ [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) for _ in range(self.nb_bitmaps)] + self.fields_dict.update(bitmap_directory=self.bitmap_directory) def dump(self, dump_json=None): super().dump(dump_json) @@ -189,6 +192,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct): table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))] self.bitmap_table = Qcow2BitmapTable(raw_table=table, cluster_size=self.cluster_size) + self.fields_dict.update(bitmap_table=self.bitmap_table) def dump(self, dump_json=None): print(f'{"Bitmap name":<25} {self.name}')
As __dict__ is being extended with class members we do not want to print, make a light copy of the initial __dict__ and extend the copy by adding lists we have to print in the JSON output. Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> --- tests/qemu-iotests/qcow2_format.py | 4 ++++ 1 file changed, 4 insertions(+)