[Bug 852174] Review Request: snapper - Tool for filesystem snapshot management

[bugzilla at redhat.com]

https://bugzilla.redhat.com/show_bug.cgi?id=852174

Peter Rajnoha <prajnoha at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |prajnoha at redhat.com

--- Comment #10 from Peter Rajnoha <prajnoha at redhat.com> ---
A few bits I've spotted related to the dependencies and usability:

As for the Requires: lvm >= ... in the spec file - the tool is supposed to be
used with btrfs and/or LVM. However, one can have LVM installed, but not btrfs
tools or vice versa. Creating a requirement for both will bring in both btrfs
tools as well as LVM tools. Taking into account that snapper works only with
mounted volumes, these must have been activated before. In case of LVM, we
can't activate without having LVM tools installed - so we couldn't even use
that LVM mountpoint on command line if LVM tools were not installed. The same
applies for btrfs tools I guess... So it's disputable whether it's really
necessary to add a requirement for lvm/btrfs tools (as without having these
devs mounted and activated, we couldn't even refer to them).

Also, I'd probably add EXAMPLES section to the snapper man page so people can
quickly see what to do to create a simple snapshot scheme - just for
convenience.

As for functionality itself:

  (/mnt/temp1 is not mounted!)
  [0] rawhide/~ # snapper create-config -f "lvm(ext4)" /mnt/temp1
  Creating config failed (invalid filesystem type).

- maybe a better message to explain that /mnt/temp1 is not mounted at all and
so it can't create a snapshot in this case (as snapper is targeted for mounted
volumes only)

The man page also refers to a "subvolume". For LVM, this could be easily
misinterpreted as the device itself, not the mountpoint and we could end up
with:

  [0] rawhide/~ # snapper create-config -f "lvm(ext4)" /dev/vg/thin_lv          
  Creating config failed (illegal subvolume).

- if possible, a better message would be welcome that would explain that we
can't refer to devices themselves, but the mountpoints only. Refering to the
mountpoint works with LVM, of course:

  [0] rawhide/~ # snapper create-config -f "lvm(ext4)" /mnt/temp

-- 
You are receiving this mail because:
You are on the CC list for the bug.