Liron Ar has posted comments on this change.
Change subject: core, restapi: Introducing support for attaching disk snapshot
......................................................................
Patch Set 31:
(11 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java
Line 370: if (!validate(storageValidator.allDomainsExistAndActive()) ||
Line 371:
!validate(storageValidator.allDomainsWithinThresholds()) ||
Line 372: !performImagesChecks() ||
Line 373: !validate(vmValidator.vmDown()) ||
Line 374: // if the user just previewed a snapshot and selected
"undo" - the vm can have snapshots attached to other vms.
I prefer to leave it like that and not add a method just because of a comment :)
Line 375: (targetSnapshot.getType() != SnapshotType.PREVIEW &&
!validate(vmValidator.vmNotHavingDeviceSnapshotsAttachedToOtherVms()))) {
Line 376: return false;
Line 377: }
Line 378:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 51: implements QuotaStorageDependent {
Line 52:
Line 53: private List<PermissionSubject> listPermissionSubjects;
Line 54: private Map<Guid, List<Disk>> otherVmDisks = new HashMap<Guid,
List<Disk>>();
Line 55: /* vms that the disk (active image) is plugged to */
The comments were removed.
Line 56: private List<VM> vmsDiskPluggedTo;
Line 57: /* vms that a snapshot of the disk is plugged to. */
Line 58: private List<VM> vmsDiskSnapshotPluggedTo;
Line 59: /* vms that a disk or it's sansphot is plugged to. */
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 534: disk.size: xs:int #deprecated, replaced by
'provisioned_size'
Line 535: disk.sparse: xs:boolean
Line 536: disk.bootable: xs:boolean
Line 537: disk.shareable: xs:boolean
Line 538: disk.snapshot.id: xs:string
i've moved the snapshot id to the attach signature as optional, if you want i'd
add a completely different signature.
Line 539: disk.propagate_errors: xs:boolean
Line 540: disk.wipe_after_delete: xs:boolean
Line 541: disk.storage_domains.storage_domain--COLLECTION:
{storage_domain.id|name: 'xs:string'}
Line 542: description: add a new disk to the virtual machine allocating
space from the storage domain
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
Line 43: drive.put(VdsProperties.Type, VmDeviceType.DISK.getName());
Line 44: addAddress(drive, getParameters().getVmDevice().getAddress());
Line 45: drive.put(VdsProperties.INTERFACE,
disk.getDiskInterface().getName());
Line 46: drive.put(VdsProperties.Shareable,
Line 47: (vmDevice.getSnapshotId() != null &&
FeatureSupported.hotPlugDiskSnapshot(getParameters().getVm()
it seems pretty clear to me, just an if clause :) ..i prefer to not add a
method for this minor thing and keep here the code shorter unless you are
really against it, i'm in favor of keeping this line as is.
Line 48: .getVdsGroupCompatibilityVersion())) ?
VdsProperties.Transient
Line 49: : disk.isShareable());
Line 50: drive.put(VdsProperties.Optional, Boolean.FALSE.toString());
Line 51: drive.put(VdsProperties.ReadOnly,
String.valueOf(vmDevice.getIsReadOnly()));
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
Line 301:
Line 302: addBootOrder(vmDevice, struct);
Line 303: struct.put(VdsProperties.Shareable,
Line 304: (vmDevice.getSnapshotId() != null &&
FeatureSupported.hotPlugDiskSnapshot(vm.getVdsGroupCompatibilityVersion())) ?
VdsProperties.Transient
Line 305: : disk.isShareable());
i prefer to keep it as is , it's used only once here and it's an if clause
which shouldn't be problematic to debug..unless you really insist, i'm in favor
of leaving as is.
Line 306: struct.put(VdsProperties.Optional,
Boolean.FALSE.toString());
Line 307: struct.put(VdsProperties.ReadOnly,
String.valueOf(vmDevice.getIsReadOnly()));
Line 308: struct.put(VdsProperties.SpecParams,
vmDevice.getSpecParams());
Line 309: struct.put(VdsProperties.DeviceId,
String.valueOf(vmDevice.getId().getDeviceId()));
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
Line 345:
Line 346: @DefaultStringValue("Cannot ${action} ${type}. The following VM
disks snapshots are attached to other VMs: ${disksInfo}. Please detach them
from those VMs and try again.")
Line 347: String
ACTION_TYPE_FAILED_VM_DISK_SNAPSHOT_IS_PLUGGED_TO_ANOTHER_VM();
Line 348:
Line 349: @DefaultStringValue("Cannot ${action} ${type}. The following VM
activated disks are disk snapshots: ${diskAliases}. Please deactivate them and
try again.")
Done
Line 350: String ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT();
Line 351:
Line 352: @DefaultStringValue("Cannot ${action} ${type}: The following
disks are locked: ${diskAliases}. Please try again in a few minutes.")
Line 353: String ACTION_TYPE_FAILED_DISKS_LOCKED();
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditDiskModel.java
Line 49: setVolumeFormat(diskImage.getVolumeFormat());
Line 50:
Line 51: boolean isExtendImageSizeEnabled = getVm() != null &&
!diskImage.isDiskSnapshot() &&
Line 52: VdcActionUtils.canExecute(Arrays.asList(getVm()),
VM.class, VdcActionType.ExtendImageSize);
Line 53:
1. IMO the clause is clear enough with the added condition and there's adding
another method will just complicate things around here - but let's hear more
opinions about that.
2. Done
Line 54: getSizeExtend().setIsChangable(isExtendImageSizeEnabled);
Line 55:
Line 56: Guid storageDomainId = diskImage.getStorageIds().get(0);
Line 57: AsyncDataProvider.getStorageDomainById(new
AsyncQuery(this, new INewAsyncCallback() {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmDiskListModel.java
Line 408: diskModel.setVm(getEntity());
Line 409:
Line 410: items.add(diskModel);
Line 411:
Line 412: // A shared disk/disk snapshot can only be detached
Done
Line 413: if (disk.getNumberOfVms() > 1) {
Line 414: model.getLatch().setIsChangable(false);
Line 415: }
Line 416: }
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
Line 129: DIRECTORY_GROUP_CANNOT_REMOVE_DIRECTORY_GROUP_ATTACHED_TO_VM=Cannot
remove Directory Group. Detach Directory Group from VM first.
Line 130: VM_NOT_FOUND=VM not found
Line 131: ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is
previewing a Snapshot.
Line 132: ACTION_TYPE_FAILED_VM_DISK_SNAPSHOT_IS_PLUGGED_TO_ANOTHER_VM=Cannot
${action} ${type}. The following VM disks snapshots are attached to other VMs:
${disksInfo}. Please detach them from those VMs and try again.
Line 133: ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT=Cannot ${action}
${type}. The following VM activated disks are disk snapshots: ${diskAliases}.
Please deactivate them and try again.
Done
Line 134: ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The
following disks are locked: ${diskAliases}. Please try again in a few minutes.
Line 135: ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The
following attached disks are in ILLEGAL status: ${diskAliases} - please remove
them and try again.
Line 136: ACTION_TYPE_FAILED_MOVE_DISKS_MIXED_PLUGGED_STATUS=Cannot ${action}
${type}. The following disks could not be moved: ${diskAliases}. Please make
sure that all disks are active or inactive in the VM.
Line 137: ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action}
${type}. The following disks already exist: ${diskAliases}. Please import as a
clone.
....................................................
File
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
Line 133: DIRECTORY_GROUP_CANNOT_REMOVE_DIRECTORY_GROUP_ATTACHED_TO_VM=Cannot
remove Directory Group. Detach Directory Group from VM first.
Line 134: VM_NOT_FOUND=VM not found
Line 135: ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is
previewing a Snapshot.
Line 136: ACTION_TYPE_FAILED_VM_DISK_SNAPSHOT_IS_PLUGGED_TO_ANOTHER_VM=Cannot
${action} ${type}. The following VM disks snapshots are attached to other VMs:
${disksInfo}. Please detach them from those VMs and try again.
Line 137: ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT=Cannot ${action}
${type}. The following VM activated disks are disk snapshots: ${diskAliases}.
Please deactivate them and try again.
Done for both of the messages
Line 138: ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The
following disks are locked: ${diskAliases}. Please try again in a few minutes.
Line 139: ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The
following attached disks are in ILLEGAL status: ${diskAliases} - please remove
them and try again.
Line 140: ACTION_TYPE_FAILED_MOVE_DISKS_MIXED_PLUGGED_STATUS=Cannot ${action}
${type}. The following disks could not be moved: ${diskAliases}. Please make
sure that all disks are active or inactive in the VM.
Line 141: ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action}
${type}. The following disks already exist: ${diskAliases}. Please import as a
clone.
Line 222: ACTION_TYPE_FAILED_NO_VDS_IN_POOL=Cannot ${action} ${type}. There is
no active Host in the Data Center.
Line 223: ACTION_TYPE_FAILED_REQUESTED_DISK_SIZE_IS_TOO_SMALL=Cannot ${action}
${type}. New disk size must be larger than current disk size.
Line 224: ACTION_TYPE_FAILED_CLOUD_INIT_IS_NOT_SUPPORTED=Cannot ${action}
${type}. Cloud-Init is only supported on cluster compatibility version 3.3 and
higher.
Line 225: ACTION_TYPE_FAILED_CANNOT_RESIZE_DISK_SNAPSHOT=Cannot ${action}
${type}. Disk snapshot cannot be resized.
Line 226: >>>>>>> core, restapi: Introducing support for attaching disk snapshot
Done
Line 227: VAR__TYPE__HOST=$type Host
Line 228: VAR__ENTITIES__HOSTS=$entities hosts
Line 229: VAR__TYPE__NETWORK=$type Network
Line 230: VAR__TYPE__NETWORKS=$type Networks
--
To view, visit http://gerrit.ovirt.org/17679
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02579bf1a91cd294a5040acf432f1fdb87eb18c1
Gerrit-PatchSet: 31
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
