qemu_vm: Avoid overwrite existing virtio properties

[PaulYuuu]

1. "virtio_dev_xxx" are global settings for virtio devices, but they will always be set if we define them, even if the device has set these properties. The property setting by the device should be higher than the global one.
2. virtio_dev_xxx will set properties to whole virtio devices, but some devices cannot work well that property such as "vhost-user-fs-pci" + "iommu_platform", so the new parameter virtio_dev_filter can filter out some devices which we want to set those properties.

ID: 2111781 Signed-off-by: Yihuang Yu [email protected]

[PaulYuuu]

Hello @vivianQizhu @yanan-fu @yiqianwei, would you help review? Thanks in advance.

[PaulYuuu]

Hello @vivianQizhu @yanan-fu @yiqianwei, would you help review? Thanks in advance.

Hey, any comments?

[yanan-fu]

With this implementation, there are two different sets of logic that can change the same device configuration. I think it's going to be a little confused. But yes, the existing logic is for all virtio device, does not allow device specific setting.
Could you please share more about the requirements ? Why does it needs different setting for the device you want to change with device.params. Thanks!

[PaulYuuu]

With this implementation, there are two different sets of logic that can change the same device configuration. I think it's going to be a little confused. But yes, the existing logic is for all virtio device, does not allow device specific setting. Could you please share more about the requirements ? Why does it needs different setting for the device you want to change with device.params. Thanks!

OK, in one case, most virtio devices want to enable iommu_platform, and only one or a few devices cannot enable it because of the limitation, such as virtio-iommu, virtio-balloon. Actually the iommu_platform should not display in the device help output, but developers think this usage can avoid in the libvirt layer and it's a wrong usage, so not plan to correct it. If we use xxx_extra_params, we need to set each device, but virtio_dev_xxx can set all virtio devices, as long as this parameter appears in the help output. So with this change, the virtio parameters can be controlled relatively flexibly, even if it's a workaround.

[yanan-fu]

OK, in one case, most virtio devices want to enable iommu_platform, and only one or a few devices cannot enable it because of the limitation, such as virtio-iommu, virtio-balloon. Actually the iommu_platform should not display in the device help output, but developers think this usage can avoid in the libvirt layer and it's a wrong usage, so not plan to correct it.

Okay, If those virtio devices which show iommu_platform in help info but does not support it actually, how do you think about defining blacklist for them ?

[PaulYuuu]

Okay, If those virtio devices which show iommu_platform in help info but does not support it actually, how do you think about defining blacklist for them ?

I thought about it before, but it's hard to maintain, I mean we don't know how many virtio devices will be introduced in the future, and what the support status of these properties of them.

[yanan-fu]

Okay, If those virtio devices which show iommu_platform in help info but does not support it actually, how do you think about defining blacklist for them ?

I thought about it before, but it's hard to maintain, I mean we don't know how many virtio devices will be introduced in the future, and what the support status of these properties of them.

I think it is same for test case level to handle xxx_extra_params, when new virtio device be introduced and have the same problem, case need be updated accordingly IMO, and may be not only one cases.

[PaulYuuu]

I think it is same for test case level to handle xxx_extra_params, when new virtio device be introduced and have the same problem, case need be updated accordingly IMO, and may be not only one cases.

In case users use compiled qemu themselves, there has a lot of virtio devices, we hard to maintain them in a white/black list. This just provides a flexible method to control virtio_dev_xxx parameters, this change will keep the previous usage and support _extra_params as well.

However, not many people/feature owners will set iommu-related parameters as a global setting.

[PaulYuuu]

@yanan-fu updated according to our offline discussion, I think these are the minimal changes based on the conversation.

[PaulYuuu]

Test:

>>> import re
>>> devices = [{'dev': 'virtio-net-pci'}, {'dev': 'vhost-vsock'}, {'dev': 'pxb-pcie'}, {'dev': 'virtio-balloon-pci'}, {'dev': 'virtio-iommu-pci'}, {'dev': 'vhost-user-fs-pci'}]
>>> virtio_dev_filter = '^(?:(?:virtio-)|(?:vhost-))(?!(?:balloon)|(?:iommu))'
>>> list(filter(lambda x: re.search(virtio_dev_filter, x.get('dev')), devices))
[{'dev': 'virtio-net-pci'}, {'dev': 'vhost-vsock'}, {'dev': 'vhost-user-fs-pci'}]
>>> virtio_dev_filter = '^(?:(?:virtio-)|(?:vhost-))(?!(?:user)|(?:net))'
>>> list(filter(lambda x: re.search(virtio_dev_filter, x.get('dev')), devices))
[{'dev': 'vhost-vsock'}, {'dev': 'virtio-balloon-pci'}, {'dev': 'virtio-iommu-pci'}]
>>> virtio_dev_filter = '^(?:(?:virtio-)|(?:vhost-))(?:net)'
>>> list(filter(lambda x: re.search(virtio_dev_filter, x.get('dev')), devices))
[{'dev': 'virtio-net-pci'}]

[PaulYuuu]

Re-push to fix the CI failure.

[PaulYuuu]

Hello @vivianQizhu, could you also help to review?