[yast-devel] [PATCH 0/3] Snapper: propose some configuration options
Hi Arvin! First, let me thank you for a great tool to handle various snapshots, i've played with it for a few hours and I think it's a great piece of utility. We would like to get snapper into Fedora official repository (and hopefully extend snapper's user/test base). Unfortunately Fedora doesn't support Ext4 snapshots in any way, so I have to to disable this feature inside snapper tool. Sure, I can create private patches for Fedora only which would cut out these snapshots, but I think it would be nice to have configuration option that would disable some unwanted features eventually. So I propose some very simple patch set that let you choose what snapshot types you want to disable. With regards Ondrej Kozina Ondrej Kozina (3): Avoid segfault if unknown/unsupported filesystem is passed by parameter Add basic configuration options to disable some features Add defines into source code to support configuration time parameters configure.in | 25 +++++++++++++++++++++++++ snapper/Filesystem.cc | 28 +++++++++++++++++++++++----- snapper/Filesystem.h | 10 +++++++--- 3 files changed, 55 insertions(+), 8 deletions(-) -- 1.7.8.6 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
Snapper SEGFAULTs if you try to create config for an unsupported filesystem: e.g.: snapper -c foo-fs-config create-config -f foo-fs /mnt/mounted-foo-fs --- snapper/Filesystem.cc | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/snapper/Filesystem.cc b/snapper/Filesystem.cc index 6d0507a..1dac168 100644 --- a/snapper/Filesystem.cc +++ b/snapper/Filesystem.cc @@ -70,7 +70,7 @@ namespace snapper static const func_t funcs[] = { &Btrfs::create, &Ext4::create, &Lvm::create, NULL }; - for (const func_t* func = funcs; func != NULL; ++func) + for (const func_t* func = funcs; *func != NULL; ++func) { Filesystem* fs = (*func)(fstype, subvolume); if (fs) -- 1.7.8.6 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
Add macros into configure.in file to support explicit snapshot type disabling --- configure.in | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/configure.in b/configure.in index 0852183..83ba494 100644 --- a/configure.in +++ b/configure.in @@ -35,6 +35,31 @@ CXXFLAGS="${CXXFLAGS} -std=c++0x -DHAVE_CXX0X -Wall -Wextra -Wformat=2 -Wnon-vir docdir=\${prefix}/share/doc/packages/snapper fillupdir=/var/adm/fillup-templates +AC_ARG_ENABLE([ext4], AC_HELP_STRING([--disable-ext4],[Disable ext4 snapshots support]), + [with_ext4=$enableval],[with_ext4=yes]) + +if test "x$with_ext4" = "xyes"; then + AC_DEFINE(ENABLE_EXT4, 1, [Enable Ext4 snapshots support]) +fi + +AC_ARG_ENABLE([lvm], AC_HELP_STRING([--disable-lvm],[Disable LVM thinprovisioned snapshots support]), + [with_lvm=$enableval],[with_lvm=yes]) + +if test "x$with_lvm" = "xyes"; then + AC_DEFINE(ENABLE_LVM, 1, [Enable LVM thinprovisioned snapshots support]) +fi + +AC_ARG_ENABLE([btrfs], AC_HELP_STRING([--disable-btrfs],[Disable Btrfs internal snapshots support]), + [with_btrfs=$enableval],[with_btrfs=yes]) + +if test "x$with_btrfs" = "xyes"; then + AC_DEFINE(ENABLE_BTRFS, 1, [Enable Btrfs internal snapshots support]) +fi + +if test "x$with_lvm" != "xyes" -a "x$with_ext4" != "xyes" -a "x$with_btrfs" != "xyes"; then + AC_MSG_ERROR([You have to enable at least one snapshot type (remove some --disable-xxx parameter)]) +fi + AC_SUBST(VERSION) AC_SUBST(LIBVERSION_MAJOR) AC_SUBST(LIBVERSION_INFO) -- 1.7.8.6 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
Add defines into related source files --- snapper/Filesystem.cc | 24 +++++++++++++++++++++--- snapper/Filesystem.h | 10 +++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/snapper/Filesystem.cc b/snapper/Filesystem.cc index 1dac168..2aabfde 100644 --- a/snapper/Filesystem.cc +++ b/snapper/Filesystem.cc @@ -34,6 +34,7 @@ #include "snapper/SystemCmd.h" #include "snapper/SnapperDefines.h" #include "snapper/Regex.h" +#include "config.h" namespace snapper @@ -68,7 +69,17 @@ namespace snapper { typedef Filesystem* (*func_t)(const string& fstype, const string& subvolume); - static const func_t funcs[] = { &Btrfs::create, &Ext4::create, &Lvm::create, NULL }; + static const func_t funcs[] = { +#ifdef ENABLE_BTRFS + &Btrfs::create, +#endif +#ifdef ENABLE_EXT4 + &Ext4::create, +#endif +#ifdef ENABLE_LVM + &Lvm::create, +#endif + NULL }; for (const func_t* func = funcs; *func != NULL; ++func) { @@ -82,6 +93,7 @@ namespace snapper } +#ifdef ENABLE_BTRFS Filesystem* Btrfs::create(const string& fstype, const string& subvolume) { @@ -178,8 +190,10 @@ namespace snapper { return checkDir(snapshotDir(num)); } + // ENABLE_BTRFS +#endif - +#ifdef ENABLE_EXT4 Filesystem* Ext4::create(const string& fstype, const string& subvolume) { @@ -374,8 +388,10 @@ namespace snapper { return checkNormalFile(snapshotFile(num)); } + // ENABLE_EXT4 +#endif - +#ifdef ENABLE_LVM Filesystem* Lvm::create(const string& fstype, const string& subvolume) { @@ -562,5 +578,7 @@ namespace snapper return "/dev/mapper/" + boost::replace_all_copy(vg_name, "-", "--") + "-" + boost::replace_all_copy(snapshotLvName(num), "-", "--"); } + // ENABLE_LVM +#endif } diff --git a/snapper/Filesystem.h b/snapper/Filesystem.h index 6383b4f..7e5a6ef 100644 --- a/snapper/Filesystem.h +++ b/snapper/Filesystem.h @@ -27,6 +27,7 @@ #include <string> #include <vector> +#include "config.h" namespace snapper { @@ -70,7 +71,7 @@ namespace snapper }; - +#ifdef ENABLE_BTRFS class Btrfs : public Filesystem { public: @@ -97,8 +98,9 @@ namespace snapper virtual bool checkSnapshot(unsigned int num) const; }; +#endif - +#ifdef ENABLE_EXT4 class Ext4 : public Filesystem { public: @@ -130,8 +132,9 @@ namespace snapper vector<string> mount_options; }; +#endif - +#ifdef ENABLE_LVM class Lvm : public Filesystem { public: @@ -172,6 +175,7 @@ namespace snapper vector<string> mount_options; }; +#endif } -- 1.7.8.6 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On 08/22/2012 04:14 PM, Ondrej Kozina wrote:
Hi Arvin!
First, let me thank you for a great tool to handle various snapshots, i've played with it for a few hours and I think it's a great piece of utility.
We would like to get snapper into Fedora official repository (and hopefully extend snapper's user/test base). Unfortunately Fedora doesn't support Ext4 snapshots in any way, so I have to to disable this feature inside snapper tool. Sure, I can create private patches for Fedora only which would cut out these snapshots, but I think it would be nice to have configuration option that would disable some unwanted features eventually. So I propose some very simple patch set that let you choose what snapshot types you want to disable.
Hi Ondrej, We use GitHub for the snapper development. You can easily clone and patch the sources and then create a pull request to the original repository. See https://github.com/openSUSE/snapper This is usually better than sending patches via e-mail as GitHub also supports code review and code comments. Thanks && Bye Lukas -- Lukas Ocilka, Cloud & Systems Management Department SUSE LINUX s.r.o., Praha -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Wed, Aug 22, 2012 at 04:14:58PM +0200, Ondrej Kozina wrote: Hi Ondrej
We would like to get snapper into Fedora official repository (and hopefully extend snapper's user/test base). Unfortunately Fedora doesn't support Ext4 snapshots in any way, so I have to to disable this feature inside snapper tool. Sure, I can create private patches for Fedora only which would cut out these snapshots, but I think it would be nice to have configuration option that would disable some unwanted features eventually. So I propose some very simple patch set that let you choose what snapshot types you want to disable.
Thanks for the patches, I have applied them. If there are more
things you need to integrate snapper in Fedora please let me
know.
Kind Regards,
Arvin
PS: openSUSE also does not provide ext4 snapshot support by
default. The user has to install a special kernel and
e2fsprogs.
--
Arvin Schnell,
participants (3)
-
Arvin Schnell -
Lukas Ocilka -
Ondrej Kozina