[PATCH 0/1] qemuDomainChangeNet: check virtio options for non-virtio models

Hi, in our project, we create all of our domain interfaces regardless of model with a <driver><host csum='off'/></driver>. The documentation is not clear as to which models support offloading or not, and the domain validates. In libvirtd, since <driver> is present, this causes each network device to have its virDomainVirtioOptions *virtio to be non-NULL, with all members ABSENT, which is also deemed valid. However, clients receive the interface elements without the driver. When the modified value is sent back via updateDeviceFlags, the candidate's virtio attribute is thus NULL. The validation in qemuDomainChangeNet requires both structs to be equal or both NULL, which is violated by such a request. This can be seen also with how virsh domif-setlink operates: virsh # domiflist 5 Interface Type Source Model MAC -------------------------------------------------------- - udp - e1000 52:54:00:1c:10:42 - udp - e1000 52:54:00:09:29:9c - udp - e1000 52:54:00:1d:a6:8d - udp - e1000 52:54:00:07:4a:83 52:54:00:1c:10:42 up virsh # domif-setlink 5 52:54:00:1c:10:42 down error: Failed to update interface link state error: Operation not supported: cannot modify virtio network device driver options A workaround is to add an empty <driver> in the update request, which works until libvirt is restarted. The driver element on a non-virtio interface is not present when the domain is reloaded, thus the pointer is NULL in olddev, causing the inverse issue and same error. Our current workaround is to retry with the empty driver element only if the first call fails, but we think libvirt should handle this struct more consistently. This patch expands the check to the non-virtio case, where the olddev is assumed to be valid, and newdev's virtio options are validated to be NULL or all-ABSENT. Since the driver element is normally preserved for virtio models, the stricter equality condition is left unchanged in that case. I did not investigate further if the discrepancy between NULL and all-ABSENT values for virDomainVirtioOptions can cause a problem elsewhere, or in other device kinds. Though I do wonder if the options handling should be improved. One possibility is that non-virtio devices keep the value NULL, e.g. after validating that the driver element does not contain any of the options, whereas virtio devices always create the struct even with no driver element present on input. Another option is that NULL pointer is treated the same as all-ABSENT struct, e.g. in virDomainCheckVirtioOptionsAreAbsent and virDomainVirtioOptionsCheckABIStability; the former does that while the latter does not. These functions would also need to become callable from the qemu driver. Miroslav Los (1): qemuDomainChangeNet: check virtio options for non-virtio models src/qemu/qemu_hotplug.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) -- 2.25.1