rbd: hide some snapshot info details

Pass the snapshot id and snapshot size rather than an index to __rbd_add_snap_dev() to specify values for a new snapshot.

Signed-off-by Alex Elder <[email protected]>

---
  drivers/block/rbd.c |   14 ++++++++------
  1 file changed, 8 insertions(+), 6 deletions(-)

drivers/block/rbd.c +2 -1

index d73edb1..2b048e3 100644
@@ -2031,7 +2031,8 @@ static int rbd_register_snap_dev(struct rbd_snap
 *snap,
  }
 
  static struct rbd_snap *__rbd_add_snap_dev(struct rbd_device *rbd_dev,
-					      int i, const char *name)
+						const char *snap_name,
+						u64 snap_id, u64 snap_size)
  {
  	struct rbd_snap *snap;

int ret;

drivers/block/rbd.c +6 -5

@@ -2041,12 +2042,12 @@ static struct rbd_snap
 *__rbd_add_snap_dev(struct rbd_device *rbd_dev,
  		return ERR_PTR(-ENOMEM);
 
  	ret = -ENOMEM;
-	snap->name = kstrdup(name, GFP_KERNEL);
+	snap->name = kstrdup(snap_name, GFP_KERNEL);
  	if (!snap->name)
  		goto err;
 
-	snap->size = rbd_dev->header.snap_sizes[i];
-	snap->id = rbd_dev->header.snapc->snaps[i];
+	snap->id = snap_id;
+	snap->size = snap_size;
 
  	return snap;

@@ -2111,12 +2112,13 @@ static int rbd_dev_snaps_update(struct
 rbd_device *rbd_dev)
  		dout("entry %u: snap_id = %llu\n", (unsigned int) snap_count,
  			(unsigned long long) snap_id);
  		if (!snap || (snap_id != CEPH_NOSNAP && snap->id < snap_id)) {
+			struct rbd_image_header	*header = &rbd_dev->header;
  			struct rbd_snap *new_snap;
 
  			/* We haven't seen this snapshot before */
 
-			new_snap = __rbd_add_snap_dev(rbd_dev, index,
-							snap_name);
+			new_snap = __rbd_add_snap_dev(rbd_dev, snap_name,
+					snap_id, header->snap_sizes[index]);
  			if (IS_ERR(new_snap)) {
  				int err = PTR_ERR(new_snap);

Record the features values for each rbd image and each of its snapshots. This is really something that only becomes meaningful for version 2 images, so this is just putting in place code that will form common infrastructure.

It may be useful to expand the sysfs entries--and therefore the information we maintain--for the image and for each snapshot. But I'm going to hold off doing that until we start making active use of the feature bits.

Signed-off-by Alex Elder <[email protected]>

---
  Documentation/ABI/testing/sysfs-bus-rbd |    7 +++++
  drivers/block/rbd.c                     |   43
+++++++++++++++++++++++++++++--
  2 files changed, 48 insertions(+), 2 deletions(-)

Documentation/ABI/testing/sysfs-bus-rbd +7 -0

index 6fe4224..1cf2adf 100644
@@ -25,6 +25,10 @@ client_id
 
  	The ceph unique client id that was assigned for this specific session.
 
+features
+
+	A hexadecimal encoding of the feature bits for this image.
+
  major
 
  	The block device major number.

@@ -78,4 +82,7 @@ snap_size
 
  	The size of the image when this snapshot was taken.
 
+snap_features
+
+	A hexadecimal encoding of the feature bits for this snapshot.
 

drivers/block/rbd.c +4 -0

index 2b048e3..c62d3f4 100644
@@ -85,6 +85,7 @@
  struct rbd_image_header {
  	/* These four fields never change for a given rbd image */
  	char *object_prefix;
+	u64 features;
  	__u8 obj_order;
  	__u8 crypt_type;
  	__u8 comp_type;

@@ -148,12 +149,14 @@ struct rbd_snap {
  	u64			size;
  	struct list_head	node;
  	u64			id;
+	u64			features;
  };
 
  struct rbd_mapping {
  	char                    *snap_name;
  	u64                     snap_id;
  	u64                     size;
+	u64                     features;
  	bool                    snap_exists;
  	bool			read_only;
  };

@@ -590,6 +593,7 @@ static int rbd_header_from_disk(struct
 rbd_image_header *header,
  		header->snap_sizes = NULL;
  	}
 
+	header->features = 0;	/* No features support in v1 images */
  	header->obj_order = ondisk->options.order;
  	header->crypt_type = ondisk->options.crypt_type;

  	header->comp_type = ondisk->options.comp_type;

drivers/block/rbd.c +1 -0

@@ -632,6 +636,7 @@ static int snap_by_name(struct rbd_device *rbd_dev,
 const char *snap_name)
  		if (!strcmp(snap_name, snap->name)) {
  			rbd_dev->mapping.snap_id = snap->id;
  			rbd_dev->mapping.size = snap->size;
+			rbd_dev->mapping.features = snap->features;
 
  			return 0;

  		}

drivers/block/rbd.c +1 -0

@@ -648,6 +653,7 @@ static int rbd_dev_set_mapping(struct rbd_device
 *rbd_dev, char *snap_name)
  		    sizeof (RBD_SNAP_HEAD_NAME))) {
  		rbd_dev->mapping.snap_id = CEPH_NOSNAP;
  		rbd_dev->mapping.size = rbd_dev->header.image_size;
+		rbd_dev->mapping.features = rbd_dev->header.features;
  		rbd_dev->mapping.snap_exists = false;
  		rbd_dev->mapping.read_only = rbd_dev->rbd_opts.read_only;

  		ret = 0;

drivers/block/rbd.c +19 -0

@@ -1835,6 +1841,19 @@ static ssize_t rbd_size_show(struct device *dev,
  	return sprintf(buf, "%llu\n", (unsigned long long) size * SECTOR_SIZE);
  }
 
+/*
+ * Note this shows the features for whatever's mapped, which is not
+ * necessarily the base image.
+ */
+static ssize_t rbd_features_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
+
+	return sprintf(buf, "0x%016llx\n",
+			(unsigned long long) rbd_dev->mapping.features);
+}
+
  static ssize_t rbd_major_show(struct device *dev,
  			      struct device_attribute *attr, char *buf)
  {

@@ -1884,6 +1903,10 @@ static ssize_t rbd_image_id_show(struct device *dev,
  	return sprintf(buf, "%s\n", rbd_dev->image_id);
  }
 
+/*
+ * Shows the name of the currently-mapped snapshot (or
+ * RBD_SNAP_HEAD_NAME for the base image).
+ */
  static ssize_t rbd_snap_show(struct device *dev,
  			     struct device_attribute *attr,
  			     char *buf)

@@ -1907,6 +1930,7 @@ static ssize_t rbd_image_refresh(struct device *dev,
  }
 
  static DEVICE_ATTR(size, S_IRUGO, rbd_size_show, NULL);
+static DEVICE_ATTR(features, S_IRUGO, rbd_features_show, NULL);
  static DEVICE_ATTR(major, S_IRUGO, rbd_major_show, NULL);
  static DEVICE_ATTR(client_id, S_IRUGO, rbd_client_id_show, NULL);
  static DEVICE_ATTR(pool, S_IRUGO, rbd_pool_show, NULL);

@@ -1918,6 +1942,7 @@ static DEVICE_ATTR(current_snap, S_IRUGO,
 rbd_snap_show, NULL);
 
  static struct attribute *rbd_attrs[] = {
  	&dev_attr_size.attr,
+	&dev_attr_features.attr,
  	&dev_attr_major.attr,
  	&dev_attr_client_id.attr,

&dev_attr_pool.attr,

drivers/block/rbd.c +14 -1

@@ -1971,12 +1996,24 @@ static ssize_t rbd_snap_id_show(struct device *dev,
  	return sprintf(buf, "%llu\n", (unsigned long long)snap->id);
  }
 
+static ssize_t rbd_snap_features_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct rbd_snap *snap = container_of(dev, struct rbd_snap, dev);
+
+	return sprintf(buf, "0x%016llx\n",
+			(unsigned long long) snap->features);
+}
+
  static DEVICE_ATTR(snap_size, S_IRUGO, rbd_snap_size_show, NULL);
  static DEVICE_ATTR(snap_id, S_IRUGO, rbd_snap_id_show, NULL);
+static DEVICE_ATTR(snap_features, S_IRUGO, rbd_snap_features_show, NULL);
 
  static struct attribute *rbd_snap_attrs[] = {
  	&dev_attr_snap_size.attr,
  	&dev_attr_snap_id.attr,
+	&dev_attr_snap_features.attr,
  	NULL,
  };
 

@@ -2032,7 +2069,8 @@ static int rbd_register_snap_dev(struct rbd_snap
 *snap,
 
  static struct rbd_snap *__rbd_add_snap_dev(struct rbd_device *rbd_dev,
  						const char *snap_name,
-						u64 snap_id, u64 snap_size)
+						u64 snap_id, u64 snap_size,
+						u64 snap_features)
  {
  	struct rbd_snap *snap;

int ret;

drivers/block/rbd.c +2 -1

@@ -2048,6 +2086,7 @@ static struct rbd_snap *__rbd_add_snap_dev(struct
 rbd_device *rbd_dev,
 
  	snap->id = snap_id;
  	snap->size = snap_size;
+	snap->features = snap_features;
 
  	return snap;

@@ -2118,7 +2157,7 @@ static int rbd_dev_snaps_update(struct rbd_device
 *rbd_dev)
  			/* We haven't seen this snapshot before */
 
  			new_snap = __rbd_add_snap_dev(rbd_dev, snap_name,
-					snap_id, header->snap_sizes[index]);
+					snap_id, header->snap_sizes[index], 0);
  			if (IS_ERR(new_snap)) {
  				int err = PTR_ERR(new_snap);

Create a function that encapsulates looking up the name, size and features related to a given snapshot, which is indicated by its index in an rbd device's snapshot context array of snapshot ids.

This interface will be used to hide differences between the format 1 and format 2 images.

At the moment this (looking up the name anyway) is slightly less efficient than what's done currently, but we may be able to optimize this a bit later on by cacheing the last lookup if it proves to be a problem.

Signed-off-by Alex Elder <[email protected]>

---
  drivers/block/rbd.c |   36 ++++++++++++++++++++++++++++++------
  1 file changed, 30 insertions(+), 6 deletions(-)

drivers/block/rbd.c +19 -1

index c62d3f4..e6b8fa6 100644
@@ -2097,6 +2097,25 @@ err:
  	return ERR_PTR(ret);
  }
 
+static char *rbd_dev_v1_snap_info(struct rbd_device *rbd_dev, u32 which,
+		u64 *snap_size, u64 *snap_features)
+{
+	char *snap_name;
+
+	rbd_assert(which < rbd_dev->header.snapc->num_snaps);
+
+	*snap_size = rbd_dev->header.snap_sizes[which];
+	*snap_features = 0;	/* No features for v1 */
+
+	/* Skip over names until we find the one we are looking for */
+
+	snap_name = rbd_dev->header.snap_names;
+	while (which--)
+		snap_name += strlen(snap_name) + 1;
+
+	return snap_name;
+}
+
  /*
   * Scan the rbd device's current snapshot list and compare it to the
   * newly-received snapshot context.  Remove any existing snapshots

@@ -2113,7 +2132,6 @@ static int rbd_dev_snaps_update(struct rbd_device
 *rbd_dev)
  {
  	struct ceph_snap_context *snapc = rbd_dev->header.snapc;
  	const u32 snap_count = snapc->num_snaps;
-	char *snap_name = rbd_dev->header.snap_names;
  	struct list_head *head = &rbd_dev->snaps;
  	struct list_head *links = head->next;

  	u32 index = 0;

drivers/block/rbd.c +3 -0

@@ -2122,6 +2140,9 @@ static int rbd_dev_snaps_update(struct rbd_device
 *rbd_dev)
  	while (index < snap_count || links != head) {
  		u64 snap_id;
  		struct rbd_snap *snap;
+		char *snap_name;
+		u64 snap_size = 0;
+		u64 snap_features = 0;
 
  		snap_id = index < snap_count ? snapc->snaps[index]

: CEPH_NOSNAP;

drivers/block/rbd.c +8 -5

@@ -2148,16 +2169,20 @@ static int rbd_dev_snaps_update(struct
 rbd_device *rbd_dev)
  			continue;
  		}
 
+		snap_name = rbd_dev_v1_snap_info(rbd_dev, index,
+						&snap_size, &snap_features);
+		if (IS_ERR(snap_name))
+			return PTR_ERR(snap_name);
+
  		dout("entry %u: snap_id = %llu\n", (unsigned int) snap_count,
  			(unsigned long long) snap_id);
  		if (!snap || (snap_id != CEPH_NOSNAP && snap->id < snap_id)) {
-			struct rbd_image_header	*header = &rbd_dev->header;
  			struct rbd_snap *new_snap;
 
  			/* We haven't seen this snapshot before */
 
  			new_snap = __rbd_add_snap_dev(rbd_dev, snap_name,
-					snap_id, header->snap_sizes[index], 0);
+					snap_id, snap_size, snap_features);
  			if (IS_ERR(new_snap)) {
  				int err = PTR_ERR(new_snap);

@@ -2178,9 +2203,9 @@ static int rbd_dev_snaps_update(struct rbd_device
 *rbd_dev)
 
  			dout("  already present\n");
 
-			rbd_assert(snap->size ==
-					rbd_dev->header.snap_sizes[index]);
+			rbd_assert(snap->size == snap_size);
  			rbd_assert(!strcmp(snap->name, snap_name));
+			rbd_assert(snap->features == snap_features);
 
  			/* Done with this list entry; advance */

@@ -2190,7 +2215,6 @@ static int rbd_dev_snaps_update(struct rbd_device
 *rbd_dev)
  		/* Advance to the next entry in the snapshot context */
 
  		index++;
-		snap_name += strlen(snap_name) + 1;
  	}
  	dout("%s: done\n", __func__);