Allon Mureinik has posted comments on this change.
Change subject: core: Add snapshot validation for empty guid.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java
Line 298: }
Line 299:
Line 300: @Override
Line 301: protected boolean canDoAction() {
Line 302: if (Guid.Empty.equals(getParameters().getDstSnapshotId())) {
Use Guid.isNullOrEmpty - a null snapshot ID is just as bad, if not worse here.
Line 303: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID);
Line 304: }
Line 305:
Line 306: SnapshotsValidator snapshotValidator =
createSnapshotValidator();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
Line 198: }
Line 199:
Line 200: @Override
Line 201: protected boolean canDoAction() {
Line 202: if (Guid.Empty.equals(getParameters().getDstSnapshotId())) {
Use Guid.isNullOrEmpty - a null snapshot ID is just as bad, if not worse here.
Line 203: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID);
Line 204: }
Line 205:
Line 206: Snapshot snapshot =
getSnapshotDao().get(getParameters().getDstSnapshotId());
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 12: VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM=Cannot delete Template. Template
is being used by the following VMs: ${vmsList}.
Line 13: VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The
Template does not exist on the following Storage Domains:
${domainsList}.\nEither verify that Template exists on all Storage Domains
listed on the domains list,\nor do not send domains list in order to delete all
instances of the Template from the system.
Line 14: ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}.
VM's Image does not exist.
Line 15: ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action}
${type}. VM's Snapshot does not exist.
Line 16: ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID=Cannot ${action} ${type}.
The snapshot configuration has been corrupted (snapshot id is empty). Please
contact the system administrator.
s/has been/is/
s/id/ID/
Also, I don't think "contact the system administrator" is an appropriate error
message, although I can't think of a better one right now.
Line 17: ACTION_TYPE_FAILED_VM_SNAPSHOT_HAS_NO_CONFIGURATION=Cannot ${action}
${type}. The snapshot ${SnapshotName} of VM ${VmName} has no configuration
available. Please choose a snapshot with configuration available.
Line 18: ACTION_TYPE_FAILED_VM_SNAPSHOT_IS_BROKEN=Cannot ${action} ${type}. The
snapshot is broken, and no further work can be done on it. Please remove this
snapshot from the VM.
Line 19: IMAGE_REPOSITORY_NOT_FOUND=Storage Domain cannot be accessed.\n\
Line 20: Possible reasons:\n\
....................................................
Commit Message
Line 7: core: Add snapshot validation for empty guid.
Line 8:
Line 9: Currently when virt-v2v is used to convert a vm from a foreign
Line 10: hypervisor (Xen, VMware and etc), it sets the vm_snapshot_id of
Line 11: all disks of that vm to empty guid.
Is this a bug in v2v, or an intended behaviour?
Line 12: This later causes issues doing different tasks on snapshots.
Line 13:
Line 14: The proposed fix is to add a validation whether the
Line 15: snapshot id is empty and provides an appropriate message.
Line 11: all disks of that vm to empty guid.
Line 12: This later causes issues doing different tasks on snapshots.
Line 13:
Line 14: The proposed fix is to add a validation whether the
Line 15: snapshot id is empty and provides an appropriate message.
Shouldn't this be handled when importing such an OVF instead of having CDAs
later on?
Line 16:
Line 17: Change-Id: I19bc2982adecca198f8dc38ca011f4528dd58db9
--
To view, visit http://gerrit.ovirt.org/15701
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I19bc2982adecca198f8dc38ca011f4528dd58db9
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
