On 07/24/2014 03:42 AM, Ilya Dryomov wrote:
> Make /sys/bus/rbd/devices/<id>/parent show the entire chain of parent
> images. While at it, kernel sprintf() doesn't return negative values,
> casting to unsigned long long is no longer necessary and there is no
> good reason to split into multiple sprintf() calls.
I like the use of a single snprintf() call, it is much less
cumbersome.
The reason I always cast u64 values to (unsigned long long)
in printf() calls is because on some 64-bit architectures,
u64 is defined as simply (unsigned long), so without the
cast they spit out warnings. I hate this, but it is reality
(see include/asm-generic/{int-l64.h,int-ll64.h}). You can
alternatively use %PRIu64 rather than %llu in your format
strings, but I think I hate that more. Anyway, if you want
to avoid warnings on all architectures you should fix that.
As another aside, I've been too timid to use the ?: conditional
expression without its middle operand. I have no objection
to it at all, I like it. I bring it up because I recently
got burned for using a gcc feature that wasn't supported
on older compilers (unnamed struct/union fields)--specifically
a version newer than gcc 3.2, which is the minimum supported
version for the kernel (see Documentation/Changes).
But fear not! That extension is supported in gcc 3.2:
https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Conditionals.html#Conditionals
Just FYI...
Reviewed-by: Alex Elder <elder@xxxxxxxxxx>
> Signed-off-by: Ilya Dryomov <ilya.dryomov@xxxxxxxxxxx>
> ---
> Documentation/ABI/testing/sysfs-bus-rbd | 4 +--
> drivers/block/rbd.c | 56 +++++++++++++------------------
> 2 files changed, 25 insertions(+), 35 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-rbd b/Documentation/ABI/testing/sysfs-bus-rbd
> index 501adc2a9ec7..2ddd680929d8 100644
> --- a/Documentation/ABI/testing/sysfs-bus-rbd
> +++ b/Documentation/ABI/testing/sysfs-bus-rbd
> @@ -94,5 +94,5 @@ current_snap
>
> parent
>
> - Information identifying the pool, image, and snapshot id for
> - the parent image in a layered rbd image (format 2 only).
> + Information identifying the chain of parent images in a layered rbd
> + image. Entries are separated by empty lines.
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 703b728e05fa..7847fbb949ff 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -3685,46 +3685,36 @@ static ssize_t rbd_snap_show(struct device *dev,
> }
>
> /*
> - * For an rbd v2 image, shows the pool id, image id, and snapshot id
> - * for the parent image. If there is no parent, simply shows
> - * "(no parent image)".
> + * For a v2 image, shows the chain of parent images, separated by empty
> + * lines. For v1 images or if there is no parent, shows "(no parent
> + * image)".
> */
> static ssize_t rbd_parent_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> + struct device_attribute *attr,
> + char *buf)
> {
> struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
> - struct rbd_spec *spec = rbd_dev->parent_spec;
> - int count;
> - char *bufp = buf;
> + ssize_t count = 0;
>
> - if (!spec)
> + if (!rbd_dev->parent)
> return sprintf(buf, "(no parent image)\n");
>
> - count = sprintf(bufp, "pool_id %llu\npool_name %s\n",
> - (unsigned long long) spec->pool_id, spec->pool_name);
> - if (count < 0)
> - return count;
> - bufp += count;
> -
> - count = sprintf(bufp, "image_id %s\nimage_name %s\n", spec->image_id,
> - spec->image_name ? spec->image_name : "(unknown)");
> - if (count < 0)
> - return count;
> - bufp += count;
> -
> - count = sprintf(bufp, "snap_id %llu\nsnap_name %s\n",
> - (unsigned long long) spec->snap_id, spec->snap_name);
> - if (count < 0)
> - return count;
> - bufp += count;
> -
> - count = sprintf(bufp, "overlap %llu\n", rbd_dev->parent_overlap);
> - if (count < 0)
> - return count;
> - bufp += count;
> -
> - return (ssize_t) (bufp - buf);
> + for ( ; rbd_dev->parent; rbd_dev = rbd_dev->parent) {
> + struct rbd_spec *spec = rbd_dev->parent_spec;
> +
> + count += sprintf(&buf[count], "%s"
> + "pool_id %llu\npool_name %s\n"
> + "image_id %s\nimage_name %s\n"
> + "snap_id %llu\nsnap_name %s\n"
> + "overlap %llu\n",
> + !count ? "" : "\n", /* first? */
> + spec->pool_id, spec->pool_name,
> + spec->image_id, spec->image_name ?: "(unknown)",
> + spec->snap_id, spec->snap_name,
> + rbd_dev->parent_overlap);
> + }
> +
> + return count;
> }
>
> static ssize_t rbd_image_refresh(struct device *dev,
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
