[libvirt] [PATCH v3 5/6] qemu-command: use vram attribute for all video devices

[Peter Krempa]

On 11/20/14 20:21, Pavel Hrdina wrote:
> So far we hadn't any option to set video memory size for qemu video

hadn't had or didn't have

> devices. There were only vram (ram for QXL) attribute but it was valid

There was  only the

> only for QXL video device.

for the

> 
> To provide this feature to users qemu has dedicated device attribute

qemu has a

> called 'vgamem_mb' to set the video memory size. We will use the 'vram'
> attribute also for setting video memory size for other qemu video

attribute for

> devices.
> 
> Only for cirrus device we will ignore the vram value because it has
> hardcoded video size in QEMU.

For the cirrus device

> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---

...

> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0c77b57..ac36567 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5031,6 +5031,19 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
>              /* QEMU accepts bytes for vram_size. */
>              virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024);
>          }
> +    } else if (video->vram &&
> +        ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
> +          virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
> +         (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
> +          virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) {
> +
> +        if (video->vram % 1024) {

With the power of two rounding function this should effectively boil
down to the following check:
 if (video->vram < 1024) { ...

Is that intended? In that case shouldn't we change the condition and
error message to make it more obvious?


> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           "%s", _("value for 'vram' must be multiple of 1024"));
> +            goto error;
> +        }
> +
> +        virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024);
>      }
>  
>      if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
> @@ -9360,6 +9373,25 @@ qemuBuildCommandLine(virConnectPtr conn,
>                                                 dev, vram * 1024);
>                      }
>                  }
> +
> +                if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
> +                    def->videos[0]->vram &&
> +                    ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA &&
> +                      virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
> +                     (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
> +                      virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) {
> +                    unsigned int vram = def->videos[0]->vram;
> +
> +                    if (vram % 1024) {

Same here.

> +                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                       "%s", _("value for 'vgamem' must be multiple of 1024"));
> +                        goto error;
> +                    }
> +
> +                    virCommandAddArg(cmd, "-global");
> +                    virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u",
> +                                           dev, vram / 1024);
> +                }
>              }
>  
>              if (def->nvideos > 1) {

ACK with the tweak if you agree with my analysys.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141124/d036bea2/attachment-0001.sig>