kvm: add VM Settings for virtual GPU hardware type and memory #5513
Conversation
|
@leolleeooleo |
|
@nvazquez |
| @@ -3879,6 +3879,8 @@ private void fillVMOrTemplateDetailOptions(final Map<String, List<String>> optio | |||
|
|
|||
| if (HypervisorType.KVM.equals(hypervisorType)) { | |||
| options.put(VmDetailConstants.ROOT_DISK_CONTROLLER, Arrays.asList("osdefault", "ide", "scsi", "virtio")); | |||
| options.put(VmDetailConstants.VIDEO_HARDWARE, Arrays.asList("cirrus", "vga", "qxl", "virtio")); | |||
| options.put(VmDetailConstants.VIDEO_RAM, Arrays.asList("16384", "32768", "65536")); | |||
There was a problem hiding this comment.
@leolleeooleo
can videoram be any integer value ?
if so, you can use
options.put(VmDetailConstants.VIDEO_RAM, Collections.emptyList());
There was a problem hiding this comment.
@leolleeooleo
I was not asking you to change the values. sorry for confusion.
I just wanted to know if there is any restriction here . for example, the min value, the max value, or it must be power of 2.
There was a problem hiding this comment.
Note that if VIDEO_RAM = 0, VIDEO_HARDWARE will not work.
If not power of 2, libvirt will auto make it to power of 2.
There was a problem hiding this comment.
would it better to change to the following ?
if (_videoModel != null && !_videoModel.isEmpty()) {
videoBuilder.append("<video>\n");
if (_videoRam != 0) {
videoBuilder.append("<model type='" + _videoModel + "' vram='" + _videoRam + "'/>\n");
} else {
videoBuilder.append("<model type='" + _videoModel + "'/>\n");
}
videoBuilder.append("</video>\n");
return videoBuilder.toString();
}
There was a problem hiding this comment.
It is a good idea.
I was worry about it if videoRam is not provided.
I had test it with no error, and here are the document.
https://libvirt.org/formatdomain.html#video-devices
If no value is provided the default is used.
| Map<String, String> details = vmTO.getDetails(); | ||
| if (details != null) { | ||
| if (details.containsKey(VideoDef.VIDEO_MODEL)) { | ||
| _videoHw = details.get(VideoDef.VIDEO_MODEL); |
There was a problem hiding this comment.
@leolleeooleo
_videoHw and _videoRam are global variables. I think local variables should be used here.
There was a problem hiding this comment.
I copy from
The value _videoHw and _videoRam are load from /etc/cloudstack/agent/agent.properties
If details value exist will override the value.
Do you mean:
protected VideoDef createVideoDef(VirtualMachineTO vmTO) {
Map<String, String> details = vmTO.getDetails();
if (details != null) {
if (details.containsKey(VideoDef.VIDEO_MODEL)) {
String videoHw = details.get(VideoDef.VIDEO_MODEL);
int videoRam = 16384;
if (details.containsKey(VideoDef.VIDEO_RAM)) {
String value = details.get(VideoDef.VIDEO_RAM);
videoRam = NumbersUtil.parseInt(value, videoRam);
}
return new VideoDef(videoHw, videoRam);
}
}
return new VideoDef(_videoHw, _videoRam);
}
There was a problem hiding this comment.
@leolleeooleo exactly. we should not change the global variables.
you can use
String videoHw = _videoHw;
int videoRam = _videoRam;
if (details != null) {
...... // update videoHw and videoRam
}
return new VideoDef(videoHw, videoRam);
|
@blueorangutan package |
|
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
| } | ||
| if (details.containsKey(VideoDef.VIDEO_RAM)) { | ||
| String value = details.get(VideoDef.VIDEO_RAM); | ||
| _videoRam = NumbersUtil.parseInt(value, 0); | ||
| videoRam = NumbersUtil.parseInt(value, 0); |
There was a problem hiding this comment.
@leolleeooleo
better to use
videoRam = NumbersUtil.parseInt(value, videoRam);
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1456 |
|
@blueorangutan package |
|
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1458 |
|
@blueorangutan test |
|
@weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian Build Failed (tid-2259) |
There was a problem hiding this comment.
manually tested ok.
default
<video>
<model type='cirrus' vram='16384' heads='1' primary='yes'/>
<alias name='video0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</video>
video.hardware=qxl
<video>
<model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
<alias name='video0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</video>
video.hardware=qxl, video.ram=32000
<video>
<model type='qxl' ram='65536' vram='32768' vgamem='16384' heads='1' primary='yes'/>
<alias name='video0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</video>
|
@blueorangutan package |
|
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1462 |
|
@blueorangutan test |
|
@weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2263)
|
|
@blueorangutan package |
|
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1472 |
| String videoHw = _videoHw; | ||
| int videoRam = _videoRam; | ||
| if (details != null) { | ||
| if (details.containsKey(VideoDef.VIDEO_MODEL)) { |
There was a problem hiding this comment.
| if (details.containsKey(VideoDef.VIDEO_MODEL)) { | |
| if (details.containsKey(VmDetailConstants.VIDEO_HARDWARE)) { |
can use the same constants defined in VmDetailConstants.
...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
Outdated
Show resolved
Hide resolved
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@leolleeooleo left few comments for minor code improvements, rest of the code LGTM.
leolleeooleo
force-pushed
the
add-vm-vga-type-setting
branch
from
705c492
to
b8c1b18
Compare
|
@blueorangutan package |
|
@leolleeooleo a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1478 |
|
@blueorangutan test |
|
@weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2282)
|
|
@blueorangutan test |
|
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2287)
|
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2301)
|
leolleeooleo
changed the title
Description
Add VM Settings for virtual GPU hardware type and memory.
The default virtual GPU hardware model type is cirrus.
KVM libvirt are support cirrus, vga, qxl and virtio.
Set the value in VM setting to override /etc/cloudstack/agent/agent.properties
video.hardware=qxl (override vm.video.hardware)
video.ram=32768 (override vm.video.ram)
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
I had test windows 10 and ubuntu 20.04 on CloudStack 4.15.2
In my test set "video.hardware=qxl, video.ram=32768" on windows 10 can get better performance.
And set "video.hardware=virtio, video.ram=32768" on ubuntu 20.04 can get better performance.
Windows driver "Stable virtio-win ISO" is from here.