On 12/01/2014 01:50 PM, Giuseppe Scrivano wrote:
Pavel Hrdina <phrdina@redhat.com> writes:
The vgamem is a new attribute for QXL. The vram stil exists and its only silently updated to proper value. The configuration with vram=9216 is wrong and should be updated in virt-manager tests.
the issue exists when the type of an existing QXL device is changed. virt-manager re-uses the XML configuration without modifying attributes it doesn't understand (as vgamem in this case) but changing only type='qxl' to type='$NEW_TYPE'.
My doubt here is if an additional attribute should be silently ignored or users of the API should be filtering it out if type != 'qxl'. Another point is that this attribute will be added in future for some other models, then all users must be updated as well to support it, instead of having it only in libvirt.
What do you think?
Regards, Giuseppe
Well, I think that this is not correct usage of the libvirt XML. You should not just change one part of the device configuration. If user of virt-manager change the device type, you should create a new XML according to the new device type and copy all valid values and then drop the original XML configuration. I don't think that just silently remove unsupported configuration is appropriate solution. Even if we will print warning that it's not supported and it will be removed I still don't like it. It's not that the user should filter the "vgamem" out if type != 'qxl' but the user of the XML configuration should have some templates for existing devices and should use for each type different template. With this approach is ease to manage the device type change and also new features introduced by libvirt. You just modify the affected templates to support new features. Pavel