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

2020-07-16 Thread Vladimir Sementsov-Ogievskiy

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










--
Best regards,
Vladimir



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

2020-07-16 Thread 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 
---
  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}')











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

2020-07-16 Thread Vladimir Sementsov-Ogievskiy

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







--
Best regards,
Vladimir



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

2020-07-16 Thread Andrey Shinkevich

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








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

2020-07-16 Thread Vladimir Sementsov-Ogievskiy

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




--
Best regards,
Vladimir



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

2020-07-13 Thread Andrey Shinkevich
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 
---
 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()
+
 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}')
-- 
1.8.3.1