Maor Lipchuk has posted comments on this change.
Change subject: core: Add storage validation when plugging an image.
......................................................................
Patch Set 4: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
Line 100: }
Line 101: if (isImageDisk) {
Line 102: StorageDomain storageDomain =
getStorageDomainDAO().getForStoragePool(
Line 103: ((DiskImage) disk).getStorageIds().get(0),
((DiskImage) disk).getStoragePoolId());
Line 104: StorageDomainValidator storageDomainValidator = new
StorageDomainValidator(storageDomain);
the validator will have an attribute of storageDomain which is null
when we will call isDomainExistAndActive it will already check if the storage
domain is null and will return false with the message
ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST
Line 105: if
(!validate(storageDomainValidator.isDomainExistAndActive())) {
Line 106: return false;
Line 107: }
Line 108: }
Line 117
Line 118
Line 119
Line 120
Line 121
done, rebase bug
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java
Line 49: checkCanPerformPlugUnPlugDisk() &&
Line 50: isVmNotInPreviewSnapshot() &&
Line 51: imageValidation();
Line 52: }
Line 53:
done, added a validation for VM status (UP or Paused) also added a comment to
describe why we do it
Line 54: private boolean imageValidation() {
Line 55: if (disk.getDiskStorageType() != DiskStorageType.IMAGE) {
Line 56: return true;
Line 57: }
Line 50: isVmNotInPreviewSnapshot() &&
Line 51: imageValidation();
Line 52: }
Line 53:
Line 54: private boolean imageValidation() {
done will change it to imageStorageValidation
Line 55: if (disk.getDiskStorageType() != DiskStorageType.IMAGE) {
Line 56: return true;
Line 57: }
Line 58: StorageDomain storageDomain =
getStorageDomainDAO().getForStoragePool(
Line 56: return true;
Line 57: }
Line 58: StorageDomain storageDomain =
getStorageDomainDAO().getForStoragePool(
Line 59: ((DiskImage) disk).getStorageIds().get(0),
((DiskImage) disk).getStoragePoolId());
Line 60: StorageDomainValidator storageDomainValidator = new
StorageDomainValidator(storageDomain);
I don't think it is that crucial, on the other hand adding a new parameter
might be less readable and consume more reference bits IMHO :-)
but I don't mind changing it
Line 61: return
validate(storageDomainValidator.isDomainExistAndActive());
Line 62: }
Line 63:
Line 64: private boolean checkCanPerformPlugUnPlugDisk() {
--
To view, visit http://gerrit.ovirt.org/17170
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieeb2522bc5aa280b9b98ef728737aeeaa82d1263
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
