https://bugzilla.redhat.com/show_bug.cgi?id=852174 Peter Rajnoha <prajnoha@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |prajnoha@xxxxxxxxxx --- Comment #10 from Peter Rajnoha <prajnoha@xxxxxxxxxx> --- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review