Daniel Erez has uploaded a new change for review.
Change subject: core: allow read-only for VirtIO-SCSI DirectLUN disks
......................................................................
core: allow read-only for VirtIO-SCSI DirectLUN disks
In order to support read-only property for VirtIO-SCSI
DirectLUN disks, the device type for such disks should
be 'disk' (SCSI emulation) instead of 'lun' (SCSI pass-through).
I.e. representation in the domain xml sent to libvirt should
look like [1] instead of [2].
Note that this limitation (enabling pass-through for read-only
VirtIO-SCSI DirectLUN disks) could be removed once 'Support
read only VirtIO-SCSI direct LUNs in QEMU' bug [3] is resolved.
To provide full flexibility, added a new check-box for enabling
pass-through (see screenshot [4]). I.e. the user can choose between
emulated and pass-through ('disk' vs. 'lun'); read-only is
currently supported only for emulated.
[1] <disk type='block' device='disk'>...</disk>
[2] <disk type='block' device='lun'>...</disk>
[3] https://bugzilla.redhat.com/1110396
[4] screenshot: http://i.imgur.com/5mS3xSk.jpg
Change-Id: Ic317722be9ef8978b50352c12bc06ea8c906dcd0
Bug-Url: https://bugzilla.redhat.com/1118847
Signed-off-by: Daniel Erez <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BaseDisk.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
M
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
M
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
M
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.java
M
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.ui.xml
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/AbstractDiskModel.java
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditDiskModel.java
M
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
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
A packaging/dbscripts/upgrade/03_04_0800_update_sgio_for_readonly_disks.sql
21 files changed, 168 insertions(+), 59 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/05/30905/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
index beae404..ef115a1 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
@@ -157,10 +157,6 @@
return false;
}
- if
(!validate(diskValidator.isReadOnlyPropertyCompatibleWithLunInterface())) {
- return false;
- }
-
return true;
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
index 0de5638..e13bcd9 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
@@ -131,11 +131,6 @@
return false;
}
- if (disk.getDiskStorageType() == DiskStorageType.LUN &&
-
!validate(diskValidator.isReadOnlyPropertyCompatibleWithLunInterface())) {
- return false;
- }
-
if (!isVmNotInPreviewSnapshot()) {
return false;
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
index 8564b0f..8181a38 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
@@ -159,7 +159,6 @@
return validateCanUpdateShareable() && validateCanUpdateReadOnly() &&
validate(diskValidator.isVirtIoScsiValid(getVm())) &&
validate(diskValidator.isReadOnlyPropertyCompatibleWithInterface()) &&
- !(getNewDisk().getDiskStorageType() == DiskStorageType.LUN &&
!validate(diskValidator.isReadOnlyPropertyCompatibleWithLunInterface())) &&
(getOldDisk().getDiskInterface() ==
getNewDisk().getDiskInterface()
|| validate(diskValidator.isDiskInterfaceSupported(getVm())));
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
index 335774d..0cc2c65 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
@@ -100,17 +100,16 @@
}
public ValidationResult isReadOnlyPropertyCompatibleWithInterface() {
- if (Boolean.TRUE.equals(disk.getReadOnly()) && disk.getDiskInterface()
== DiskInterface.IDE) {
- return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR);
- }
+ if (Boolean.TRUE.equals(disk.getReadOnly())) {
+ DiskInterface diskInterface = disk.getDiskInterface();
- return ValidationResult.VALID;
- }
+ if (diskInterface == DiskInterface.IDE) {
+ return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR);
+ }
- public ValidationResult isReadOnlyPropertyCompatibleWithLunInterface() {
- if (Boolean.TRUE.equals(disk.getReadOnly()) && disk.getDiskInterface()
== DiskInterface.VirtIO_SCSI) {
- return new ValidationResult(
-
VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR);
+ if (disk.isScsiPassthrough()) {
+ return new
ValidationResult(VdcBllMessages.SCSI_PASSTHROUGH_IS_NOT_SUPPORTED_FOR_READ_ONLY_DISK);
+ }
}
return ValidationResult.VALID;
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
index 5679fc4..37bc97c 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
@@ -851,10 +851,11 @@
}
@Test
- public void testCanDoFailOnAddLunVirtIOSCSIReadOnlyDisk() {
+ public void testCanDoSuccessOnAddLunVirtIOSCSIReadOnlyDisk() {
LunDisk disk = createISCSILunDisk();
disk.setDiskInterface(DiskInterface.VirtIO_SCSI);
disk.setReadOnly(true);
+ disk.setSgio(null);
AddDiskParameters parameters = createParameters();
parameters.setDiskInfo(disk);
@@ -868,12 +869,9 @@
when(diskValidator.isVirtIoScsiValid(any(VM.class))).thenReturn(ValidationResult.VALID);
when(diskValidator.isDiskInterfaceSupported(any(VM.class))).thenReturn(ValidationResult.VALID);
when(diskValidator.isReadOnlyPropertyCompatibleWithInterface()).thenReturn(ValidationResult.VALID);
-
when(diskValidator.isReadOnlyPropertyCompatibleWithLunInterface()).thenReturn(
- new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR));
doReturn(diskValidator).when(command).getDiskValidator(any(Disk.class));
- CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
-
VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR);
+ CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command);
}
private void fillDiskMap(LunDisk disk, VM vm, int expectedMapSize) {
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java
index 5e82da2..79cc000 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java
@@ -19,6 +19,9 @@
import org.ovirt.engine.core.common.businessentities.Disk;
import org.ovirt.engine.core.common.businessentities.DiskImage;
import org.ovirt.engine.core.common.businessentities.DiskInterface;
+
+import org.ovirt.engine.core.common.businessentities.LunDisk;
+import org.ovirt.engine.core.common.businessentities.ScsiGenericIO;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmDevice;
@@ -38,7 +41,9 @@
@Mock
private VmDAO vmDAO;
private DiskValidator validator;
+ private DiskValidator lunValidator;
private DiskImage disk;
+ private LunDisk lunDisk;
private static DiskImage createDisk() {
DiskImage disk = new DiskImage();
@@ -71,6 +76,12 @@
disk.setDiskInterface(DiskInterface.VirtIO);
validator = spy(new DiskValidator(disk));
doReturn(vmDAO).when(validator).getVmDAO();
+ }
+
+ private void setupForLun() {
+ lunDisk = new LunDisk();
+ lunValidator = spy(new DiskValidator(lunDisk));
+ doReturn(vmDAO).when(lunValidator).getVmDAO();
}
private VmDevice createVmDeviceForDisk(VM vm, Disk disk, Guid snapshotId,
boolean isPlugged) {
@@ -147,6 +158,13 @@
assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(),
failsWith(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR));
+
+ setupForLun();
+ lunDisk.setReadOnly(true);
+ lunDisk.setDiskInterface(DiskInterface.VirtIO_SCSI);
+ lunDisk.setSgio(ScsiGenericIO.FILTERED);
+ assertThat(lunValidator.isReadOnlyPropertyCompatibleWithInterface(),
+
failsWith(VdcBllMessages.SCSI_PASSTHROUGH_IS_NOT_SUPPORTED_FOR_READ_ONLY_DISK));
}
@Test
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BaseDisk.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BaseDisk.java
index f0363c0..c38e1c2 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BaseDisk.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BaseDisk.java
@@ -171,6 +171,10 @@
this.sgio = sgio;
}
+ public boolean isScsiPassthrough() {
+ return getSgio() != null;
+ }
+
public DiskAlignment getAlignment() {
return alignment;
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
index 54d09db..014c6e8 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
@@ -748,6 +748,7 @@
DISK_IS_ALREADY_SHARED_BETWEEN_VMS(ErrorType.CONFLICT),
VM_CANNOT_RUN_FROM_DISK_WITHOUT_PLUGGED_DISK(ErrorType.CONFLICT),
SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK(ErrorType.NOT_SUPPORTED),
+
SCSI_PASSTHROUGH_IS_NOT_SUPPORTED_FOR_READ_ONLY_DISK(ErrorType.NOT_SUPPORTED),
VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL(ErrorType.INCOMPATIBLE_VERSION),
CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED(ErrorType.CONFLICT),
CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS(ErrorType.CONFLICT),
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 7fa7d2f..5796e91 100644
---
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -936,6 +936,7 @@
ERROR_CANNOT_DETACH_DISK_WITH_SNAPSHOT=Cannot ${action} ${type}. The disk is
already configured in a snapshot. In order to detach it, remove the disk's
snapshots.
DISK_IS_ALREADY_SHARED_BETWEEN_VMS=Cannot ${action} ${type}. Disk is shared
between vms and cannot become unshareable . Detach the disk from the rest of
the vms it is attached to and then update the disk to be unshareable.
SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK="Cannot ${action} ${type}.
SCSI Generic IO is not supported for image disk."
+SCSI_PASSTHROUGH_IS_NOT_SUPPORTED_FOR_READ_ONLY_DISK="Cannot ${action}
${type}. SCSI device pass-throguh is not supported for a read-only disk."
VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL=Virtio-SCSI interface
is only available on cluster level 3.3 or higher.
CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED=Cannot ${action} ${type}.
VirtIO-SCSI is disabled for the VM
CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS=Cannot disable VirtIO-SCSI when disks
with a VirtIO-SCSI interface are plugged into the VM.
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
index 9f92325..0abdede 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
@@ -68,7 +68,24 @@
drive.put(VdsProperties.PropagateErrors,
disk.getPropagateErrors().toString().toLowerCase());
} else {
LunDisk lunDisk = (LunDisk) disk;
- drive.put(VdsProperties.Device, VmDeviceType.LUN.getName());
+
+ // If SCSI pass-through is enabled (VirtIO-SCSI/DirectLUN disk and
SGIO is defined),
+ // set device type as 'lun' (instead of 'disk') and set the
specified SGIO
+ boolean isVirtioScsi =
getParameters().getDisk().getDiskInterface() == DiskInterface.VirtIO_SCSI;
+ boolean isScsiPassthrough =
getParameters().getDisk().isScsiPassthrough();
+ if (isVirtioScsi) {
+ if (isScsiPassthrough) {
+ drive.put(VdsProperties.Device,
VmDeviceType.LUN.getName());
+ drive.put(VdsProperties.Sgio,
getParameters().getDisk().getSgio().toString().toLowerCase());
+ }
+ else {
+ drive.put(VdsProperties.Device,
VmDeviceType.DISK.getName());
+ }
+ }
+ else {
+ drive.put(VdsProperties.Device, VmDeviceType.LUN.getName());
+ }
+
drive.put(VdsProperties.Guid, lunDisk.getLun().getLUN_id());
drive.put(VdsProperties.Format,
VolumeFormat.RAW.toString().toLowerCase());
drive.put(VdsProperties.PropagateErrors,
PropagateErrors.Off.toString()
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
index c7ccd66..4dc5444 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
@@ -286,7 +286,9 @@
break;
case VirtIO_SCSI:
struct.put(VdsProperties.INTERFACE, VdsProperties.Scsi);
- if (disk.getDiskStorageType() == DiskStorageType.LUN) {
+ // If SCSI pass-through is enabled (DirectLUN disk and
SGIO is defined),
+ // set device type as 'lun' (instead of 'disk') and set
the specified SGIO.
+ if (disk.getDiskStorageType() == DiskStorageType.LUN &&
disk.isScsiPassthrough()) {
struct.put(VdsProperties.Device,
VmDeviceType.LUN.getName());
struct.put(VdsProperties.Sgio,
disk.getSgio().toString().toLowerCase());
}
diff --git
a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
index 60f6928..3b78c6c 100644
---
a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
+++
b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
@@ -2541,6 +2541,9 @@
@DefaultStringValue("Cannot ${action} ${type}. SCSI Generic IO is not
supported for image disk.")
String SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK();
+ @DefaultStringValue("Cannot ${action} ${type}. SCSI device pass-throguh is
not supported for a read-only disk.")
+ String SCSI_PASSTHROUGH_IS_NOT_SUPPORTED_FOR_READ_ONLY_DISK();
+
@DefaultStringValue("VirtIO-SCSI interface is only available on cluster
level 3.3 or higher.")
String VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL();
diff --git
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
index 84da147..f2a3847 100644
---
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
+++
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
@@ -1067,6 +1067,9 @@
@DefaultStringValue("Read Only")
String isReadOnlyVmDiskPopup();
+ @DefaultStringValue("Enable SCSI Pass-Through")
+ String isScsiPassthroughEditor();
+
@DefaultStringValue("Allow Privileged SCSI I/O")
String isSgIoUnfilteredEditor();
diff --git
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.java
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.java
index e6b3039..6dc5d51 100644
---
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.java
+++
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.java
@@ -163,6 +163,11 @@
EntityModelCheckBoxEditor isReadOnlyEditor;
@UiField(provided = true)
+ @Path("isScsiPassthrough.entity")
+ @WithElementId("isScsiPassthrough")
+ EntityModelCheckBoxEditor isScsiPassthroughEditor;
+
+ @UiField(provided = true)
@Path("isSgIoUnfiltered.entity")
@WithElementId("isSgIoUnfiltered")
EntityModelCheckBoxEditor isSgIoUnfilteredEditor;
@@ -273,6 +278,7 @@
isBootableEditor.setLabel(constants.isBootableVmDiskPopup());
isShareableEditor.setLabel(constants.isShareableVmDiskPopup());
isReadOnlyEditor.setLabel(constants.isReadOnlyVmDiskPopup());
+ isScsiPassthroughEditor.setLabel(constants.isScsiPassthroughEditor());
isSgIoUnfilteredEditor.setLabel(constants.isSgIoUnfilteredEditor());
attachEditor.setLabel(constants.attachDiskVmDiskPopup());
isPluggedEditor.setLabel(constants.activateVmDiskPopup());
@@ -315,6 +321,7 @@
isBootableEditor = new EntityModelCheckBoxEditor(Align.RIGHT);
isShareableEditor = new EntityModelCheckBoxEditor(Align.RIGHT);
isReadOnlyEditor = new EntityModelCheckBoxEditor(Align.RIGHT);
+ isScsiPassthroughEditor = new EntityModelCheckBoxEditor(Align.RIGHT);
isSgIoUnfilteredEditor = new EntityModelCheckBoxEditor(Align.RIGHT);
isPluggedEditor = new EntityModelCheckBoxEditor(Align.RIGHT);
attachEditor = new EntityModelCheckBoxEditor(Align.RIGHT);
@@ -800,11 +807,12 @@
storageTypeEditor.setTabIndex(nextTabIndex++);
plugDiskToVmEditor.setTabIndex(nextTabIndex++);
wipeAfterDeleteEditor.setTabIndex(nextTabIndex++);
+ isPluggedEditor.setTabIndex(nextTabIndex++);
isBootableEditor.setTabIndex(nextTabIndex++);
isShareableEditor.setTabIndex(nextTabIndex++);
- isSgIoUnfilteredEditor.setTabIndex(nextTabIndex++);
isReadOnlyEditor.setTabIndex(nextTabIndex++);
- isPluggedEditor.setTabIndex(nextTabIndex++);
+ isScsiPassthroughEditor.setTabIndex(nextTabIndex++);
+ isSgIoUnfilteredEditor.setTabIndex(nextTabIndex++);
return nextTabIndex;
}
diff --git
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.ui.xml
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.ui.xml
index 7737721..7e109b9 100644
---
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.ui.xml
+++
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmDiskPopupWidget.ui.xml
@@ -61,6 +61,10 @@
padding-left: 75px;
}
+ .checkBoxSecondary {
+ margin-left: 20px;
+ }
+
.externalDiskPanel {
height: 298px;
width: 100%;
@@ -118,8 +122,9 @@
<e:EntityModelCheckBoxEditor
ui:field="wipeAfterDeleteEditor" addStyleNames="{style.checkBoxEditor}"/>
<e:EntityModelCheckBoxEditor
ui:field="isBootableEditor" addStyleNames="{style.checkBoxEditor}" />
<e:EntityModelCheckBoxEditor
ui:field="isShareableEditor" addStyleNames="{style.checkBoxEditor}"/>
- <e:EntityModelCheckBoxEditor
ui:field="isSgIoUnfilteredEditor" addStyleNames="{style.checkBoxEditor}"/>
<e:EntityModelCheckBoxEditor ui:field="isReadOnlyEditor"
addStyleNames="{style.checkBoxEditor}"/>
+ <e:EntityModelCheckBoxEditor
ui:field="isScsiPassthroughEditor" addStyleNames="{style.checkBoxEditor}"/>
+ <e:EntityModelCheckBoxEditor
ui:field="isSgIoUnfilteredEditor" addStyleNames="{style.checkBoxEditor}
{style.checkBoxSecondary}"/>
</g:VerticalPanel>
</g:HorizontalPanel>
<g:Label ui:field="message"
addStyleNames="{style.errorMessageLabel}" />
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/AbstractDiskModel.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/AbstractDiskModel.java
index 794d0bd..9c9230e 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/AbstractDiskModel.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/AbstractDiskModel.java
@@ -62,10 +62,11 @@
private EntityModel isBootable;
private EntityModel isShareable;
private EntityModel isPlugged;
- private EntityModel isReadOnly;
private EntityModel isAttachDisk;
- private EntityModel isInternal;
private EntityModel isDirectLunDiskAvaialable;
+ private EntityModel isInternal;
+ private EntityModel isReadOnly;
+ private EntityModel isScsiPassthrough;
private EntityModel isSgIoUnfiltered;
private EntityModel sizeExtend;
@@ -117,20 +118,20 @@
this.isPlugged = isPlugged;
}
- public EntityModel getIsReadOnly() {
- return isReadOnly;
- }
-
- public void setIsReadOnly(EntityModel isReadOnly) {
- this.isReadOnly = isReadOnly;
- }
-
public EntityModel getIsAttachDisk() {
return isAttachDisk;
}
public void setIsAttachDisk(EntityModel isAttachDisk) {
this.isAttachDisk = isAttachDisk;
+ }
+
+ public EntityModel getIsDirectLunDiskAvaialable() {
+ return isDirectLunDiskAvaialable;
+ }
+
+ public void setIsDirectLunDiskAvaialable(EntityModel
isDirectLunDiskAvaialable) {
+ this.isDirectLunDiskAvaialable = isDirectLunDiskAvaialable;
}
public EntityModel getIsInternal() {
@@ -141,12 +142,20 @@
this.isInternal = isInternal;
}
- public EntityModel getIsDirectLunDiskAvaialable() {
- return isDirectLunDiskAvaialable;
+ public EntityModel getIsReadOnly() {
+ return isReadOnly;
}
- public void setIsDirectLunDiskAvaialable(EntityModel
isDirectLunDiskAvaialable) {
- this.isDirectLunDiskAvaialable = isDirectLunDiskAvaialable;
+ public void setIsReadOnly(EntityModel isReadOnly) {
+ this.isReadOnly = isReadOnly;
+ }
+
+ public EntityModel getIsScsiPassthrough() {
+ return isScsiPassthrough;
+ }
+
+ public void setIsScsiPassthrough(EntityModel isScsiPassthrough) {
+ this.isScsiPassthrough = isScsiPassthrough;
}
public EntityModel getIsSgIoUnfiltered() {
@@ -284,9 +293,17 @@
setIsReadOnly(new EntityModel());
getIsReadOnly().setEntity(false);
+ getIsReadOnly().getEntityChangedEvent().addListener(this);
+
+ setIsScsiPassthrough(new EntityModel<Boolean>());
+ getIsScsiPassthrough().setIsAvailable(false);
+ getIsScsiPassthrough().setEntity(true);
+ getIsScsiPassthrough().getEntityChangedEvent().addListener(this);
setIsSgIoUnfiltered(new EntityModel());
getIsSgIoUnfiltered().setIsAvailable(false);
+ getIsSgIoUnfiltered().setEntity(false);
+ getIsSgIoUnfiltered().getEntityChangedEvent().addListener(this);
setIsDirectLunDiskAvaialable(new EntityModel());
getIsDirectLunDiskAvaialable().setEntity(true);
@@ -657,9 +674,28 @@
boolean isInternal = (Boolean) getIsInternal().getEntity();
DiskInterface diskInterface = (DiskInterface)
getDiskInterface().getSelectedItem();
getIsSgIoUnfiltered().setIsAvailable(!isInternal &&
DiskInterface.VirtIO_SCSI.equals(diskInterface));
+ getIsScsiPassthrough().setIsAvailable(!isInternal &&
DiskInterface.VirtIO_SCSI.equals(diskInterface));
+ updateScsiPassthroughChangeability();
updateReadOnlyChangeability();
updatePlugChangeability();
+ }
+
+ protected void updateScsiPassthroughChangeability() {
+
getIsScsiPassthrough().setIsChangable(!Boolean.TRUE.equals(getIsReadOnly().getEntity())
&& isEditEnabled());
+
getIsScsiPassthrough().setChangeProhibitionReason(CONSTANTS.cannotEnableScsiPassthroughForLunReadOnlyDisk());
+
+ updateSgIoUnfilteredChangeability();
+ }
+
+ protected void updateSgIoUnfilteredChangeability() {
+ if (!Boolean.TRUE.equals(getIsScsiPassthrough().getEntity())) {
+
getIsSgIoUnfiltered().setChangeProhibitionReason(CONSTANTS.cannotEnableSgioWhenScsiPassthroughDisabled());
+ getIsSgIoUnfiltered().setIsChangable(false);
+ getIsSgIoUnfiltered().setEntity(false);
+ return;
+ }
+ getIsSgIoUnfiltered().setIsChangable(isEditEnabled());
}
protected void updateReadOnlyChangeability() {
@@ -673,13 +709,17 @@
}
boolean isDirectLUN =
Boolean.FALSE.equals(getIsInternal().getEntity());
- if (diskInterface == DiskInterface.VirtIO_SCSI && isDirectLUN) {
-
getIsReadOnly().setChangeProhibitionReason(CONSTANTS.cannotEnableVirtIoScsiInterfaceForLunReadOnlyDisk());
+ boolean isScsiPassthrough =
Boolean.TRUE.equals(getIsScsiPassthrough().getEntity());
+ if (diskInterface == DiskInterface.VirtIO_SCSI && isDirectLUN &&
isScsiPassthrough) {
+
getIsReadOnly().setChangeProhibitionReason(CONSTANTS.cannotEnableReadonlyWhenScsiPassthroughEnabled());
getIsReadOnly().setIsChangable(false);
getIsReadOnly().setEntity(false);
return;
}
+
+
getIsReadOnly().setIsChangable(isEditEnabled());
+ getIsReadOnly().setEntity(getIsNew() ? Boolean.FALSE :
getDisk().getReadOnly());
}
private void updatePlugChangeability() {
@@ -853,7 +893,8 @@
LunDisk lunDisk = getLunDisk();
DiskInterface diskInterface = (DiskInterface)
getDiskInterface().getSelectedItem();
if (DiskInterface.VirtIO_SCSI.equals(diskInterface)) {
-
lunDisk.setSgio(Boolean.TRUE.equals(getIsSgIoUnfiltered().getEntity()) ?
+
lunDisk.setSgio(Boolean.FALSE.equals(getIsScsiPassthrough().getEntity()) ? null
:
+ Boolean.TRUE.equals(getIsSgIoUnfiltered().getEntity())
?
ScsiGenericIO.UNFILTERED : ScsiGenericIO.FILTERED);
}
setDisk(lunDisk);
@@ -889,17 +930,19 @@
public void eventRaised(Event ev, Object sender, EventArgs args) {
super.eventRaised(ev, sender, args);
- if (ev.matchesDefinition(EntityModel.entityChangedEventDefinition) &&
sender == getIsWipeAfterDelete())
- {
- wipeAfterDelete_EntityChanged(args);
- }
- else if
(ev.matchesDefinition(EntityModel.entityChangedEventDefinition) && sender ==
getIsAttachDisk())
- {
- attachDisk_EntityChanged(args);
- }
- else if (ev.matchesDefinition(ListModel.entityChangedEventDefinition)
&& sender == getIsInternal())
- {
- isInternal_EntityChanged();
+ if (ev.matchesDefinition(EntityModel.entityChangedEventDefinition)) {
+ if (sender == getIsWipeAfterDelete()) {
+ wipeAfterDelete_EntityChanged(args);
+ } else if (sender == getIsAttachDisk()) {
+ attachDisk_EntityChanged(args);
+ } else if (sender == getIsReadOnly()) {
+ updateScsiPassthroughChangeability();
+ } else if (sender == getIsScsiPassthrough()) {
+ updateSgIoUnfilteredChangeability();
+ updateReadOnlyChangeability();
+ } else if (sender == getIsInternal()) {
+ isInternal_EntityChanged();
+ }
}
else if
(ev.matchesDefinition(ListModel.selectedItemChangedEventDefinition) && sender
== getVolumeType())
{
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditDiskModel.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditDiskModel.java
index 9bce547..9ec8337 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditDiskModel.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditDiskModel.java
@@ -39,6 +39,7 @@
getDescription().setEntity(getDisk().getDiskDescription());
getIsShareable().setEntity(getDisk().isShareable());
getIsWipeAfterDelete().setEntity(getDisk().isWipeAfterDelete());
+ getIsScsiPassthrough().setEntity(getDisk().isScsiPassthrough());
getIsSgIoUnfiltered().setEntity(getDisk().getSgio() ==
ScsiGenericIO.UNFILTERED);
getIsReadOnly().setEntity(getDisk().getReadOnly());
diff --git
a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
index f014e3d..7c2af00 100644
---
a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
+++
b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
@@ -2223,8 +2223,14 @@
@DefaultStringValue("Are you sure you want to place the following storage
domain(s) into maintenance mode?")
String
areYouSureYouWantToPlaceFollowingStorageDomainsIntoMaintenanceModeMsg();
- @DefaultStringValue("A VirtIO-ISCSI direct LUN disk can't be read-only.")
- String cannotEnableVirtIoScsiInterfaceForLunReadOnlyDisk();
+ @DefaultStringValue("A VirtIO-SCSI DirectLUN disk can't be set as
read-only when SCSI pass-through is enabled.")
+ String cannotEnableReadonlyWhenScsiPassthroughEnabled();
+
+ @DefaultStringValue("Privileged SCSI I/O can be set only when SCSI
pass-through is enabled.")
+ String cannotEnableSgioWhenScsiPassthroughDisabled();
+
+ @DefaultStringValue("SCSI pass-through is not supported for read-only
disks.")
+ String cannotEnableScsiPassthroughForLunReadOnlyDisk();
@DefaultStringValue("Global Maintenance Enabled")
String haGlobalMaintenance();
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 7be9ad1..fbc7cb2 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
@@ -907,6 +907,7 @@
ERROR_CANNOT_DETACH_DISK_WITH_SNAPSHOT=Cannot ${action} ${type}. The disk is
already configured in a snapshot. In order to detach it, remove the disk's
snapshots.
DISK_IS_ALREADY_SHARED_BETWEEN_VMS=Cannot ${action} ${type}. Disk is shared
between vms and cannot become unshareable . Detach the disk from the rest of
the vms it is attached to and then update the disk to be unshareable.
SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK="Cannot ${action} ${type}.
SCSI Generic IO is not supported for image disk."
+SCSI_PASSTHROUGH_IS_NOT_SUPPORTED_FOR_READ_ONLY_DISK="Cannot ${action}
${type}. SCSI device pass-throguh is not supported for a read-only disk."
VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL=Virtio-SCSI interface
is only available on cluster level 3.3 or higher.
CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED=Cannot ${action} ${type}.
VirtIO-SCSI is disabled for the VM
CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS=Cannot disable VirtIO-SCSI when disks
with a VirtIO-SCSI interface are plugged into the VM.
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 144da45..4893010 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
@@ -937,6 +937,7 @@
ERROR_CANNOT_DETACH_DISK_WITH_SNAPSHOT=Cannot ${action} ${type}. The disk is
already configured in a snapshot. In order to detach it, remove the disk's
snapshots.
DISK_IS_ALREADY_SHARED_BETWEEN_VMS=Cannot ${action} ${type}. Disk is shared
between vms and cannot become unshareable . Detach the disk from the rest of
the vms it is attached to and then update the disk to be unshareable.
SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK="Cannot ${action} ${type}.
SCSI Generic IO is not supported for image disk."
+SCSI_PASSTHROUGH_IS_NOT_SUPPORTED_FOR_READ_ONLY_DISK="Cannot ${action}
${type}. SCSI device pass-throguh is not supported for a read-only disk."
VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL=Virtio-SCSI interface
is only available on cluster level 3.3 or higher.
CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED=Cannot ${action} ${type}.
VirtIO-SCSI is disabled for the VM
CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS=Cannot disable VirtIO-SCSI when disks
with a VirtIO-SCSI interface are plugged into the VM.
diff --git
a/packaging/dbscripts/upgrade/03_04_0800_update_sgio_for_readonly_disks.sql
b/packaging/dbscripts/upgrade/03_04_0800_update_sgio_for_readonly_disks.sql
new file mode 100644
index 0000000..c7dc4be
--- /dev/null
+++ b/packaging/dbscripts/upgrade/03_04_0800_update_sgio_for_readonly_disks.sql
@@ -0,0 +1,8 @@
+-- Since SCSI device pass-through is not supported for read-only disks,
+-- nullify 'sgio' property for such disks.
+-- As discussed in BZ1118847, read-only/virtio-scsi/direct-lun disks
+-- should use SCSI device emulation instead.
+
+UPDATE base_disks
+SET sgio = NULL
+WHERE base_disks.disk_id IN (SELECT device_id FROM vm_device WHERE is_readonly
= TRUE);
--
To view, visit http://gerrit.ovirt.org/30905
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic317722be9ef8978b50352c12bc06ea8c906dcd0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Daniel Erez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
