bhouse-nexthop commented on code in PR #14378:
URL: https://github.com/apache/cloudstack/pull/14378#discussion_r4273923369
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -6839,22 +6837,45 @@ protected void deleteIfExists(String snapshotPath) {
}
}
- public void setInterfaceDefQueueSettings(Map<String, String> details,
Integer cpus, InterfaceDef interfaceDef) {
+ private Integer getMultiqueueFromVmDetails(Map<String, String> details) {
String nicMultiqueueNumber =
details.get(VmDetailConstants.NIC_MULTIQUEUE_NUMBER);
if (nicMultiqueueNumber != null) {
try {
- Integer nicMultiqueueNumberInteger =
Integer.valueOf(nicMultiqueueNumber);
- if (nicMultiqueueNumberInteger ==
InterfaceDef.MULTI_QUEUE_NUMBER_MEANS_CPU_CORES) {
- if (cpus != null) {
- interfaceDef.setMultiQueueNumber(cpus);
- }
- } else {
-
interfaceDef.setMultiQueueNumber(nicMultiqueueNumberInteger);
- }
+ return Integer.valueOf(nicMultiqueueNumber);
} catch (NumberFormatException ex) {
- LOGGER.warn(String.format("VM details %s is not a valid
integer value %s", VmDetailConstants.NIC_MULTIQUEUE_NUMBER,
nicMultiqueueNumber));
+ logger.warn("VM detail [{}] is not a valid integer value {}.",
VmDetailConstants.NIC_MULTIQUEUE_NUMBER, nicMultiqueueNumber);
}
}
+
+ return null;
+ }
+
+ public void setInterfaceDefQueueSettings(Map<String, String> details,
Integer cpus, InterfaceDef interfaceDef, Integer multiqueueForVrs, boolean
isVr) {
+ if (details == null) {
+ details = new HashMap<>();
+ }
+
+ Integer nicMultiqueueNumber = getMultiqueueFromVmDetails(details);
+ if (nicMultiqueueNumber != null) {
+ logger.info("VM detail [{}] is equal to [{}]; therefore, it will
be used as the multiqueue value.", VmDetailConstants.NIC_MULTIQUEUE_NUMBER,
nicMultiqueueNumber);
+ } else if (isVr && multiqueueForVrs != null) {
Review Comment:
Here the VM detail takes priority over the offering value, so a
`nicMultiqueueNumber` set on a VR (e.g. through `extraconfig`/details by an
admin) silently overrides the per-NIC offering values.
The per-NIC value is the more specific one, so I'd let it win for VRs:
```java
Integer nicMultiqueueNumber = isVr && multiqueueForVrs != null
? multiqueueForVrs
: getMultiqueueFromVmDetails(details);
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -6839,22 +6837,45 @@ protected void deleteIfExists(String snapshotPath) {
}
}
- public void setInterfaceDefQueueSettings(Map<String, String> details,
Integer cpus, InterfaceDef interfaceDef) {
+ private Integer getMultiqueueFromVmDetails(Map<String, String> details) {
String nicMultiqueueNumber =
details.get(VmDetailConstants.NIC_MULTIQUEUE_NUMBER);
if (nicMultiqueueNumber != null) {
try {
- Integer nicMultiqueueNumberInteger =
Integer.valueOf(nicMultiqueueNumber);
- if (nicMultiqueueNumberInteger ==
InterfaceDef.MULTI_QUEUE_NUMBER_MEANS_CPU_CORES) {
- if (cpus != null) {
- interfaceDef.setMultiQueueNumber(cpus);
- }
- } else {
-
interfaceDef.setMultiQueueNumber(nicMultiqueueNumberInteger);
- }
+ return Integer.valueOf(nicMultiqueueNumber);
} catch (NumberFormatException ex) {
- LOGGER.warn(String.format("VM details %s is not a valid
integer value %s", VmDetailConstants.NIC_MULTIQUEUE_NUMBER,
nicMultiqueueNumber));
+ logger.warn("VM detail [{}] is not a valid integer value {}.",
VmDetailConstants.NIC_MULTIQUEUE_NUMBER, nicMultiqueueNumber);
}
}
+
+ return null;
+ }
+
+ public void setInterfaceDefQueueSettings(Map<String, String> details,
Integer cpus, InterfaceDef interfaceDef, Integer multiqueueForVrs, boolean
isVr) {
+ if (details == null) {
+ details = new HashMap<>();
+ }
+
+ Integer nicMultiqueueNumber = getMultiqueueFromVmDetails(details);
+ if (nicMultiqueueNumber != null) {
+ logger.info("VM detail [{}] is equal to [{}]; therefore, it will
be used as the multiqueue value.", VmDetailConstants.NIC_MULTIQUEUE_NUMBER,
nicMultiqueueNumber);
+ } else if (isVr && multiqueueForVrs != null) {
Review Comment:
Here the VM detail takes priority over the offering value, so a
`nicMultiqueueNumber` detail on a VR silently overrides the per-NIC offering
values.
The per-NIC value is the more specific one, so I'd let it win for VRs:
```java
Integer nicMultiqueueNumber = isVr && multiqueueForVrs != null
? multiqueueForVrs
: getMultiqueueFromVmDetails(details);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
