From 270763214776596b8a17b726e41a3719378a23af Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 10 Jun 2021 11:21:12 +0200 Subject: [PATCH] Doc: Improve name_id documentation Currently our code and contributor documentation doesn't mention the difference between the two different UUIDs we can have and how drivers should use them. This patch tries to provide a brief description about it in multiple places to reduce the number of bugs that happen around it. Change-Id: I946ead7547571ccd1b2df55ac1f3d4689d1f8089 --- cinder/objects/volume.py | 20 +++++++++++++++++++ cinder/volume/drivers/nfs.py | 8 ++++++++ doc/source/contributor/drivers.rst | 17 ++++++++++++++++ .../contributor/new_driver_checklist.rst | 1 + 4 files changed, 46 insertions(+) diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index e44babfb1e8..08d0e9860cf 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -75,7 +75,11 @@ class Volume(cleanable.CinderCleanableObject, base.CinderObject, # NOTE: When adding a field obj_make_compatible needs to be updated fields = { + # id is the user facing UUID that should be passed to API calls 'id': fields.UUIDField(), + # _name_id is the real volume's UUID that should be used by the driver + # when it is set. This is used when migrating a volume. Property + # name_id is provided for convenience. '_name_id': fields.UUIDField(nullable=True), 'ec2_id': fields.UUIDField(nullable=True), 'user_id': fields.StringField(nullable=True), @@ -154,6 +158,22 @@ class Volume(cleanable.CinderCleanableObject, base.CinderObject, @property def name_id(self): + """Actual volume's UUID for driver usage. + + There may be two different UUIDs for the same volume, the user facing + one, and the one the driver should be using. + + When a volume is created these two are the same, but when doing a + generic migration (create new volume, then copying data) they will be + different if we were unable to rename the new volume in the final + migration steps. + + So the volume will have been created using the new volume's UUID and + the driver will have to look for it using that UUID, but the user on + the other hand will keep referencing the volume with the original UUID. + + This property facilitates using the right UUID in the driver's code. + """ return self.id if not self._name_id else self._name_id @name_id.setter diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 2315d899d67..2ef4cb42e8d 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -473,6 +473,14 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed): This method should rename the back-end volume name(id) on the destination host back to its original name(id) on the source host. + Due to this renaming, inheriting drivers may need to also re-associate + other entities (such as backend QoS) with the new name. Alternatively + they could overwrite this method and raise NotImplemented. + + In any case, driver's code should always use the volume OVO's 'name_id' + field instead of 'id' to get the volume's real UUID, as the renaming + could fail. + :param ctxt: The context used to run the method update_migrated_volume :param volume: The original volume that was migrated to this backend :param new_volume: The migration volume object that was created on diff --git a/doc/source/contributor/drivers.rst b/doc/source/contributor/drivers.rst index ea96c2b1579..226d7f3d9e3 100644 --- a/doc/source/contributor/drivers.rst +++ b/doc/source/contributor/drivers.rst @@ -171,6 +171,23 @@ The LVM driver is our reference for all new driver implementations. The information below can provide additional documentation for the methods that volume drivers need to implement. +Volume ID +````````` + +Drivers should always get a volume's ID using the ``name_id`` attribute instead +of the ``id`` attribute. + +A Cinder volume may have two different UUIDs, a user facing one, and one the +driver should use. + +When a volume is created these two are the same, but when doing a generic +migration (create new volume, then copying data) they will be different if we +were unable to rename the new volume in the final migration steps. + +So the volume will have been created using the new volume's UUID and the driver +will have to look for it using that UUID, but the user on the other hand will +keep referencing the volume with the original UUID. + Base Driver Interface ````````````````````` The methods documented below are the minimum required interface for a volume diff --git a/doc/source/contributor/new_driver_checklist.rst b/doc/source/contributor/new_driver_checklist.rst index 8bd131e91f3..3307e202022 100644 --- a/doc/source/contributor/new_driver_checklist.rst +++ b/doc/source/contributor/new_driver_checklist.rst @@ -24,6 +24,7 @@ Review Checklist * Common gotchas + * Code should use ``volume.name_id`` instead of ``volume.id``. * Handles detach where ``connector == None`` for force detach * Create from snapshot and clone properly account for new volume size being larger than original volume size