[libvirt] [PATCH V2 2/6] qemu: Introduce vgamem attribute for video model

Wang Rui moon.wangrui at huawei.com
Fri Jul 25 09:02:48 UTC 2014


On 2014/7/24 17:40, Michal Privoznik wrote:

>> ---
>>   src/conf/domain_conf.c   | 48 ++++++++++++++++++++++++++++++-
>>   src/conf/domain_conf.h   |  3 +-
>>   src/libvirt_private.syms |  1 +
>>   src/qemu/qemu_command.c  | 75 ++++++++++++++++++++++++++++++++----------------
>>   4 files changed, 101 insertions(+), 26 deletions(-)
> 
> missing docs and RNG schema adjustment. And I'd introduce a new XML to test too.
> 
Thank you for your review.
docs and RNG schema are in patch 6/6. Do you mean that docs and source code
should be merged into one patch or I missed some other docs?

>>
>> +    if (vgamem) {
>> +        if (def->type != VIR_DOMAIN_VIDEO_TYPE_VGA &&
>> +            def->type != VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
>> +            def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
>> +            virReportError(VIR_ERR_XML_ERROR, "%s",
>> +                           _("vgamem attribute only supported "
>> +                             "for type of vga, vmvga and qxl"));
>> +            goto error;
>> +        }
> 
> Spaces at EOL
> 

Sorry. "make syntax-check" shows me the nits.
>> +        if (virStrToLong_ui(vgamem, NULL, 10, &def->vgamem) < 0) {
>> +            virReportError(VIR_ERR_XML_ERROR,
>> +                           _("cannot parse video vgamem '%s'"), vgamem);
>> +            goto error;
>> +        }
>> +    } else {
>> +        def->vgamem = virDomainVideoDefaultVgamem(def->type);
>> +    }
>> +
> 
> And again.
> 
> 
> But I'm wondering how's vgamem different to vram. If it covers the same attribute but for different models, I'd vote for keeping already existing attribute name.
> 

Qxl model has attribute vram_size/vram_size_mb and vgamem_mb in QEMU.
Vga model has attribute vgamem_mb in QEMU but not vram_size/vram_size_mb.
So I use a new attribute vgamem in libvirt to make variable/attribute
name be consistent with QEMU's.

IIUC, attributes in libvirt and qemu for different models are shown as below.

Before:
model       libvirt-attribute(xml)      qemu-attribute

qxl         vram                        vram_size
qxl         no attribute                vgamem_mb
vga         vram                        libvirt passes no attribute to qemu; and qemu has no attribute named like vram*
vga         no attribute                vgamem_mb

after:
model       libvirt-attribute(xml)      qemu-attribute

qxl         vram                        vram_size
qxl         vgamem                      vgamem_mb
vga         vram                        libvirt passes no attribute to qemu; and qemu has no attribute named like vram*
vga         vgamem                      vgamem_mb

And so do other models.(I hope my expression is clear.)

I think it's less confusion to introduce new attribute vgamem.

Gerd Hoffmann's suggestion is almost the same.
https://www.redhat.com/archives/libvir-list/2014-June/msg00569.html




More information about the libvir-list mailing list