Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kvm: add VM Settings for virtual GPU hardware type and memory #5513

Merged
merged 8 commits into from Oct 4, 2021
Merged

kvm: add VM Settings for virtual GPU hardware type and memory #5513

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1c0cd2d
kvm: add MV Settings for virtual GPU hardware type and memory
leolleeooleo Sep 25, 2021
e2d4fd2
fix method createVideoDef argument in test package
leolleeooleo Sep 25, 2021
89d26b8
add available options for KVM virtual GPU hardware VM setting
leolleeooleo Sep 30, 2021
789d68e
fix videoram can be any integer
leolleeooleo Sep 30, 2021
d47f29f
fix use local variables
leolleeooleo Sep 30, 2021
07f30f6
fix videoRam default value
leolleeooleo Sep 30, 2021
4025890
fix _videoRam is 0, it will use default provided by libvirt
leolleeooleo Sep 30, 2021
b8c1b18
re-use the constants at VmDetailConstants
leolleeooleo Oct 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/src/main/java/com/cloud/vm/VmDetailConstants.java
View file
Open in desktop
Expand Up @@ -40,6 +40,10 @@ public interface VmDetailConstants {
String KVM_VNC_PORT = "kvm.vnc.port";
String KVM_VNC_ADDRESS = "kvm.vnc.address";

// KVM specific, custom virtual GPU hardware
String VIDEO_HARDWARE = "video.hardware";
String VIDEO_RAM = "video.ram";

// Mac OSX guest specific (internal)
String SMC_PRESENT = "smc.present";
String FIRMWARE = "firmware";
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
View file
Open in desktop
Expand Up @@ -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"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leolleeooleo
can videoram be any integer value ?
if so, you can use

options.put(VmDetailConstants.VIDEO_RAM, Collections.emptyList());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

cloudstack/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java

Line 1631 in 07f30f6

if (_videoModel != null && !_videoModel.isEmpty() && _videoRam != 0){

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
            }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

if (HypervisorType.VMware.equals(hypervisorType)) {
Expand Down