When reverting volume to smaller snapshot size is incorrect

[Jay Bryant]

It has been discovered that the original design for reverting to a snapshot was not supposed to allow reverting to a snapshot that was smaller than the volume. I.E. the volume was extended. LVM, however, resizes the snapshots and it should be safe to do so.

The code was implemented, however, in a way that it doesn't update the volume size when reverting to the smaller snapshot. This needs to be fixed.

The spec should probably also be updated to match the actual functionality.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/611491

[Yikun Jiang (yikunkero)]

We have metioned this case in original design [1], there is a common implementation in [2] (if driver not support revert_to_snapshot will call it)

this common implementation will first create a temp volume from snapshot, and then copy temp volume to origin volume [3]

Although If volume.size > snapshot.size, it's a safe case, but we still need shrink volume to snapshot.size (which is not supported in volume dirver)

So, I prefer to add this size check before we do the real revert action.

Link:
[1] https://review.openstack.org/#/c/316540/27/specs/pike/cinder-volume-revert-by-snapshot.rst@125
[2] https://github.com/openstack/cinder/blob/2ec51c3/cinder/volume/manager.py#L934
[3] https://github.com/openstack/cinder/blob/2ec51c3/cinder/volume/manager.py#L911

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/611491
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=bfc27c9abaf366b6534ccb9e032cc26fd3ad3b65
Submitter: Zuul
Branch: master

commit bfc27c9abaf366b6534ccb9e032cc26fd3ad3b65
Author: Yikun Jiang <email address hidden>
Date: Thu Oct 18 11:47:29 2018 +0800

    Forbidden to revert volume to a different size snapshot

    As mentioned from original design:
    "As we support extend volumes at present, we could meet the
    case that snapshot is smaller than volume when reverting to
    snapshot and it's obviously safe to revert in that case. But we
    will still restrict to revert the volume which current volume size
    is equal to snapshot, cause we don't support shrink volume yet
    (that ability will be used in the generic method, if the driver
    don't support revert to snapshot)."

    So, we need add a size check before we revert a volume to a
    snapshot. If the volume size is not equal to its latest
    snapshot size, it will be raise a HTTPBadRequest(400).

    Change-Id: Ib3cc95a10870822b9f597d66007699fb40908b61
    Closes-bug: #1798503

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/rocky
Review: https://review.openstack.org/613664

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/613664
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=8ef380c8aef33615adcf435da6c29cb151bc3ee4
Submitter: Zuul
Branch: stable/rocky

commit 8ef380c8aef33615adcf435da6c29cb151bc3ee4
Author: Yikun Jiang <email address hidden>
Date: Thu Oct 18 11:47:29 2018 +0800

    Forbidden to revert volume to a different size snapshot

    As mentioned from original design:
    "As we support extend volumes at present, we could meet the
    case that snapshot is smaller than volume when reverting to
    snapshot and it's obviously safe to revert in that case. But we
    will still restrict to revert the volume which current volume size
    is equal to snapshot, cause we don't support shrink volume yet
    (that ability will be used in the generic method, if the driver
    don't support revert to snapshot)."

    So, we need add a size check before we revert a volume to a
    snapshot. If the volume size is not equal to its latest
    snapshot size, it will be raise a HTTPBadRequest(400).

    Change-Id: Ib3cc95a10870822b9f597d66007699fb40908b61
    Closes-bug: #1798503
    (cherry picked from commit bfc27c9abaf366b6534ccb9e032cc26fd3ad3b65)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 13.0.2 release.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/644959

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 14.0.0.0rc1 release candidate.

[Brian Rosmaita (brian-rosmaita)]

Note: we decided not to backport this to stable/queens. See the discussion on the review:
https://review.openstack.org/644959

[OpenStack Infra (hudson-openstack)]

Change abandoned by Brian Rosmaita (<email address hidden>) on branch: stable/queens
Review: https://review.opendev.org/644959
Reason: We agreed not to accept this backport.