On 2014/7/24 17:37, Pavel Hrdina wrote:
>> ---
>> src/conf/domain_conf.c | 17 ++++++++++++-----
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
[...]
>> @@ -9474,9 +9471,19 @@ virDomainVideoDefParseXML(xmlNodePtr node,
>> }
>>
>> if (vram) {
>> + /* For type of kvm, vram attribute seems to be invalid
>> + * for VIR_DOMAIN_VIDEO_TYPE_VMVGA. Shall we also need
>> + * to add judge here? Will it affect other drivers? */
>> + if (def->type == VIR_DOMAIN_VIDEO_TYPE_VGA ||
>> + def->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS) {
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("vram attribute is not supported "
>> + "for type of vga and cirrus"));
>> + goto error;
>> + }
>> if (virStrToLong_ui(vram, NULL, 10, &def->vram) < 0) {
>> virReportError(VIR_ERR_XML_ERROR,
>> - _("cannot parse video ram '%s'"), vram);
>> + _("cannot parse video vram '%s'"), vram);
>> goto error;
>> }
>> } else {
>>
>
> We cannot disable use of vram for VGA and CIRRUS because it would be
> a regression and all existing domains having this attribute in xml
> would not be loaded on libvirtd start. We have to accept the fact that
> the vram attribute could be defined for all current video types and
> for some of them it will be just ignored.
>
> So don't return error if the vram is set for VGA and CIRRUS.
> The only possibility is to silently remove it for qemu in
> qemuDomainDevicePostParse function, but that will also requires
> updating all qemuxml2argv and qemuxml2xml tests.
>
Thanks for your review.
You are right. In libvirt upgrade scenario, this patch will make a regression.
In this case , VM should be started successfully without vram in qemu
command line.
> Pavel
>
>
> .
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list