Description of problem: When taking a snapshot using the REST API, there's no validation for the disk's id(s) supplied by the user. Therefore, in case of supplying disks' ids which are not part of the snapshot, they are silently been ignored instead of raising an error. Version-Release number of selected component (if applicable): Reproduced for 4.2 version, although according to the code, was probably present before that. How reproducible: 100% Steps to Reproduce: 1. Take a snapshot for a VM with a disk. 2. Using the REST API, run the following: Method: ------- POST URL: ---- https://xxxx.xxxx.xxxx.xxxx.com/ovirt-engine/api/vms/<VM_ID>/snapshots/<SNAPSHOT_ID>/restore Body: ----- <action> <disks> <disk id = "<SOME_NEW_GUID>"></disk> </disks> </action> Actual results: The disks are silently been ignored. Expected results: Error: The following disks IDs are not part of the snapshot. Additional info: -
The proposed solution is based on a refactoring proposed in patch https://gerrit.ovirt.org/#/c/82017/. While this refactoring looks like a step in the right direction, it's in the very least incomplete, and will result in lost functionality (full details in gerrit). Properly refactoring the command/API will require quite a bit of effort, and considering the fact that this issue has been present since the partial snapshot feature [1] was introduced in 3.4, and was never reported from the field, I'm pushing it out to 4.3. [1] https://www.ovirt.org/develop/release-management/features/storage/single-disk-snapshot/
In addition to the original scenario of the problem, another missing validation is for supplying duplicated disk ids: When the user supplies the same disk image more than once, the following error message appears on the engine log: (Checked on ovirt-engine-4.2-7684911dad) 2018-03-12 10:45:21,102+02 ERROR [org.ovirt.engine.core.bll.snapshots.RestoreFromSnapshotCommand] (default task-38) [38c09a80-e7ee-481d-ba5e-167b32910362] Command 'org.ovirt.engine.core.bll.snapshots.RestoreFromSnapshotCommand' failed: Duplicate key org.ovirt.engine.core.common.businessentities.storage.DiskImage@c63003ec 2018-03-12 10:45:21,102+02 ERROR [org.ovirt.engine.core.bll.snapshots.RestoreFromSnapshotCommand] (default task-38) [38c09a80-e7ee-481d-ba5e-167b32910362] Exception: java.lang.IllegalStateException: Duplicate key org.ovirt.engine.core.common.businessentities.storage.DiskImage@c63003ec Therefore, although this bug is targeted for 4.3.0, backporting patch http://gerrit.ovirt.org/88646 is needed.
(In reply to shani from comment #2) > Therefore, although this bug is targeted for 4.3.0, backporting patch > http://gerrit.ovirt.org/88646 is needed. Let's split the difference, and just retarget it to 4.2.z.
Shani, can you add some text explaining what was added here?
As a part of disks' validation, while trying to preview/restore a snapshot, I had to revert some older patches which used a Set<Guid> back to a Lisk<DiskImage>. The added validation uses a new DB query which trying to retrieve the diskImage with the disk id and image_id specified by the user on REST API (or a 404 error if such doesn't exist) as part of patch 88355. Also, there's a new validation of the disk ids' uniqueness, so no disk id will be supplied more than once as patched on 88646. To make it easier to validate, the diskImage field on DiskImageValidator was changed from an iterable to a collection.
Verified with the following code: ---------------------------------------- ovirt-engine-4.2.2.6-0.1.el7.noarch vdsm-4.20.23-1.el7ev.x86_64 Verified with the following scenario: ------------------------------------------- Url https://storage-ge-10.scl.lab.tlv.redhat.com/ovirt-engine/api/vms/9f00362d-3712-44e5-b846-7e6298690ad7/snapshots/967d52c6-0aab-4947-8550-e586e541dfe2/restore Method POST Body <action> <disks> <disk id = "a2487ec9-53db-42d6-b112-687dd8ddd846"> </disk> </disks> </action> Response <?xml version="1.0" encoding="UTF-8" standalone="yes"?> <fault> <detail>Entity not found: GetDiskImageByDiskAndImageIds: disk id=a2487ec9-53db-42d6-b112-687dd8ddd846, image_id=00000000-0000-0000-0000-000000000000</detail> <reason>Operation Failed</reason> </fault> Moving to VERIFIED
This bugzilla is included in oVirt 4.2.2 release, published on March 28th 2018. Since the problem described in this bug report should be resolved in oVirt 4.2.2 release, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report.