Discussion:
Change in ovirt-engine[master]: core: Fix backend allow 'cpu pinning' with no host
omasad-H+wXaHxf7aLQT0dZR+
2013-04-17 08:35:05 UTC
Permalink
ofri masad has uploaded a new change for review.

Change subject: core: Fix backend allow 'cpu pinning' with no host
......................................................................

core: Fix backend allow 'cpu pinning' with no host

When working via API the user could add a VM with no defined host (start
from any host in cluster) and set the 'cpu pinning' string. This should
not be permitted.

Fixed the validity checked in the AddVmCommand and UpdateVmCommand.
Added a new error message.

Change-Id: Iff00debc039a5476c19966dd91425566a3562e02
Bug-Url: https://bugzilla.redhat.com/928689
Signed-off-by: Ofri Masad <omasad-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java
M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
6 files changed, 14 insertions(+), 0 deletions(-)


git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/71/13971/1

diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
index b1cbde1..b6fb526 100644
--- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
+++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
@@ -340,6 +340,11 @@
return failCanDoAction(VdcBllMessages.VM_HOSTCPU_MUST_BE_PINNED_TO_HOST);
}

+ if (StringUtils.isNotEmpty(getParameters().getVm().getCpuPinning())
+ && getParameters().getVmStaticData().getDedicatedVmForVds() == null) {
+ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST);
+ }
+
return returnValue && checkCpuSockets();
}

diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
index ff74334..fc6a016 100644
--- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
+++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
@@ -278,6 +278,11 @@
return false;
}

+ if (StringUtils.isNotEmpty(vmFromParams.getCpuPinning())
+ && getParameters().getVmStaticData().getDedicatedVmForVds() == null) {
+ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST);
+ }
+
// check cpuPinning
if (!isCpuPinningValid(vmFromParams.getCpuPinning(), vmFromParams.getStaticData())) {
return false;
diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java
index 52652c3..4a0939a 100644
--- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java
+++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java
@@ -554,6 +554,7 @@
VDS_CANNOT_MAINTENANCE_IT_INCLUDES_NON_MIGRATABLE_VM,
ACTION_TYPE_FAILED_VM_CANNOT_BE_HIGHLY_AVAILABLE_AND_PINNED_TO_HOST,
ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_AND_MIGRATABLE,
+ ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST,
VM_PINNED_TO_HOST_CANNOT_RUN_ON_THE_DEFAULT_VDS,
HOST_NAME_NOT_AVAILABLE,
VM_HOSTCPU_MUST_BE_PINNED_TO_HOST,
diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
index b2c227d..fcc57a2 100644
--- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -838,6 +838,7 @@
ERROR_GET_STORAGE_DOMAIN_LIST=Cannot get Storage Domains list.
VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME=Cannot remove gluster server. Server having Gluster volume(s).
ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_AND_MIGRATABLE=Migratable VM's cannot be pinned to CPU.
+ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST=Cannot set host CPU pinning when host is not selected
ACTION_TYPE_FAILED_NETWORK_INTERFACE_MAC_INVALID=Cannot ${action} ${type}. The Network Interface ${IfaceName} has an invalid MAC address ${MacAddress}. MAC address must be in format "HH:HH:HH:HH:HH:HH" where H is a hexadecimal character (either a digit or A-F, case is insignificant).
MIGRATE_PAUSED_VM_IS_UNSUPPORTED=Migrating a VM in paused status is unsupported.
VM_INTERFACE_NOT_EXIST=Cannot ${action} ${type}. The VM Network Interface does not exist.
diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
index 6f867e7..669c751 100644
--- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
+++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -155,6 +155,7 @@
ACTION_TYPE_FAILED_VM_NOT_EXIST=Cannot ${action} ${type}. VM doesn't exist.
ACTION_TYPE_FAILED_VM_CANNOT_BE_HIGHLY_AVAILABLE_AND_PINNED_TO_HOST=Cannot ${action} ${type}. A highly available VM cannot be pinned to a specific Host
ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_AND_MIGRATABLE=Migratable VM's cannot be pinned to CPU.
+ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST=Cannot set host CPU pinning when host is not selected
ACTION_TYPE_FAILED_VM_GUID_ALREADY_EXIST=Cannot ${action} ${type}. VM with the same identifier already exists.
ACTION_TYPE_FAILED_VM_ATTACHED_TO_POOL=Cannot ${action} ${type}. VM is attached to a VM-Pool.
ACTION_TYPE_FAILED_NO_AVAILABLE_POOL_VMS=Cannot ${action} ${type}. There are no available VMs in the VM-Pool.
diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
index f0971e4..91f69b0 100644
--- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
+++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -772,6 +772,7 @@
VMPAYLOAD_FLOPPY_EXCEEDED=Payload Floppy cannot be used when using an additional Floppy
ERROR_GET_STORAGE_DOMAIN_LIST=Cannot get Storage Domains list.
ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_AND_MIGRATABLE=Migratable VM's cannot be pinned to CPU.
+ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST=Cannot set host CPU pinning when host is not selected
ACTION_TYPE_FAILED_NETWORK_INTERFACE_MAC_INVALID=Cannot ${action} ${type}. The Network Interface ${IfaceName} has an invalid MAC address ${MacAddress}. MAC address must be in format "HH:HH:HH:HH:HH:HH" where H is a hexadecimal character (either a digit or A-F, case is insignificant).

VDS_GROUP_ENABLING_BOTH_VIRT_AND_GLUSTER_SERVICES_NOT_ALLOWED=Cannot ${action} ${type}. Enabling both Virt and Gluster services is not allowed.
omasad-H+wXaHxf7aLQT0dZR+
2013-04-17 13:09:32 UTC
Permalink
ofri masad has posted comments on this change.

Change subject: core: Fix backend allow 'cpu pinning' with no host
......................................................................


Patch Set 2: Verified
lhornyak-H+wXaHxf7aLQT0dZR+
2013-04-17 13:21:45 UTC
Permalink
Laszlo Hornyak has posted comments on this change.

Change subject: core: Fix backend allow 'cpu pinning' with no host
......................................................................


Patch Set 2: Verified; Looks good to me, approved

verified, thx!
lhornyak-H+wXaHxf7aLQT0dZR+
2013-04-17 13:23:23 UTC
Permalink
Laszlo Hornyak has submitted this change and it was merged.

Change subject: core: Fix backend allow 'cpu pinning' with no host
......................................................................


core: Fix backend allow 'cpu pinning' with no host

When working via API the user could add a VM with no defined host (start
from any host in cluster) and set the 'cpu pinning' string. This should
not be permitted.

Fixed the validity checked in the AddVmCommand and UpdateVmCommand.
Added a new error message.

Change-Id: Iff00debc039a5476c19966dd91425566a3562e02
Bug-Url: https://bugzilla.redhat.com/928689
Signed-off-by: Ofri Masad <omasad-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java
M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java
M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
5 files changed, 6 insertions(+), 0 deletions(-)

Approvals:
Laszlo Hornyak: Verified; Looks good to me, approved
lhornyak-H+wXaHxf7aLQT0dZR+
2013-04-17 13:23:22 UTC
Permalink
Laszlo Hornyak has posted comments on this change.

Change subject: core: Fix backend allow 'cpu pinning' with no host
......................................................................


Patch Set 3: Verified; Looks good to me, approved

Loading...