create volume from snapshot will lose encrypted head when source volume is encrypted in RBD.

[haixin]

1:create an encrypted rbd volume A, which size is 2G.
2:create A's snapshot snap_A.
3:create new encrypted volume B form snap_A, the size of B is 2G too.

   we know that, currently create rbd encrypted volume, we use qume-img create to create an encrypted file, and then use rbd import into rbd cluster, and the encrypted head takes up the first 1 MB capacity, so in the rbd cluster, the real size of volume A is 2G + 1M.
   in rbd driver, at function def create_volume_from_snapshot(), after clone new volume B, will resize volume B to size of A. the code below:
   if int(volume.size)
       self._resize(volume)
  we can see that if volume.size is also 2G. then we resize B to 2G. this will lead to volume B lose it't encrypted head.

  if volume B lose encrypted head, bug volume B has encrypt_key_id in database, this will lead to volume B can not attach to VM.

[Sofia Enriquez (lsofia-enriquez)]

https://review.opendev.org/c/openstack/cinder/+/784623

[Brian Rosmaita (brian-rosmaita)]

Could you explain why you think the header is missing from the new volume? I'd think that what would happen is the new volume would contain the header plus the data, and the resize operation would truncate the data. But the header would still be there.

[haixin (haixin77)]

1:create an encrypted rbd volume A, which size is 2G.
2:create A's snapshot snap_A.
3:create new encrypted volume B form snap_A, the size of B is 2G too.

def create_volume_from_snap():
...
    if int(volume.size)
       self._resize(volume)
...

before exec _resize the size of B is 2G + 1M, after exec _resize. B will be truncate to 2G
,this operation would cut the header, after resize operation, use cryptsetup Dumpluks this volume
,will not found encryption header.

the possilbel resolve: https://review.opendev.org/c/openstack/cinder/+/784623

[Sofia Enriquez (lsofia-enriquez)]

Usually the source volume would be the same size or smaller than the destination volume and they *must* share the same volume-type.

In particular RBD workflow would be something like this:
A source luks volume would be 1026M, we write some data and create a snap from it. We like to create a new luks volume from a snapshot so the create_volume_from_snapshot() method performs a RBD clone first and then a resize if needed.

In addition the _clone() method creates a clone (copy-on-write child) of the parent snapshot. Object size will be identical to that of the parent image unless specified (we don't in cinder) so size will be the same as the parent snapshot.

If the desired size of the destination luks volume is 1G the create_volume_from_snapshot() won't perform any resize and will be 1026M as the parent. This solves bug #1922408 because we don't force it to resize and because of that we don't truncate the data anymore.

The second case scenario is when we would like to increase the size of the destination volume. As far as I test it this won't face the encryption header problem but we still need to calculate the difference size to provide the size that the user is expecting.

That's why the fix proposed calculate the new_size based on :
size difference = desired size - size of source volume
new size = current size + size difference

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/cinder/+/784623
Committed: https://opendev.org/openstack/cinder/commit/cf1c5252965e731d77af503529ca42e269c305c6
Submitter: "Zuul (22348)"
Branch: master

commit cf1c5252965e731d77af503529ca42e269c305c6
Author: haixin <email address hidden>
Date: Sat Apr 3 12:36:14 2021 +0800

    [rbd] Fix create encrypted volume from snapshot

    Usually the source volume would be the same size or smaller
    than the destination volume and they must share the same
    volume-type. In particular, when the destination volume is
    same size as the source volume, creating an encrypted volume
    from a snapshot of an encrypted volume truncates the data in
    the new volume.

    In order to fix this the RBD workflow would be something
    like this:
    A source luks volume would be 1026M, we write some data
    and create a snap from it. We like to create a new luks
    volume from a snapshot so the create_volume_from_snapshot()
    method performs a RBD clone first and then a resize if needed.

    In addition the _clone() method creates a clone
    (copy-on-write child) of the parent snapshot. Object size
    will be identical to that of the parent image unless specified
    (we don't in cinder) so size will be the same as the parent
    snapshot.

    If the desired size of the destination luks volume is 1G the
    create_volume_from_snapshot() won't perform any resize and
    will be 1026M as the parent. This solves bug #1922408 because
    we don't force it to resize and because of that we don't
    truncate the data anymore.

    The second case scenario is when we would like to increase
    the size of the destination volume. As far as I can tell this
    won't face the encryption header problem but we still need to
    calculate the difference size to provide the size that the
    user is expecting.

    That's why the fix proposed calculate the new_size based on:
    size difference = desired size - size of source volume
    new size = current size + size difference

    Closes-Bug: #1922408
    Co-Authored-By: Sofia Enriquez <email address hidden>
    Change-Id: I220b5e3b01d115262a8b1dd45758f0531aea0edf

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/cinder/+/803181

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/cinder/+/803181
Committed: https://opendev.org/openstack/cinder/commit/165122557d4976ae902ac6156d151214fec037ae
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 165122557d4976ae902ac6156d151214fec037ae
Author: haixin <email address hidden>
Date: Sat Apr 3 12:36:14 2021 +0800

    [rbd] Fix create encrypted volume from snapshot

    Usually the source volume would be the same size or smaller
    than the destination volume and they must share the same
    volume-type. In particular, when the destination volume is
    same size as the source volume, creating an encrypted volume
    from a snapshot of an encrypted volume truncates the data in
    the new volume.

    In order to fix this the RBD workflow would be something
    like this:
    A source luks volume would be 1026M, we write some data
    and create a snap from it. We like to create a new luks
    volume from a snapshot so the create_volume_from_snapshot()
    method performs a RBD clone first and then a resize if needed.

    In addition the _clone() method creates a clone
    (copy-on-write child) of the parent snapshot. Object size
    will be identical to that of the parent image unless specified
    (we don't in cinder) so size will be the same as the parent
    snapshot.

    If the desired size of the destination luks volume is 1G the
    create_volume_from_snapshot() won't perform any resize and
    will be 1026M as the parent. This solves bug #1922408 because
    we don't force it to resize and because of that we don't
    truncate the data anymore.

    The second case scenario is when we would like to increase
    the size of the destination volume. As far as I can tell this
    won't face the encryption header problem but we still need to
    calculate the difference size to provide the size that the
    user is expecting.

    That's why the fix proposed calculate the new_size based on:
    size difference = desired size - size of source volume
    new size = current size + size difference

    Closes-Bug: #1922408
    Co-Authored-By: Sofia Enriquez <email address hidden>
    Change-Id: I220b5e3b01d115262a8b1dd45758f0531aea0edf
    (cherry picked from commit cf1c5252965e731d77af503529ca42e269c305c6)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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