RBD create volume from encrypted snapshot can not be used

Bug #1852168 reported by yenai
24
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Medium
Sofia Enriquez
Victoria
New
Low
Sofia Enriquez
Wallaby
Fix Committed
Medium
Sofia Enriquez
Xena
Fix Committed
Medium
Sofia Enriquez

Bug Description

Steps:
1. Create en encrypted Ceph RBD volume(Vol-A).
2. Create a snapshot for this volume.
3. Create a volume(Vol-B) form this snapshot.
4. Attach this volume(Vol-B) to a VM.

Attach successfully, but in the VM, we can not use this volume, the msg in the dmesg:
bad geometry: block count xxx exceeds size of device.
Obviously, the size of the volume is out of order.

Ceph RBD encrypted volume is created by:
qemu-img create -f luks -o cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64 --object secret,id=luks_sec,format=raw,file=/var/lib/cinder/conversion/tmpvroiaf -o key-secret=luks_sec /var/lib/cinder/conversion/tmpXx6k6O 1024M

Then I do it myself:
# echo 'bae3516cc1c0eb18b05440eba8012a4a880a2ee04d584a9c1579445e675b12defdc716ec' > secret
# qemu-img create -f luks -o cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64 --object secret,id=luks_sec,format=raw,file=secret -o key-secret=luks_sec secret-file 1024M
Formatting 'secret-file', fmt=luks size=1073741824 key-secret=luks_sec cipher-alg=aes-256 cipher-mode=xts ivgen-alg=plain64

# qemu-img info secret-file
image: secret-file
file format: luks
virtual size: 1.0G (1073741824 bytes)
disk size: 256K
encrypted: yes
Format specific information:
    ivgen alg: plain64
    hash alg: sha256
    cipher alg: aes-256
    uuid: 0666ed63-cf74-4e09-9e16-79beff7ba287
    cipher mode: xts
    slots:
    ......

# ls -lh secret-file
-rw-r--r--. 1 root root 1.1G Sep 14 18:09 secret-file
# ls -lh secret-file --block-size=M
-rw-r--r--. 1 root root 1026M Sep 14 18:09 secret-file

The encrypted Ceph RBD volume info on Ceph is:
# rbd info volumes/volume-48247a47-1c1a-4dc1-837b-0e204636520e
rbd image 'volume-48247a47-1c1a-4dc1-837b-0e204636520e':
        size 1025 MB in 257 objects
        order 22 (4096 kB objects)
        block_name_prefix: rbd_data.1210c876b8b456
        format: 2
        features: layering, exclusive-lock, object-map, fast-diff, deep-flatten
        flags:

When do [3. Create a volume(Vol-B) form this snapshot.], what we do in Ceph RBD Volume Driver is:
    def _resize(self, volume, **kwargs):
        size = kwargs.get('size', None)
        if not size:
            size = int(volume.size) * units.Gi

        with RBDVolumeProxy(self, volume.name) as vol:
            vol.resize(size) ★★★★★

    def create_volume_from_snapshot(self, volume, snapshot):
        """Creates a volume from a snapshot."""
        volume_update = self._clone(volume, self.configuration.rbd_pool,
                                    snapshot.volume_name, snapshot.name)
        if self.configuration.rbd_flatten_volume_from_snapshot:
            self._flatten(self.configuration.rbd_pool, volume.name)
        if int(volume.size):
            self._resize(volume)
        return volume_update

So we can get the following conclusions:
1. The actuall size is larger than what we specify(volume size) by qemu-img create!
2. create_volume_from_snapshot will resize the target volume to the volume size(in this case, actually reduce the capacity of the volume), so the volume can't be used!

Some thoughts on the above conclusions:
Conclusion 1: What we specify in qemu-img create is not the final file size. qemu-img info's virtual size is not equal with the actual file size. This may be a bug in qemu-img which I can not fix it easily.
Conclusion 2: Why clone volume is JUST OK. I find the difference:
    def _extend_if_required(self, volume, src_vref):
        """Extends a volume if required

        In case src_vref size is smaller than the size if the requested
        new volume call _resize().
        """
        if volume.size != src_vref.size: ★★★★★
            LOG.debug("resize volume '%(dst_vol)s' from %(src_size)d to "
                      "%(dst_size)d",
                      {'dst_vol': volume.name, 'src_size': src_vref.size,
                       'dst_size': volume.size})
            self._resize(volume)
    So in this case, I will fix it by this.

yenai (yenai2008)
Changed in cinder:
assignee: nobody → yenai (yenai2008)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

Fix proposed to branch: master
Review: https://review.opendev.org/693772

Changed in cinder:
status: New → In Progress
Gorka Eguileor (gorka)
tags: added: rbd
Revision history for this message
Tzach Shefi (tshefi) wrote :

While reproducing this in our case volB remained usable (R/W).
lsblk reported volB was a bit larger than volA but that is all.
Wondering why we don't hit same result.

1G volume
aes-xts-256 encryption
image cirros

Revision history for this message
yenai (yenai2008) wrote :

@Tzach Shefi
On which backend?

Revision history for this message
Tzach Shefi (tshefi) wrote :

RBD, however safe to ignore comment problem found.

If volA is left empty before the snapshot
creating a FS and using VolB works fine without errors.

Duh moment..
We were not re-applying the FS from volA to a larger volB hence no problem.
Once we wrote data to VolA before the snapshot we hit the problem on VolB.

Revision history for this message
yenai (yenai2008) wrote :

@Gorka Eguileor
The qemu patch is:
https://github.com/qemu/qemu/commit/78368575a63df3ef95653024fa21a91d441b0c8d

static ssize_t block_crypto_init_func(QCryptoBlock *block,
                                      size_t headerlen,
                                      Error **errp,
                                      void *opaque)
{
    struct BlockCryptoCreateData *data = opaque;
    int ret;

    /* User provided size should reflect amount of space made ★★★★★
     * available to the guest, so we must take account of that ★★★★★
     * which will be used by the crypto header ★★★★★
     */
    data->size += headerlen;

Revision history for this message
Sofia Enriquez (lsofia-enriquez) wrote :

Hello, I'd like to reopen this because it's still a problem: https://bugs.launchpad.net/cinder/+bug/1922408

The root of this problem is that cinder force to truncate the destination size of the volume.[1]

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.

[1]https://opendev.org/openstack/cinder/src/branch/master/cinder/volume/drivers/rbd.py#L1050

Changed in cinder:
importance: Undecided → High
importance: High → Medium
assignee: yenai (yenai2008) → Sofia Enriquez (lsofia-enriquez)
Revision history for this message
Sofia Enriquez (lsofia-enriquez) wrote :
tags: added: encryption snapshot
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (master)

Change abandoned by "Brian Rosmaita <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/cinder/+/693772
Reason: Looks like this has been fixed by https://review.opendev.org/c/openstack/cinder/+/784623/

Eric Harney (eharney)
Changed in cinder:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.