Maor Lipchuk has posted comments on this change.

Change subject: core: Deprecate broken snapshots upgrade script
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.ovirt.org/#/c/27681/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java:

Line 275:                     
!validate(diskImagesValidator.diskImagesNotLocked())) {
Line 276:               return false;
Line 277:           }
Line 278: 
Line 279:           DiskImagesValidator diskImagesToPreviewValidator = new 
DiskImagesValidator(getImagesToPreview());
please use formatter
Line 280:           if 
(!validate(diskImagesToPreviewValidator.diskImagesNotIllegal()) ||
Line 281:                   
!validate(diskImagesToPreviewValidator.diskImagesNotLocked())) {
Line 282:               return false;
Line 283:           }


Line 276:               return false;
Line 277:           }
Line 278: 
Line 279:           DiskImagesValidator diskImagesToPreviewValidator = new 
DiskImagesValidator(getImagesToPreview());
Line 280:           if 
(!validate(diskImagesToPreviewValidator.diskImagesNotIllegal()) ||
I would consider another message for this scenario:
Cannot ${action} ${type}. The following attached disks was in ILLEGAL status at 
the time which the snapshot was created: ${diskAliases} - please remove them 
and try again.
Line 281:                   
!validate(diskImagesToPreviewValidator.diskImagesNotLocked())) {
Line 282:               return false;
Line 283:           }
Line 284: 


Line 277:           }
Line 278: 
Line 279:           DiskImagesValidator diskImagesToPreviewValidator = new 
DiskImagesValidator(getImagesToPreview());
Line 280:           if 
(!validate(diskImagesToPreviewValidator.diskImagesNotIllegal()) ||
Line 281:                   
!validate(diskImagesToPreviewValidator.diskImagesNotLocked())) {
How could we have locked status non active images? We can't have a snapshot if 
one of the images is locked.
Line 282:               return false;
Line 283:           }
Line 284: 
Line 285:           Set<Guid> storageIds = 
ImagesHandler.getAllStorageIdsForImageIds(diskImages);


-- 
To view, visit http://gerrit.ovirt.org/27681
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia91153b1c3c363bca2d947974e6a67c459dba008
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to