[RFC PATCH 3/3] discover: Create dm-snapshots for all eligible devices
Samuel Mendoza-Jonas
sam.mj at au1.ibm.com
Tue Apr 28 10:53:22 AEST 2015
On 28/04/15 06:49, Cyril Bur wrote:
> 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;
> }
Yep
>
>> 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.
Yeah this is definitely RFC material - ideally I'll solve this before I commit anything :)
>
>> +
>> 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?
I did originally but removed it due to noise - I may re-add it in a pb_debug() call.
>> + 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;
>
--
-----------
LTC Ozlabs
IBM
More information about the Petitboot
mailing list