Mark Fasheh
2013-Jan-07 22:24 UTC
[PATCH 17/21] btrfs-progs: libify some parts of btrfs-progs
External software wanting to use the functionality provided by the btrfs send ioctl has a hard time doing so without replicating tons of work. Of particular interest are functions like btrfs_read_and_process_send_stream() and subvol_uuid_search(). As that functionality requires a bit more than just send-stream.c and send-utils.c we have to pull in some other parts of the progs package. This patch adds code to the Makefile and headers to create a library, libbtrfs which the btrfs command now links to. Signed-off-by: Mark Fasheh <mfasheh@suse.de> Signed-off-by: David Sterba <dsterba@suse.cz> --- Makefile | 80 ++++++++++++++++++++++++++++++++++++++-------------------- btrfs-list.h | 4 +++ crc32c.h | 4 +++ ctree.h | 9 +++++++ extent-cache.h | 6 +++++ extent_io.h | 7 +++++ radix-tree.h | 4 +++ rbtree.h | 4 +++ send-utils.h | 5 ++++ 9 files changed, 96 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index e83cf5b..1dc24e3 100644 --- a/Makefile +++ b/Makefile @@ -1,16 +1,20 @@ CC = gcc LN = ln -AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 +AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -DBTRFS_FLAT_INCLUDES -fPIC CFLAGS = -g -O1 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \ root-tree.o dir-item.o file-item.o inode-item.o \ - inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \ + inode-map.o extent-cache.o extent_io.o \ volumes.o utils.o btrfs-list.o btrfslabel.o repair.o \ - send-stream.o send-utils.o qgroup.o raid6.o + qgroup.o raid6.o cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \ cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \ cmds-quota.o cmds-qgroup.o cmds-replace.o cmds-check.o \ cmds-restore.o +libbtrfs_objects = send-stream.o send-utils.o rbtree.o btrfs-list.o crc32c.o +libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \ + crc32c.h list.h kerncompat.h radix-tree.h extent-cache.h \ + extent_io.h ioctl.h ctree.h CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \ -Wuninitialized -Wshadow -Wundef @@ -19,7 +23,10 @@ DEPFLAGS = -Wp,-MMD,$(@D)/.$(@F).d,-MT,$@ INSTALL = install prefix ?= /usr/local bindir = $(prefix)/bin -LIBS=-luuid -lm -lz +libdir = $(prefix)/lib +incdir = $(prefix)/include/btrfs +lib_LIBS = -luuid -lm -lz -L. +LIBS = $(lib_LIBS) -lbtrfs ifeq ("$(origin V)", "command line") BUILD_VERBOSE = $(V) @@ -50,6 +57,10 @@ STATIC_CFLAGS = $(CFLAGS) -ffunction-sections -fdata-sections STATIC_LDFLAGS = -static -Wl,--gc-sections STATIC_LIBS = $(LIBS) -lpthread +libs = libbtrfs.so.0.1 +lib_links = libbtrfs.so.0 libbtrfs.so +headers = $(libbtrfs_headers) + # make C=1 to enable sparse ifdef C check = sparse $(CHECKFLAGS) @@ -72,38 +83,47 @@ all: version.h $(progs) manpages # NOTE: For static compiles, you need to have all the required libs # static equivalent available # -static: version.h btrfs.static +static: version.h $(libs) btrfs.static version.h: $(Q)bash version.sh -btrfs: $(objects) btrfs.o help.o $(cmds_objects) +$(libs): $(libbtrfs_objects) $(lib_links) send.h + @echo " [LD] $@" + $(Q)$(CC) $(CFLAGS) $(libbtrfs_objects) $(lib_LIBS) -shared -Wl,-soname,libbtrfs.so -o libbtrfs.so.0.1 + +$(lib_links): + @echo " [LN] $@" + $(Q)$(LN) -sf libbtrfs.so.0.1 libbtrfs.so.0 + $(Q)$(LN) -sf libbtrfs.so.0.1 libbtrfs.so + +btrfs: $(objects) btrfs.o help.o $(cmds_objects) $(libs) @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs btrfs.o help.o $(cmds_objects) \ $(objects) $(LDFLAGS) $(LIBS) -lpthread -btrfs.static: $(static_objects) btrfs.static.o help.static.o $(static_cmds_objects) +btrfs.static: $(static_objects) $(libs) btrfs.static.o help.static.o $(static_cmds_objects) @echo " [LD] $@" $(Q)$(CC) $(STATIC_CFLAGS) -o btrfs.static btrfs.static.o help.static.o $(static_cmds_objects) \ $(static_objects) $(STATIC_LDFLAGS) $(STATIC_LIBS) -calc-size: $(objects) calc-size.o +calc-size: $(objects) $(libs) calc-size.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o calc-size calc-size.o $(objects) $(LDFLAGS) $(LIBS) -btrfs-find-root: $(objects) find-root.o +btrfs-find-root: $(objects) $(libs) find-root.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-find-root find-root.o $(objects) $(LDFLAGS) $(LIBS) -btrfsctl: $(objects) btrfsctl.o +btrfsctl: $(objects) $(libs) btrfsctl.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfsctl btrfsctl.o $(objects) $(LDFLAGS) $(LIBS) -btrfs-vol: $(objects) btrfs-vol.o +btrfs-vol: $(objects) $(libs) btrfs-vol.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-vol btrfs-vol.o $(objects) $(LDFLAGS) $(LIBS) -btrfs-show: $(objects) btrfs-show.o +btrfs-show: $(objects) $(libs) btrfs-show.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-show btrfs-show.o $(objects) $(LDFLAGS) $(LIBS) @@ -112,55 +132,55 @@ btrfsck: btrfs @echo " [LN] $@" $(Q)$(LN) -f btrfs btrfsck -mkfs.btrfs: $(objects) mkfs.o +mkfs.btrfs: $(objects) $(libs) mkfs.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS) -lblkid -btrfs-debug-tree: $(objects) debug-tree.o +btrfs-debug-tree: $(objects) $(libs) debug-tree.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-debug-tree $(objects) debug-tree.o $(LDFLAGS) $(LIBS) -btrfs-zero-log: $(objects) btrfs-zero-log.o +btrfs-zero-log: $(objects) $(libs) btrfs-zero-log.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-zero-log $(objects) btrfs-zero-log.o $(LDFLAGS) $(LIBS) -btrfs-show-super: $(objects) btrfs-show-super.o +btrfs-show-super: $(objects) $(libs) btrfs-show-super.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-show-super $(objects) btrfs-show-super.o $(LDFLAGS) $(LIBS) -btrfs-select-super: $(objects) btrfs-select-super.o +btrfs-select-super: $(objects) $(libs) btrfs-select-super.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-select-super $(objects) btrfs-select-super.o $(LDFLAGS) $(LIBS) -btrfstune: $(objects) btrfstune.o +btrfstune: $(objects) $(libs) btrfstune.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfstune $(objects) btrfstune.o $(LDFLAGS) $(LIBS) -btrfs-map-logical: $(objects) btrfs-map-logical.o +btrfs-map-logical: $(objects) $(libs) btrfs-map-logical.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-map-logical $(objects) btrfs-map-logical.o $(LDFLAGS) $(LIBS) -btrfs-corrupt-block: $(objects) btrfs-corrupt-block.o +btrfs-corrupt-block: $(objects) $(libs) btrfs-corrupt-block.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-corrupt-block $(objects) btrfs-corrupt-block.o $(LDFLAGS) $(LIBS) -btrfs-image: $(objects) btrfs-image.o +btrfs-image: $(objects) $(libs) btrfs-image.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-image $(objects) btrfs-image.o -lpthread -lz $(LDFLAGS) $(LIBS) -dir-test: $(objects) dir-test.o +dir-test: $(objects) $(libs) dir-test.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o dir-test $(objects) dir-test.o $(LDFLAGS) $(LIBS) -quick-test: $(objects) quick-test.o +quick-test: $(objects) $(libs) quick-test.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o quick-test $(objects) quick-test.o $(LDFLAGS) $(LIBS) -btrfs-convert: $(objects) convert.o +btrfs-convert: $(objects) $(libs) convert.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-convert $(objects) convert.o -lext2fs -lcom_err $(LDFLAGS) $(LIBS) -ioctl-test: $(objects) ioctl-test.o +ioctl-test: $(objects) $(libs) ioctl-test.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o ioctl-test $(objects) ioctl-test.o $(LDFLAGS) $(LIBS) @@ -174,11 +194,17 @@ clean : @echo "Cleaning" $(Q)rm -f $(progs) cscope.out *.o .*.d btrfs-convert btrfs-image btrfs-select-super \ btrfs-zero-log btrfstune dir-test ioctl-test quick-test btrfs.static btrfsck \ - version.h + version.h \ + $(libs) libbtrfs.so libbtrfs.so.0 libbtrfs.so.0.1 $(Q)$(MAKE) $(MAKEOPTS) -C man $@ -install: $(progs) install-man +install: $(libs) $(progs) install-man $(INSTALL) -m755 -d $(DESTDIR)$(bindir) $(INSTALL) $(progs) $(DESTDIR)$(bindir) + $(INSTALL) -m755 -d $(DESTDIR)$(libdir) + $(INSTALL) $(libs) $(DESTDIR)$(libdir) + cp -a $(lib_links) $(DESTDIR)$(libdir) + $(INSTALL) -m755 -d $(DESTDIR)$(incdir) + $(INSTALL) -m644 $(headers) $(DESTDIR)$(incdir) -include .*.d diff --git a/btrfs-list.h b/btrfs-list.h index 5d87454..397f06c 100644 --- a/btrfs-list.h +++ b/btrfs-list.h @@ -16,7 +16,11 @@ * Boston, MA 021110-1307, USA. */ +#if BTRFS_FLAT_INCLUDES #include "kerncompat.h" +#else +#include <btrfs/kerncompat.h> +#endif /* BTRFS_FLAT_INCLUDES */ #define BTRFS_LIST_LAYOUT_DEFAULT 0 #define BTRFS_LIST_LAYOUT_TABLE 1 diff --git a/crc32c.h b/crc32c.h index 7f12e77..c552ef6 100644 --- a/crc32c.h +++ b/crc32c.h @@ -19,7 +19,11 @@ #ifndef __CRC32C__ #define __CRC32C__ +#if BTRFS_FLAT_INCLUDES #include "kerncompat.h" +#else +#include <btrfs/kerncompat.h> +#endif /* BTRFS_FLAT_INCLUDES */ u32 crc32c_le(u32 seed, unsigned char const *data, size_t length); void crc32c_optimization_init(void); diff --git a/ctree.h b/ctree.h index 12f8fe3..0aed2fc 100644 --- a/ctree.h +++ b/ctree.h @@ -19,12 +19,21 @@ #ifndef __BTRFS__ #define __BTRFS__ +#if BTRFS_FLAT_INCLUDES #include "list.h" #include "kerncompat.h" #include "radix-tree.h" #include "extent-cache.h" #include "extent_io.h" #include "ioctl.h" +#else +#include <btrfs/list.h> +#include <btrfs/kerncompat.h> +#include <btrfs/radix-tree.h> +#include <btrfs/extent-cache.h> +#include <btrfs/extent_io.h> +#include <btrfs/ioctl.h> +#endif /* BTRFS_FLAT_INCLUDES */ struct btrfs_root; struct btrfs_trans_handle; diff --git a/extent-cache.h b/extent-cache.h index 7f2f2a6..4cd0f79 100644 --- a/extent-cache.h +++ b/extent-cache.h @@ -18,8 +18,14 @@ #ifndef __PENDING_EXTENT__ #define __PENDING_EXTENT__ + +#if BTRFS_FLAT_INCLUDES #include "kerncompat.h" #include "rbtree.h" +#else +#include <btrfs/kerncompat.h> +#include <btrfs/rbtree.h> +#endif /* BTRFS_FLAT_INCLUDES */ struct cache_tree { struct rb_root root; diff --git a/extent_io.h b/extent_io.h index 5df44f7..feedfc3 100644 --- a/extent_io.h +++ b/extent_io.h @@ -18,9 +18,16 @@ #ifndef __EXTENTMAP__ #define __EXTENTMAP__ + +#if BTRFS_FLAT_INCLUDES #include "kerncompat.h" #include "extent-cache.h" #include "list.h" +#else +#include <btrfs/kerncompat.h> +#include <btrfs/extent-cache.h> +#include <btrfs/list.h> +#endif /* BTRFS_FLAT_INCLUDES */ #define EXTENT_DIRTY 1 #define EXTENT_WRITEBACK (1 << 1) diff --git a/radix-tree.h b/radix-tree.h index d99ea7e..bf96d83 100644 --- a/radix-tree.h +++ b/radix-tree.h @@ -37,7 +37,11 @@ #ifndef _LINUX_RADIX_TREE_H #define _LINUX_RADIX_TREE_H +#if BTRFS_FLAT_INCLUDES #include "kerncompat.h" +#else +#include <btrfs/kerncompat.h> +#endif /* BTRFS_FLAT_INCLUDES */ #define RADIX_TREE_MAX_TAGS 2 diff --git a/rbtree.h b/rbtree.h index bed054d..b636ddd 100644 --- a/rbtree.h +++ b/rbtree.h @@ -93,7 +93,11 @@ static inline struct page * rb_insert_page_cache(struct inode * inode, #ifndef _LINUX_RBTREE_H #define _LINUX_RBTREE_H +#if BTRFS_FLAT_INCLUDES #include "kerncompat.h" +#else +#include <btrfs/kerncompat.h> +#endif /* BTRFS_FLAT_INCLUDES */ struct rb_node { unsigned long rb_parent_color; diff --git a/send-utils.h b/send-utils.h index da407eb..8040c50 100644 --- a/send-utils.h +++ b/send-utils.h @@ -18,8 +18,13 @@ #ifndef SEND_UTILS_H_ #define SEND_UTILS_H_ +#if BTRFS_FLAT_INCLUDES #include "ctree.h" #include "rbtree.h" +#else +#include <btrfs/ctree.h> +#include <btrfs/rbtree.h> +#endif /* BTRFS_FLAT_INCLUDES */ enum subvol_search_type { subvol_search_by_root_id, -- 1.8.0.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Sterba
2013-Feb-12 16:39 UTC
[PATCH 13/21] btrfs-progs: build btrsfck to keep compatibility
The command ''btrfsck'' is commonly used and we should build it by default. Signed-off-by: David Sterba <dsterba@suse.cz> --- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 039f0dc..7a679d0 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ CC = gcc +LN = ln AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 CFLAGS = -g -O1 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \ @@ -35,7 +36,7 @@ endif MAKEOPTS = --no-print-directory Q=$(Q) -progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol \ +progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol btrfsck \ btrfs btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \ btrfs-find-root btrfs-restore btrfstune btrfs-show-super @@ -110,6 +111,11 @@ btrfs-show: $(objects) btrfs-show.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o btrfs-show btrfs-show.o $(objects) $(LDFLAGS) $(LIBS) +# For backward compatibility, ''btrfs'' changes behaviour to fsck if it''s named ''btrfsck'' +btrfsck: btrfs + @echo " [LN] $@" + $(Q)$(LN) -f btrfs btrfsck + mkfs.btrfs: $(objects) mkfs.o @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS) -lblkid @@ -171,7 +177,7 @@ install-man: clean : @echo "Cleaning" $(Q)rm -f $(progs) cscope.out *.o .*.d btrfs-convert btrfs-image btrfs-select-super \ - btrfs-zero-log btrfstune dir-test ioctl-test quick-test btrfs.static \ + btrfs-zero-log btrfstune dir-test ioctl-test quick-test btrfs.static btrfsck \ version.h $(Q)$(MAKE) $(MAKEOPTS) -C man $@ -- 1.8.0.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Sterba
2013-Feb-12 17:44 UTC
[PATCH 20/21] btrfs-progs: don''t link binaries to a dynamic library
Linking ''btrfs'' and other binaries against the dynamic library makes it tedious to use directly from the git repo. This is useful for testing various fixes, but now it''d need to also set LD_LIBRARY_PATH or install the library to a known path. Add a target for static library and use it for linking, the dynamic library is to be used by external users. Signed-off-by: David Sterba <dsterba@suse.cz> --- Makefile | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 6013fe2..84c39c0 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,6 @@ CC = gcc LN = ln +AR = ar AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -DBTRFS_FLAT_INCLUDES -fPIC CFLAGS = -g -O1 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \ @@ -26,7 +27,7 @@ bindir = $(prefix)/bin libdir = $(prefix)/lib incdir = $(prefix)/include/btrfs lib_LIBS = -luuid -lm -lz -L. -LIBS = $(lib_LIBS) -lbtrfs +LIBS = $(lib_LIBS) $(libs_static) ifeq ("$(origin V)", "command line") BUILD_VERBOSE = $(V) @@ -57,7 +58,9 @@ STATIC_CFLAGS = $(CFLAGS) -ffunction-sections -fdata-sections STATIC_LDFLAGS = -static -Wl,--gc-sections STATIC_LIBS = $(LIBS) -lpthread -libs = libbtrfs.so.0.1 +libs_shared = libbtrfs.so.0.1 +libs_static = libbtrfs.a +libs = $(libs_shared) $(libs_static) lib_links = libbtrfs.so.0 libbtrfs.so headers = $(libbtrfs_headers) @@ -88,10 +91,14 @@ static: version.h $(libs) btrfs.static version.h: $(Q)bash version.sh -$(libs): $(libbtrfs_objects) $(lib_links) send.h +$(libs_shared): $(libbtrfs_objects) $(lib_links) send.h @echo " [LD] $@" $(Q)$(CC) $(CFLAGS) $(libbtrfs_objects) $(lib_LIBS) -shared -Wl,-soname,libbtrfs.so -o libbtrfs.so.0.1 +$(libs_static): $(libbtrfs_objects) + @echo " [AR] $@" + $(Q)$(AR) cru libbtrfs.a $(libbtrfs_objects) + $(lib_links): @echo " [LN] $@" $(Q)$(LN) -sf libbtrfs.so.0.1 libbtrfs.so.0 @@ -199,7 +206,7 @@ clean : $(Q)rm -f $(progs) cscope.out *.o .*.d btrfs-convert btrfs-image btrfs-select-super \ btrfs-zero-log btrfstune dir-test ioctl-test quick-test send-test btrfs.static btrfsck \ version.h \ - $(libs) libbtrfs.so libbtrfs.so.0 libbtrfs.so.0.1 + $(libs) libbtrfs.so libbtrfs.so.0 libbtrfs.so.0.1 libbtrfs.a $(Q)$(MAKE) $(MAKEOPTS) -C man $@ install: $(libs) $(progs) install-man -- 1.8.0.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, a few more fixes and enhancements: check and restore subcommands, support for send mode NO_FILE_DATA and partial "libifying". The libify patchset porting was not straightforward (based on old sources) and I fixed it here up to my satisfaction. The changes were mostly small but there was quite a lot of them. I''ll send patches as a followup to this mail for review in case I did non-trivial modifications. There''s the patch converting build to autotools, but the conflict surface was too big so it''s left out for now. (The patch backlog is not yet finished.) Thanks for testing, david --- Arvin Schnell (1): btrfs-progs: make libbtrfs usable from C++ David Sterba (2): btrfs-progs: build btrsfck to keep compatibility btrfs-progs: don''t link binaries to a dynamic library Eric Sandeen (2): Btrfs-progs print more informative error when we fail to open a device btrfs-progs: remove unused bit-radix.[ch] files Ian Kumlien (6): Btrfs-progs: add static compile target Btrfs-progs: -U_FORTIFY_SOURCE before -D Btrfs-progs: Rename btrfsck.c -> cmds-check.c Btrfs-progs: add btrfsck functionality to btrfs Btrfs-progs: restore.c -> cmds-restore.c Btrfs-progs: add restore command to btrfs Ilya Dryomov (3): Btrfs-progs: move crc32c optimization init Btrfs-progs: add btrfsck name detection to btrfs Btrfs-progs: make 0 a valid usage filter argument Josef Bacik (3): Btrfs-progs: handle errors reading fs roots Btrfs-progs: fix double free of extent buffer Btrfs-progs: return an error if we can''t find an fs root Mark Fasheh (3): btrfs-progs: Add support for BTRFS_SEND_FLAG_NO_FILE_DATA btrfs-progs: libify some parts of btrfs-progs btrfs-progs: add send-test Wang Sheng-Hui (1): btrfs-progs: code cleanup for root-tree.c/btrfs_del_root -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David, I really have concerns about the libification, in particular this commit: 6fc8b21 btrfs-progs: libify some parts of btrfs-progs The relevant snippets that concern me below: +libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \ + crc32c.h list.h kerncompat.h radix-tree.h extent-cache.h \ + extent_io.h ioctl.h ctree.h ... +headers = $(libbtrfs_headers) ... + $(INSTALL) -m644 $(headers) $(DESTDIR)$(incdir) I really don''t think that all those headers should be exposed to the userspace. I think, to do it right, there should be a single or a few header files, like /usr/include/btrfs.h or /usr/include/libbtrfs.h or /usr/include/btrfs/xyz.h, that export only the parts that are really necessary for an application that wants to use libbtrfs. Does it really make sense to have btrfs-progs expose things like ctree.h or crc32c.h or even list.h to userspace?! Another reason of my concerns is that I''ve been trying to work on exporting the equivalent of ioctl.h, with the constants and structs needed to call btrfs-specific ioctls, from the kernel side, I already submitted a patch to export it from a Linux kernel build as /usr/include/linux/btrfs.h. I believe that''s the right way to export that particular information. The other part of it is synchronizing the header files (and to some extent some source files, like ctree.c) between the kernel and btrfs-progs. The current version of those headers (and C source files) in btrfs-progs was a copy of the same files from the kernel tree that was edited to compile in userspace and then diverged from the copy of the same files in the kernel. We should try to unify those (I sent another patch with a suggestion of a script that would update those from the kernel git tree.) Exposing those to userspace now would only muddle that situation more. If you think you can build a new header file that is clean and contain only the prototypes and constants and structs that are strictly required to export functions in a libbtrfs, then I''d fully support this patchset. If it''s going to export all the header files as it''s doing now, then I wouldn''t... Thanks, Filipe -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 12, 2013 at 07:35:31PM -0800, Filipe Brandenburger wrote:> Hi David, > > I really have concerns about the libification, in particular this commit: > > 6fc8b21 btrfs-progs: libify some parts of btrfs-progs > > The relevant snippets that concern me below: > > +libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \ > + crc32c.h list.h kerncompat.h radix-tree.h extent-cache.h \ > + extent_io.h ioctl.h ctree.h > ... > +headers = $(libbtrfs_headers) > ... > + $(INSTALL) -m644 $(headers) $(DESTDIR)$(incdir) > > > I really don''t think that all those headers should be exposed to the userspace. > > I think, to do it right, there should be a single or a few header > files, like /usr/include/btrfs.h or /usr/include/libbtrfs.h or > /usr/include/btrfs/xyz.h, that export only the parts that are really > necessary for an application that wants to use libbtrfs. Does it > really make sense to have btrfs-progs expose things like ctree.h or > crc32c.h or even list.h to userspace?!It''s worked in ocfs2-tools for years now. In fact on my system here I counted 6 other packages which expose some version of it which they took initially from the kernel. There''s nothing inherently "kernely" about a lot of that stuff and it''s stable code which has worked well for years. The same goes for the crc32 functions. Btrfs-progs needs that code to function. Anything linking against libbtrfs will need that too.> Another reason of my concerns is that I''ve been trying to work on > exporting the equivalent of ioctl.h, with the constants and structs > needed to call btrfs-specific ioctls, from the kernel side, I already > submitted a patch to export it from a Linux kernel build as > /usr/include/linux/btrfs.h. I believe that''s the right way to export > that particular information.I don''t disagree with this goal, in fact I quite like it. Can you point us to your work?> The other part of it is synchronizing the header files (and to some > extent some source files, like ctree.c) between the kernel and > btrfs-progs. The current version of those headers (and C source files) > in btrfs-progs was a copy of the same files from the kernel tree that > was edited to compile in userspace and then diverged from the copy of > the same files in the kernel. We should try to unify those (I sent > another patch with a suggestion of a script that would update those > from the kernel git tree.) Exposing those to userspace now would only > muddle that situation more. > > If you think you can build a new header file that is clean and contain > only the prototypes and constants and structs that are strictly > required to export functions in a libbtrfs, then I''d fully support > this patchset. If it''s going to export all the header files as it''s > doing now, then I wouldn''t...It''s impractical to expect us to hide that data when there is no official distribution of /usr/include/linux/btrfs.h and there are applications which want to be developed against this. In fact there''s really no point of having the library if we don''t expose interfaces (including necessary structure definitions). If you look at the patch series that I posted, you''ll notice that while I actually did some of the initial grunt work, there are patches from several people who were interested that the library work a certain way and I included their patches. What about porting some of your work over to libbtrfs? That way we could get the nicely unified header files in both places. Btw, it''s also trivial to add a configure check for /usr/include/linux/btrfs.h if different behavior on install is required. We could even add that now (if you have an idea of what would change) despite your patch not being upstream. --Mark -- Mark Fasheh -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mark, On Wed, Feb 13, 2013 at 1:11 PM, Mark Fasheh <mfasheh@suse.de> wrote:> On Tue, Feb 12, 2013 at 07:35:31PM -0800, Filipe Brandenburger wrote: >> Another reason of my concerns is that I''ve been trying to work on >> exporting the equivalent of ioctl.h, with the constants and structs >> needed to call btrfs-specific ioctls, from the kernel side, I already >> submitted a patch to export it from a Linux kernel build as >> /usr/include/linux/btrfs.h. I believe that''s the right way to export >> that particular information. > > I don''t disagree with this goal, in fact I quite like it. Can you point us > to your work?Sure. 1) Exposing list of constants needed for ioctls from the kernel include files (that makes it into /usr/include/linux): http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg21678.html I also wrote an initial patchset to use that from strace in order to be able to use strace on the btrfs-progs and interpret the ioctls: http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg21679.html I want to resume that work, but that definitely involves shifting things around in header files and that looks to me like walking on quicksand right now so I''m a bit wary... 2) Merging headers and source files from kernel btrfs and btrfs-progs: http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg22124.html That''s simply a proposal on using a script to "checkout" a certain version of the shared files from the kernel sources into the btrfs-progs git tree (think of it as a sparse git submodule.) It doesn''t go into the lengths of making the kernel source compatible with userspace (e.g. using #ifdef''s and #define''s etc.) but it''s a start having the files there so that you could try to compile them and see the issues. Cheers, Filipe -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 12, 2013 at 07:35:31PM -0800, Filipe Brandenburger wrote:> I really have concerns about the libification, in particular this commit: > > 6fc8b21 btrfs-progs: libify some parts of btrfs-progs[...]> I really don''t think that all those headers should be exposed to the userspace. > > I think, to do it right, there should be a single or a few header > files, like /usr/include/btrfs.h or /usr/include/libbtrfs.h or > /usr/include/btrfs/xyz.h, that export only the parts that are really > necessary for an application that wants to use libbtrfs. Does it > really make sense to have btrfs-progs expose things like ctree.h or > crc32c.h or even list.h to userspace?![...] For the record and public status of the libify patches: after the discussion on IRC a few days ago there is more work need to clean and finalize the header interfaces so that two currently proposed and future users of the library are satisfied. For that reason I''ll not add libify patches into the integration branch directly, but keep it in a separate branch until all sides are ok, so that the integration branch is a relatively stable codebase for fixes and minor updates. That''s my view of the situation, feel free to correct or update the bits I might have missed. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html