Maor Lipchuk has posted comments on this change.
Change subject: core: introducing virtio-scsi support
......................................................................
Patch Set 1: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java
Line 127: protected void updateDisksFromDb() {
Line 128: VmHandler.updateDisksFromDb(getVm());
Line 129: }
Line 130:
Line 131: protected boolean validateVirtIoScsi(Disk oldDisk, Disk newDisk) {
This API of this method is confusing when adding/attaching a disk (You pass the
same disk argument twice)
Why not add two methods, one for new disk, and another for update disk.
both methods can share some logic using refactoring.
Line 132: if (DiskInterface.VirtIO_SCSI != newDisk.getDiskInterface()) {
Line 133: return true;
Line 134: }
Line 135:
Line 154:
Line 155: protected boolean isOsSupportedForVirtIoScsi() {
Line 156: String vmOs = getVm().getOs().name();
Line 157: String[] unsupportedOSs = Config.<String>
GetValue(ConfigValues.VirtIoScsiUnsupportedOsList).split(",");
Line 158: for (String os : unsupportedOSs) {
Question: Have u considered to use contains() (With toLowerCase) instead
iterate with a for loop here.
Line 159: if (os.equalsIgnoreCase(vmOs)) {
Line 160:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_GUEST_OS_VERSION_IS_NOT_SUPPORTED);
Line 161: return false;
Line 162: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConfigurationValues.java
Line 84: MigrationSupportForNativeUsb(ConfigAuthType.User),
Line 85: MigrationNetworkEnabled,
Line 86: VncKeyboardLayout(ConfigAuthType.User),
Line 87: VncKeyboardLayoutValidValues(ConfigAuthType.User),
Line 88: VirtIoScsiEnabled(ConfigAuthType.User),
Seems to me the comma is redundant here
Line 89: ;
Line 90:
Line 91: public static enum ConfigAuthType {
Line 92: Admin,
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 757: SHAREABLE_DISK_IS_NOT_SUPPORTED_FOR_DISK=Cannot ${action} ${type}.
Disk cannot be shareable if it depends on a snapshot. In order to share it,
remove the disk's snapshots.
Line 758: SHAREABLE_DISK_IS_NOT_SUPPORTED_BY_VOLUME_FORMAT=Cannot ${action}
${type}. Disk's volume format is not supported for shareable disk.
Line 759: 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.
Line 760: 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.
Line 761: SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISk="Cannot ${action}
${type}. SCSI Generic IO is not supported for image disk."
s/DISk/DISK
Line 762:
Line 763: #Suspected (not in use?)
Line 764: USER_PASSWORD_EXPIRED=Cannot Login. User Password has expired, Please
change your password.
Line 765: USER_CANNOT_LOGIN_DOMAIN_NOT_SUPPORTED=Cannot Login. The Domain
provided is not configured, please contact your system administrator.
--
To view, visit http://gerrit.ovirt.org/14906
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie572b8106b3fe5becec69c140546db81bc671c96
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
