Re: [PATCH v10 09/10] qcow2_format.py: collect fields to dump in JSON format

[Andrey Shinkevich]

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}')