Liron Ar has posted comments on this change.
Change subject: core, restapi: Introducing support for attaching disk snapshot
......................................................................
Patch Set 6:
(14 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java
Line 125: }
Line 126:
Line 127: protected Disk loadDisk(Guid diskId) {
Line 128: if (getParameters().getSnapshotId() == null) {
Line 129: return getDiskDao().get((Guid)
getParameters().getEntityInfo().getId());
Done
Line 130: } else {
Line 131: return
getDiskImageDao().getDiskSnapshotForVmSnapshot(diskId,
getParameters().getSnapshotId());
Line 132: }
Line 133: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java
Line 76: }
Line 77:
Line 78: updateDisksFromDb();
Line 79: if ((!isOperationPerformedOnDiskSnapshot() &&
!isDiskCanBeAddedToVm(disk, getVm())) || !isDiskPassPciAndIdeLimit(disk)) {
Line 80: return false;
Can you please elaborate? if the operation is performed on disk snapshot the
first check is just ignored, the messages remains as they were.
Line 81: }
Line 82:
Line 83: if (getVmDeviceDao().exists(new VmDeviceId(disk.getId(),
getVmId()))) {
Line 84: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_ALREADY_ATTACHED);
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DetachDiskFromVmCommand.java
Line 54:
Line 55: // Check if disk has no snapshots before detaching it.
Line 56: if (retValue && DiskStorageType.IMAGE ==
disk.getDiskStorageType()) {
Line 57: // if a disk snapshot is attached, it can be detached if
the snapshot as currently
Line 58: // a user can attach a disk snapshot from another vm only.
Done
Line 59: if (vmDevice.getSnapshotId() == null &&
getDiskImageDao().getAllSnapshotsForImageGroup(disk.getId()).size() > 1) {
Line 60:
addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_DETACH_DISK_WITH_SNAPSHOT);
Line 61: retValue = false;
Line 62: }
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java
Line 89: * @return The boot disk that is attached to the specified VM,
null if no attached disk is defined as boot.
Line 90: */
Line 91: Disk getVmBootDisk(Guid vmId);
Line 92:
Line 93:
Done
Line 94: /**
Line 95: * Returns the Disk with the specified id, with optional filtering.
Line 96: *
Line 97: * @param id
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDeviceDAODbFacadeImpl.java
Line 48: .addValue("is_readonly", entity.getIsReadOnly())
Line 49: .addValue("alias", entity.getAlias())
Line 50: .addValue("custom_properties",
Line 51:
SerializationFactory.getSerializer().serialize(entity.getCustomProperties()))
Line 52: .addValue("snapshot_id", entity.getSnapshotId());
yes, for previous entities it would be null.
Line 53: }
Line 54:
Line 55: @Override
Line 56: protected RowMapper<VmDevice> createEntityRowMapper() {
Line 143: vmDevice.setIsReadOnly(rs.getBoolean("is_readonly"));
Line 144: vmDevice.setAlias(rs.getString("alias"));
Line 145:
vmDevice.setCustomProperties(SerializationFactory.getDeserializer()
Line 146:
.deserializeOrCreateNew(rs.getString("custom_properties"),
LinkedHashMap.class));
Line 147: vmDevice.setSnapshotId(getGuid(rs, "snapshot_id"));
it's fine - getGuid() specifies -
"@return a {@link Guid} representing the UUID in the column, or the default
value if it was <code>null</code>."
in that method overload the default sent value is null, so it would be set as
null.
Line 148: return vmDevice;
Line 149: }
Line 150: }
Line 151:
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 2661: <xs:element ref="quota" minOccurs="0" maxOccurs="1"/>
Line 2662: <xs:element name="lun_storage" type="Storage"
minOccurs="0"/>
Line 2663: <xs:element name="sgio" type="xs:string" minOccurs="0"/>
Line 2664: <xs:element ref="snapshot" minOccurs="0" maxOccurs="1"/>
Line 2665: <xs:element name="snapshot_id" type="xs:string"
minOccurs="0" maxOccurs="1"/>
it was added intentionally, because of what seems like infrastructure addition
that we need - quoting from the commit message :) :
"2. REST issue with multiple parents of resource, as a temp soltuion -
i've added the snapshot id info as a new tag "snapshot_id""
the issue is in the addLinks() method and is caused by that a disk has vm as a
parent and snapshot as well, snapshot is "settable" from vm and vm from
snapshot..etc :) so for testing only, i've added the snapshot id meanwhile so
we could test meanwhile.
Line 2666: </xs:sequence>
Line 2667: </xs:extension>
Line 2668: </xs:complexContent>
Line 2669: </xs:complexType>
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
Line 337: @DefaultStringValue("Cannot ${action} ${type}. VM is previewing a
Snapshot.")
Line 338: String ACTION_TYPE_FAILED_VM_IN_PREVIEW();
Line 339:
Line 340: @DefaultStringValue("Cannot ${action} ${type}. The following VM
activated disks are snapshots: ${diskAliases}, please deactivate them and try
again.")
Line 341: String ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT();
AbstractDiskVmCommand - isHotPlugOperationSupported()
Line 342:
Line 343: @DefaultStringValue("Cannot ${action} ${type}: The following
disks are locked: ${diskAliases}. Please try again in a few minutes.")
Line 344: String ACTION_TYPE_FAILED_DISKS_LOCKED();
Line 345:
....................................................
File
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
Line 130: VMT_CANNOT_UPDATE_ILLEGAL_FIELD=Failed updating the properties of the
VM template.
Line 131: DIRECTORY_GROUP_CANNOT_REMOVE_DIRECTORY_GROUP_ATTACHED_TO_VM=Cannot
remove Directory Group. Detach Directory Group from VM first.
Line 132: VM_NOT_FOUND=VM not found
Line 133: ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is
previewing a Snapshot.
Line 134: ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT=Cannot ${action}
${type}. The following VM activated disks are snapshots: ${diskAliases}, please
deactivate them and try again.
Done
Line 135: ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The
following disks are locked: ${diskAliases}. Please try again in a few minutes.
Line 136: ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The
following attached disks are in ILLEGAL status: ${diskAliases} - please remove
them and try again.
Line 137: ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action}
${type}. The following disks already exist: ${diskAliases}. Please import as a
clone.
Line 138: ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is
locked. Please try again in a few minutes.
Line 801: USER_CANNOT_BE_ADDED_TO_VM=User cannot be added to VM
Line 802: USER_CANNOT_BE_ADDED_TO_VM_POOL=User cannot be added to VM-Pool
Line 803: ACTION_TYPE_FAILED_DETECTED_PINNED_VMS=Cannot ${action} ${type}. The
following VMs are set to run specifically only on this Host: ${VmNames}.\nIn
order to ${action} ${type}, you need to remove the association between the VMs
and the Host (Using Edit VM properties).
Line 804: HOT_PLUG_IS_NOT_SUPPORTED=Activate/Deactivate while VM is running, is
only supported for Clusters of version 3.1 and above.
Line 805: HOT_PLUG_DISK_SNAPSHOT_IS_NOT_SUPPORTED=Hot-plug/Hot-unplug, is only
supported for Clusters of version 3.3 and above.
Done
Line 806: UNLINKING_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Link state is
set to 'Down' on the virtual machine's interface, this is not supported for
clusters of version ${clusterVersion}.
Line 807: NULL_NETWORK_IS_NOT_SUPPORTED=Cannot ${action} ${type}. There is no
network on the virtual machine's interface, this is not supported for clusters
of version ${clusterVersion}.
Line 808: HOT_VM_INTERFACE_UPDATE_IS_NOT_SUPPORTED=Cannot ${action} ${type}.
Updating the virtual machine interface while the virtual machine is running is
not supported for clusters of version ${clusterVersion}.
Line 809: CANNOT_PERFOM_HOT_UPDATE=Cannot ${action} ${type}. Updating some of
the properties is not supported while the interface is plugged into a running
virtual machine. Please un-plug the interface, update the properties, and then
plug it back.
....................................................
File packaging/dbscripts/all_disks_sp.sql
Line 61: WHERE vm_device.vm_id = v_vm_guid and boot = true;
Line 62: END; $procedure$
Line 63: LANGUAGE plpgsql;
Line 64:
Line 65:
Done
Line 66: -- Returns all the attachable disks in the storage pool
Line 67: -- If storage pool is ommited, all the attachable disks are retrurned.
Line 68: -- in case vm id is provided, returning all the disks in SP that are
not attached to the vm
Line 69: Create or replace FUNCTION
GetAllAttachableDisksByPoolId(v_storage_pool_id UUID, v_vm_id uuid, v_user_id
UUID, v_is_filtered BOOLEAN)
....................................................
File packaging/dbscripts/disk_images_sp.sql
Line 131: RETURNS SETOF images_storage_domain_view
Line 132: AS $procedure$
Line 133: BEGIN
Line 134: RETURN QUERY SELECT *
Line 135: FROM images_storage_domain_view images_storage_domain_view
I agree, i just copied it from the previous functions which uses the same
notion, i'll remove it.
Line 136: WHERE image_group_id = v_image_group_id
Line 137: AND vm_snapshot_id = v_vm_snapshot_id;
Line 138: END; $procedure$
Line 139: LANGUAGE plpgsql;
....................................................
File
packaging/dbscripts/upgrade/03_03_0710_add_snapshot_id_column_to_vm_device.sql
Line 1: select fn_db_add_column('vm_device', 'snapshot_id', 'UUID REFERENCES
SNAPSHOTS(snapshot_id) ON DELETE CASCADE DEFAULT NULL');
plugged_snapshot_id isn't good enough IMO as there are plugged and unplugged
devices so unless someone has better suggestion i prefer to keep it as is
....................................................
File packaging/dbscripts/vm_device_sp.sql
Line 152: AND (v_vm_id IS NULL OR vm_id = v_vm_id);
Line 153: END; $procedure$
Line 154: LANGUAGE plpgsql;
Line 155:
Line 156: Create or replace FUNCTION
GetVmDevicesBySnapshotIdExcludingVm(v_snapshot_id UUID, v_vm_id UUID)
It's needed for further revision to find out if snapshot devices are used by
other vms
Line 157: RETURNS SETOF vm_device_view
Line 158: AS $procedure$
Line 159: BEGIN
Line 160: RETURN QUERY
--
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: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
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
