[RFC PATCH 3/3] discover: Create dm-snapshots for all eligible devices

[Cyril Bur]

On Mon, 2015-04-27 at 15:52 +1000, Samuel Mendoza-Jonas wrote:
> Create a dm-snapshot for any device with a valid filesystem. This
> protects the backing disk from any writes Petitboot may attempt to make
> regardless of mount type, such as journal replays.
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam.mj at au1.ibm.com>
> ---
>  discover/device-handler.c | 40 +++++++++++++++++++++++++++++-----------
>  discover/udev.c           | 20 ++++++++++++++++++--
>  2 files changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/discover/device-handler.c b/discover/device-handler.c
> index 69cd699..edf3a0c 100644
> --- a/discover/device-handler.c
> +++ b/discover/device-handler.c
> @@ -1279,11 +1279,14 @@ static const char *fs_parameters(unsigned int rw_flags, const char *fstype)
>  static bool check_existing_mount(struct discover_device *dev)
>  {
>  	struct stat devstat, mntstat;
> +	const char *device_path;
>  	struct mntent *mnt;
>  	FILE *fp;
>  	int rc;
>  
> -	rc = stat(dev->device_path, &devstat);
> +	device_path = dev->ramdisk ? dev->ramdisk->snapshot : dev->device_path;
> +
> +	rc = stat(device_path, &devstat);
>  	if (rc) {
>  		pb_debug("%s: stat failed: %s\n", __func__, strerror(errno));
>  		return false;
> @@ -1333,7 +1336,7 @@ static bool check_existing_mount(struct discover_device *dev)
>  
>  static int mount_device(struct discover_device *dev)
>  {
> -	const char *fstype;
> +	const char *fstype, *device_path;
>  	int rc;
>  
>  	if (!dev->device_path)
> @@ -1365,9 +1368,11 @@ static int mount_device(struct discover_device *dev)
>  		goto err_free;
>  	}
>  
> +	device_path = dev->ramdisk ? dev->ramdisk->snapshot : dev->device_path;
> +

You've done this a bunch of times, could you turn it into a simple
accessor?
static inline const char *get_device_path(struct discover_device *dev)
{
	return dev->ramdisk ? dev->ramdisk->snapshot : dev->device_path;
}

>  	pb_log("mounting device %s read-only\n", dev->device_path);
>  	errno = 0;
> -	rc = mount(dev->device_path, dev->mount_path, fstype,
> +	rc = mount(device_path, dev->mount_path, fstype,
>  			MS_RDONLY | MS_SILENT,
>  			fs_parameters(MS_RDONLY, fstype));
>  	if (!rc) {
> @@ -1378,7 +1383,10 @@ static int mount_device(struct discover_device *dev)
>  	}
>  
>  	pb_log("couldn't mount device %s: mount failed: %s\n",
> -			dev->device_path, strerror(errno));
> +			device_path, strerror(errno));
> +
> +	/* If mount fails clean up any snapshot */
> +	discover_device_destroy_snapshot(dev);
>  
>  	pb_rmdir_recursive(mount_base(), dev->mount_path);
>  err_free:
> @@ -1389,17 +1397,21 @@ err_free:
>  
>  static int umount_device(struct discover_device *dev)
>  {
> +	const char *device_path;
>  	int rc;
>  
>  	if (!dev->mounted || !dev->unmount)
>  		return 0;
>  
> -	pb_log("unmounting device %s\n", dev->device_path);
> +	device_path = dev->ramdisk ? dev->ramdisk->snapshot : dev->device_path;
> +
> +	pb_log("unmounting device %s\n", device_path);
>  	rc = umount(dev->mount_path);
>  	if (rc)
>  		return -1;
>  
>  	dev->mounted = false;
> +	discover_device_destroy_snapshot(dev);
>  
>  	pb_rmdir_recursive(mount_base(), dev->mount_path);
>  
> @@ -1411,7 +1423,7 @@ static int umount_device(struct discover_device *dev)
>  
>  int device_request_write(struct discover_device *dev, bool *release)
>  {
> -	const char *fstype;
> +	const char *fstype, *device_path;
>  	int rc;
>  
>  	*release = false;
> @@ -1419,19 +1431,25 @@ int device_request_write(struct discover_device *dev, bool *release)
>  	if (!dev->mounted)
>  		return -1;
>  
> +	//FIXME: how do we decide to commit writes, eg from grub save actions?
> +	if (!dev->ramdisk)
> +		return -1;
Might want to add an log message if you hit this, at least for the life
of the comment.

> +
>  	if (dev->mounted_rw)
>  		return 0;
>  
>  	fstype = discover_device_get_param(dev, "ID_FS_TYPE");
>  
> -	pb_log("remounting device %s read-write\n", dev->device_path);
> +	device_path = dev->ramdisk ? dev->ramdisk->snapshot : dev->device_path;
> +
> +	pb_log("remounting device %s read-write\n", device_path);
>  
>  	rc = umount(dev->mount_path);
>  	if (rc) {
>  		pb_log("Failed to unmount %s\n", dev->mount_path);
>  		return -1;
>  	}
> -	rc = mount(dev->device_path, dev->mount_path, fstype,
> +	rc = mount(device_path, dev->mount_path, fstype,
>  			MS_SILENT,
>  			fs_parameters(MS_REMOUNT, fstype));
>  	if (rc)
> @@ -1442,12 +1460,12 @@ int device_request_write(struct discover_device *dev, bool *release)
>  	return 0;
>  
>  mount_ro:
> -	pb_log("Unable to remount device %s read-write\n", dev->device_path);
> -	rc = mount(dev->device_path, dev->mount_path, fstype,
> +	pb_log("Unable to remount device %s read-write\n", device_path);
> +	rc = mount(device_path, dev->mount_path, fstype,
>  			MS_RDONLY | MS_SILENT,
>  			fs_parameters(MS_RDONLY, fstype));
>  	if (rc)
> -		pb_log("Unable to recover mount for %s\n", dev->device_path);
> +		pb_log("Unable to recover mount for %s\n", device_path);
>  	return -1;
>  }
>  
> diff --git a/discover/udev.c b/discover/udev.c
> index 6ccb8d4..8a3cabb 100644
> --- a/discover/udev.c
> +++ b/discover/udev.c
> @@ -94,12 +94,16 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
>  
>  	node = udev_device_get_devnode(dev);
>  	path = udev_device_get_devpath(dev);
> -	if (path && (strstr(path, "virtual/block/loop")
> -			|| strstr(path, "virtual/block/ram"))) {
> +	if (path && strstr(path, "virtual/block/loop")) {
>  		pb_log("SKIP: %s: ignored (path=%s)\n", name, path);
>  		return 0;
>  	}
>  
> +	if (path && strstr(path, "virtual/block/ram")) {
> +		device_handler_add_ramdisk(udev->handler, node);
tbh I have no idea, just came to mind, do you want to log something
here?
> +		return 0;
> +	}
> +
>  	cdrom = node && !!udev_device_get_property_value(dev, "ID_CDROM");
>  	if (cdrom) {
>  		/* CDROMs require a little initialisation, to get
> @@ -111,6 +115,14 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
>  		}
>  	}
>  
> +	/* dm-snapshots don't appear as udev events currently, but catch this
> +	 * in case udev/libdm are updated in Petitboot's environment */
> +	const char *devname = udev_device_get_property_value(dev, "DM_NAME");
> +	if (devname) {
> +		pb_log("This is a snapshot! %s\n", devname);
> +		return 0;
> +	}
> +
>  	type = udev_device_get_property_value(dev, "ID_FS_TYPE");
>  	if (!type) {
>  		pb_log("SKIP: %s: no ID_FS_TYPE property\n", name);
> @@ -142,6 +154,10 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
>  
>  	udev_setup_device_params(dev, ddev);
>  
> +	/* Create snapshot first if needed */
> +	if (ddev->device->type == DEVICE_TYPE_DISK)
> +		device_handler_create_snapshot(udev->handler, ddev);
> +
>  	device_handler_discover(udev->handler, ddev);
>  
>  	return 0;