Recommend setup_attrs() method for creating/updating a libvirt vm device object

[dzhengfy]

Problem
As we know, libvirt automation test scripts usually prepare a special guest vm with different devices for testing. To make that, we created utility functions in avocado-vt/tp-libvirt, like
In avocado-vt/virrtest/utils_test/libvirt.py, we have below functions.

_create_channel_xml(), create_controller_xml(), create_disk_xml(), create_hostdev_xml(), create_net_xml(), create_rng_xml(), create_tpm_dev(), create_vsock_xml(), …_

We almost define every function for creating every different device. But most of their codes are similar like below:

1. Create a device object
2. Get the attribute and value one by one from parameters and assign it to the above device object. If there are 20 attributes, we have 20 if clauses to assign values.
3. Return the object or its xml

And when updating a device xml, we have similar problem like below sample function :
modify_vm_iface(), whose logic is below:

1. Get or create an interface object
2. Get the attribute and value one by one from parameters and assign it to the above device object one by one. So if there are 20 attributes, we have 20 if clauses to assign values.
3. Return the object or its xml

When we need to update devices in other types, we have to define different functions for them.

So is it necessary to define so many functions for different devices with the same code logic? I do not think so. I think the old way has too many duplicate efforts.

Suggestion
I recommend we use the setup_attrs() method to solve the above problem at one stop.

About setup_attrs()
setup_attrs() function was implemented in PR Add function to setup attrs of an xml object #3168
and its paired function fetch_attrs() in Add function to fetch attrs of an xml object #3212
by haizhao (github: chloerh).

Usage for setup_attrs()
Take memory device as an example for the usage of setup_attrs() as below:
We define the below dict in the cfg file of tp-libvirt. The fields should align with __slots__ and __init__() in the device class’s definition, see avocado-vt-/virttest/libvirt_xml/devices/memory.py.

dimm_device_attrs = {
   'address': {'type_name': 'dimm', 'attrs': {'type': 'dimm', 'slot': '2', 'base': '0x120000000'}},
   'alias': {'name': 'dimm2'},
   'mem_access': 'private',
   'mem_discard': 'yes',
   'mem_model': 'dimm',
   'source': {
       'pagesize': 4096,
       'pagesize_unit': 'KiB',
       'nodemask': '1-3',
       'path': '/tmp/nvdimm'
   },
   'target': {
       'size': 524288, 'size_unit': 'KiB', 'node': 0,
       'label': {'size': 128, 'size_unit': 'KiB'}
   },
   'uuid': '9066901e-c90a-46ad-8b55-c18868cf92ae',
}

Then we use below codes in python file which can easily create a memory device and get the memory device’s full attributes :

dimm_device = memory.Memory()
dimm_device_attrs = eval(params.get(‘dimm_device_attrs’, ‘{}’))
dimm_device.setup_attrs(**dimm_device_attrs)
dimm_attrs = dimm_device.fetch_attrs()

There are also some unit tests for devices of other types under avocado-vt/virttest/unittests/libvirt_xml/ which were provided by chloerh.

Benefits
With setup_attrs(), we do not need to define those create_xxx() functions for different devices, while tp-libvirt users could easily use above 4 lines to meet their needs or they can use create_vm_device_by_type() in virttest/utils_libvirt/libvirt_vmxml.py to meet their needs. And for updating a device, they can use modify_vm_device() in virttest/utils_libvirt/libvirt_vmxml.py. Both above two functions are based on setup_attrs().
In a word, we use two functions to provide the same functionalities which more than ten functions do.

Requirement
If you like this way, now there is a requirement that when we define a new device class in avocado-vt, let’s make it support setup_attrs() usage and provide your test result using setup_attrs() in your PR. This is helpful for other users to use this way in their tp-libvirt cases. There is an example to support it tpm.py: optimize pcrbank part to support setup_attr #3404

Hi guys, what do you think about this? And if you would like to use it, what's your opinion for providing the test result using it when defining a new device xml class?

[dzhengfy]

@chunfuwen @Yingshun @chloerh @kylazhang @iccaszhulili @yafu-1 @yanqzhan @meinaLi @yalzhang @smitterl @cliping @nanli1 @vasekhodina

[Yingshun]

@dzhengfy @chloerh I like it very much, it's very convenient to set xml in the code and we don't need to maintain several create_xml functions as you mentioned.
Regarding the test result for newly involved device class, I wonder if it's possible to ask author to provide unittests script which has setup_attrs/fetch_attrs.

[meinaLi]

@dzhengfy I think this method will be very effective and I also really like it. Here I have some questions for it:

1. From Add function to fetch attrs of an xml object #3212 we can see some new device xml: test_interface_xml.py, test_controller_xml.py and test_memory_xml.py. Does this mean that we need to add new py files to the xml of each feature, like disk_xml, os_xml and so on? Or is the script here just for validating the PR？
2. Do our previously existing utility functions need to be changed? Or we can only use it when defining a new device xml class?
3. After using setup_attrs(), when we have a new attribute in device xml, we don't need to update the avocado-vt, but only update the xxx_attrs in tp-libvirt cfg file. Is my understanding correct？

Thank you very much for your contribution on this.

[dzhengfy]

@meinaLi

@dzhengfy I think this method will be very effective and I also really like it. Here I have some questions for it:

1. From Add function to fetch attrs of an xml object #3212 we can see some new device xml: test_interface_xml.py, test_controller_xml.py and test_memory_xml.py. Does this mean that we need to add new py files to the xml of each feature, like disk_xml, os_xml and so on? Or is the script here just for validating the PR？

[dzheng] No. For example, virttest/unittests/libvirt_xml/device_xml/test_controller_xml.py, it is just an unit test case which is used to verify the function and give users reference for the usage.

2. Do our previously existing utility functions need to be changed? Or we can only use it when defining a new device xml class?

[dzheng] Existing utility functions need not to be changed as they are used by most existing test scripts. But in new test scripts if we use setup_attrs() way, I believe those utility functions are not necessary to use any more.
When we define a new device xml class, let's support this setup_attrs() usage, which is one purpose in this proposal.
Some existing device xml class's definitions do not support setup_attrs() usage, @chloerh is always actively to help on improvement.

3. After using setup_attrs(), when we have a new attribute in device xml, we don't need to update the avocado-vt, but only update the xxx_attrs in tp-libvirt cfg file. Is my understanding correct？

[dzheng] 1. update xxx_attrs in tp-libvirt cfg file 2. add this attribute to the device xml class definition in avocado-vt
We need to do both 1 and 2. 2 is necessary no matter with setup_attrs() or not.

Thank you very much for your contribution on this.

[fangge1212]

I totally support this, I found setup_attrs() is very convenient in my practice.

[yalzhang]

It's very concise. I totally support it, too.

[iccaszhulili]

I will use it next time when creating a device xml. Thanks very much for providing such a great solution.

[nanli1]

I love it, I used this for many times. It's very convenient to edit or post one object by dict .

[cliping]

It is very convenient. I like it, too.

[yafu-1]

I love it too, it's very convenient and concise. Thanks for the clever idea.