Re: [PATCH 3/5] dm: linear: Add linear target

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 8/28/25 5:05 PM, Tobias Waldekranz wrote:
> This target allows you to map in a region of another device as a
> contiguous range of blocks of a dm device.
> 
> This is the basic building block used by LVM to stitch together
> logical volumes from one or more ranges of blocks from one or more
> physical disks.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  drivers/block/dm/Kconfig     |   7 ++
>  drivers/block/dm/Makefile    |   1 +
>  drivers/block/dm/dm-linear.c | 123 +++++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
>  create mode 100644 drivers/block/dm/dm-linear.c
> 
> diff --git a/drivers/block/dm/Kconfig b/drivers/block/dm/Kconfig
> index f64c69e03d..e763e530d3 100644
> --- a/drivers/block/dm/Kconfig
> +++ b/drivers/block/dm/Kconfig
> @@ -5,3 +5,10 @@ menuconfig DM_BLK
>  	  other data sources. Modeled after, and intended to be
>  	  compatible with, the Linux kernel's device mapper subsystem.
>  
> +config DM_BLK_LINEAR
> +        bool "Linear target"
> +	depends on DM_BLK

Tab/Spaces mix up.

> +        help
> +	  Maps a contiguous region of an existing device into a dm
> +	  device.
> +
> diff --git a/drivers/block/dm/Makefile b/drivers/block/dm/Makefile
> index 9c045087c0..f52d19f9c4 100644
> --- a/drivers/block/dm/Makefile
> +++ b/drivers/block/dm/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_DM_BLK) += dm-core.o
> +obj-$(CONFIG_DM_BLK_LINEAR) += dm-linear.o
> diff --git a/drivers/block/dm/dm-linear.c b/drivers/block/dm/dm-linear.c
> new file mode 100644
> index 0000000000..d3415814cb
> --- /dev/null
> +++ b/drivers/block/dm/dm-linear.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// SPDX-FileCopyrightText: © 2025 Tobias Waldekranz <tobias@waldekranz.com>, Wires
> +
> +#include <block.h>
> +#include <common.h>

Nitpick: If possible, I'd like to avoid common.h in new code. It's a lot
of unneeded stuff in there.

> +#include <disks.h>
> +#include <fcntl.h>
> +
> +#include "dm-target.h"
> +
> +struct dm_linear {
> +	struct cdev *cdev;
> +	loff_t offset;
> +};
> +
> +static int dm_linear_read(struct dm_target *ti, void *buf,
> +			  sector_t block, blkcnt_t num_blocks)
> +{
> +	struct dm_linear *l = ti->private;
> +
> +	block <<= SECTOR_SHIFT;
> +	num_blocks <<= SECTOR_SHIFT;
> +
> +	if (cdev_read(l->cdev, buf, num_blocks, l->offset + block, 0) < num_blocks)
> +		return -EIO;

Why not propagate the underlying error code if it's negative?

> +
> +	return 0;
> +}
> +
> +static int dm_linear_write(struct dm_target *ti, const void *buf,
> +			   sector_t block, blkcnt_t num_blocks)
> +{
> +	struct dm_linear *l = ti->private;
> +
> +	block <<= SECTOR_SHIFT;
> +	num_blocks <<= SECTOR_SHIFT;
> +
> +	if (cdev_write(l->cdev, buf, num_blocks, l->offset + block, 0) < num_blocks)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int dm_linear_create(struct dm_target *ti, unsigned int argc, char **argv)
> +{
> +	struct dm_linear *l;
> +	int err;
> +
> +	if (argc != 2) {
> +		dm_target_err(ti, "Expected 2 arguments (\"<dev> <offset>\"), got %u\n",
> +			      argc);
> +		err = -EINVAL;
> +		goto err;
> +	}
> +
> +	l = xzalloc(sizeof(*l));
> +	ti->private = l;
> +
> +	if (kstrtoull(argv[1], 0, &l->offset)) {
> +		dm_target_err(ti, "Invalid offset: \"%s\"\n", argv[1]);
> +		err = -EINVAL;
> +		goto err_free;
> +	}
> +	l->offset <<= SECTOR_SHIFT;
> +
> +	l->cdev = cdev_open_by_path_name(argv[0], O_RDWR);
> +	if (!l->cdev) {
> +		dm_target_err(ti, "Cannot open device %s: %m\n", argv[0]);
> +		err = -ENODEV;
> +		goto err_free;
> +	}
> +
> +	l->cdev = cdev_readlink(l->cdev);
> +
> +	if ((ti->size << SECTOR_SHIFT) > (l->cdev->size - l->offset)) {
> +		dm_target_err(ti, "%s is too small to map %llu blocks at %llu, %llu available\n",
> +			      argv[0], ti->size, l->offset >> SECTOR_SHIFT,
> +			      (l->cdev->size - l->offset) >> SECTOR_SHIFT);
> +		err = -ENOSPC;
> +		goto err_close;
> +	}
> +
> +	return 0;
> +
> +err_close:
> +	cdev_close(l->cdev);
> +err_free:
> +	free(l);
> +err:
> +	return err;
> +}
> +
> +static int dm_linear_destroy(struct dm_target *ti)
> +{
> +	struct dm_linear *l = ti->private;
> +
> +	cdev_close(l->cdev);
> +	free(l);
> +	return 0;
> +}
> +
> +static char *dm_linear_asprint(struct dm_target *ti)
> +{
> +	struct dm_linear *l = ti->private;
> +
> +	return xasprintf("dev:%s offset:%llu",
> +			 cdev_name(l->cdev), l->offset >> SECTOR_SHIFT);
> +}
> +
> +static struct dm_target_ops dm_linear_ops = {
> +	.name = "linear",
> +	.asprint = dm_linear_asprint,
> +	.create = dm_linear_create,
> +	.destroy = dm_linear_destroy,
> +	.read = dm_linear_read,
> +	.write = dm_linear_write,
> +};
> +
> +static int __init dm_linear_init(void)
> +{
> +	return dm_target_register(&dm_linear_ops);
> +}
> +device_initcall(dm_linear_init);

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |