From 867b131e09a43ba7ed36267dcdec751abd652f14 Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Mon, 9 Sep 2013 18:47:00 -0400 Subject: [PATCH] GlusterFS: Copy snap from correct source file The GlusterFS driver's copy_volume_from_snapshot was previously not parsing the snapshot information to determine which backing file to copy from, instead copying from the active file at all times. This would result in the wrong data being supplied when a volume is cloned from a snapshot. Add a test for the copy_volume_from_snapshot method. Closes-Bug: #1222907 Change-Id: Ib829ca1a6812b61845f3b2eb9c5507779fa6ec15 --- cinder/tests/test_glusterfs.py | 51 ++++++++++++++++++++++++++++++ cinder/volume/drivers/glusterfs.py | 21 ++++++++++-- 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py index 4f6c7aec6..ef34fd463 100644 --- a/cinder/tests/test_glusterfs.py +++ b/cinder/tests/test_glusterfs.py @@ -1435,6 +1435,57 @@ class GlusterFsDriverTestCase(test.TestCase): self.assertEqual(len(chain), 1) self.assertEqual(chain[0]['filename'], vol_filename) + def test_copy_volume_from_snapshot(self): + (mox, drv) = self._mox, self._driver + + mox.StubOutWithMock(image_utils, 'convert_image') + mox.StubOutWithMock(drv, '_read_info_file') + mox.StubOutWithMock(image_utils, 'qemu_img_info') + + dest_volume = self._simple_volume( + 'c1073000-0000-0000-0000-0000000c1073') + src_volume = self._simple_volume() + + vol_dir = os.path.join(self.TEST_MNT_POINT_BASE, + drv._get_hash_str(self.TEST_EXPORT1)) + src_vol_path = os.path.join(vol_dir, src_volume['name']) + dest_vol_path = os.path.join(vol_dir, dest_volume['name']) + info_path = os.path.join(vol_dir, src_volume['name']) + '.info' + + snapshot = {'volume_name': src_volume['name'], + 'name': 'clone-snap-%s' % src_volume['id'], + 'size': src_volume['size'], + 'volume_size': src_volume['size'], + 'volume_id': src_volume['id'], + 'id': 'tmp-snap-%s' % src_volume['id'], + 'volume': src_volume} + + snap_file = dest_volume['name'] + '.' + snapshot['id'] + snap_path = os.path.join(vol_dir, snap_file) + + size = dest_volume['size'] + + drv._read_info_file(info_path).AndReturn( + {'active': snap_file, + snapshot['id']: snap_file} + ) + + qemu_img_output = """image: %s + file format: raw + virtual size: 1.0G (1073741824 bytes) + disk size: 173K + backing file: %s + """ % (snap_file, src_volume['name']) + img_info = image_utils.QemuImgInfo(qemu_img_output) + + image_utils.qemu_img_info(snap_path).AndReturn(img_info) + + image_utils.convert_image(src_vol_path, dest_vol_path, 'raw') + + mox.ReplayAll() + + drv._copy_volume_from_snapshot(snapshot, dest_volume, size) + def test_create_volume_from_snapshot(self): (mox, drv) = self._mox, self._driver diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py index b16d5384c..96f12bf59 100644 --- a/cinder/volume/drivers/glusterfs.py +++ b/cinder/volume/drivers/glusterfs.py @@ -233,18 +233,29 @@ class GlusterfsDriver(nfs.RemoteFsDriver): 'vol': volume['id'], 'size': volume_size}) - path_to_disk = self._local_path_volume(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot['volume']) + snap_info = self._read_info_file(info_path) + vol_path = self._local_volume_dir(snapshot['volume']) + forward_file = snap_info[snapshot['id']] + forward_path = os.path.join(vol_path, forward_file) + + # Find the file which backs this file, which represents the point + # when this snapshot was created. + img_info = self._qemu_img_info(forward_path) + path_to_snap_img = os.path.join(vol_path, img_info.backing_file) path_to_new_vol = self._local_path_volume(volume) - LOG.debug(_("will copy from snapshot at %s") % path_to_disk) + LOG.debug(_("will copy from snapshot at %s") % path_to_snap_img) if self.configuration.glusterfs_qcow2_volumes: out_format = 'qcow2' else: out_format = 'raw' - image_utils.convert_image(path_to_disk, path_to_new_vol, out_format) + image_utils.convert_image(path_to_snap_img, + path_to_new_vol, + out_format) def delete_volume(self, volume): """Deletes a logical volume.""" @@ -287,6 +298,10 @@ class GlusterfsDriver(nfs.RemoteFsDriver): If the volume is attached, the VM will switch to this file as part of the snapshot process. + Note that volume-1234.aaaa represents changes after snapshot + 'aaaa' was created. So the data for snapshot 'aaaa' is actually + in the backing file(s) of volume-1234.aaaa. + This file has a qcow2 header recording the fact that volume-1234 is its backing file. Delta changes since the snapshot was created are stored in this file, and the backing file (volume-1234) does not -- 2.45.2