Re: [PATCH 02/26] Add libbtrfsutil

2018-01-29 Thread Omar Sandoval
On Mon, Jan 29, 2018 at 10:16:40AM +0800, Qu Wenruo wrote:
> 
> 
> On 2018年01月27日 02:40, Omar Sandoval wrote:
> > From: Omar Sandoval 
> > 
> > Currently, users wishing to manage Btrfs filesystems programatically
> > have to shell out to btrfs-progs and parse the output. This isn't ideal.
> > The goal of libbtrfsutil is to provide a library version of as many of
> > the operations of btrfs-progs as possible and to migrate btrfs-progs to
> > use it.
> > 
> > Rather than simply refactoring the existing btrfs-progs code, the code
> > has to be written from scratch for a couple of reasons:
> > 
> > * A lot of the btrfs-progs code was not designed with a nice library API
> >   in mind in terms of reusability, naming, and error reporting.
> > * libbtrfsutil is licensed under the LGPL, whereas btrfs-progs is under
> >   the GPL, which makes it dubious to directly copy or move the code.
> > 
> > Eventually, most of the low-level btrfs-progs code should either live in
> > libbtrfsutil or the shared kernel/userspace filesystem code, and
> > btrfs-progs will just be the CLI wrapper.
> > 
> > This first commit just includes the build system changes, license,
> > README, and error reporting helper.
> > 
> > Signed-off-by: Omar Sandoval 
> > ---
> >  .gitignore  |   2 +
> >  Makefile|  47 +--
> >  libbtrfsutil/COPYING| 674 
> > 
> >  libbtrfsutil/COPYING.LESSER | 165 +++
> >  libbtrfsutil/README.md  |  32 +++
> >  libbtrfsutil/btrfsutil.h|  72 +
> >  libbtrfsutil/errors.c   |  55 
> >  7 files changed, 1031 insertions(+), 16 deletions(-)
> >  create mode 100644 libbtrfsutil/COPYING
> >  create mode 100644 libbtrfsutil/COPYING.LESSER
> >  create mode 100644 libbtrfsutil/README.md
> >  create mode 100644 libbtrfsutil/btrfsutil.h
> >  create mode 100644 libbtrfsutil/errors.c
> > 

[snip]

> > diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
> > new file mode 100644
> > index ..fe1091ca
> > --- /dev/null
> > +++ b/libbtrfsutil/btrfsutil.h
> > @@ -0,0 +1,72 @@
> > +/*
> > + * Copyright (C) 2018 Facebook
> > + *
> > + * This file is part of libbtrfsutil.
> > + *
> > + * libbtrfsutil is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU Lesser General Public License as 
> > published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * libbtrfsutil is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public License
> > + * along with libbtrfsutil.  If not, see .
> > + */
> > +
> > +#ifndef BTRFS_UTIL_H
> > +#define BTRFS_UTIL_H
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * enum btrfs_util_error - libbtrfsutil error codes.
> > + *
> > + * All functions in libbtrfsutil that can return an error return this type 
> > and
> > + * set errno.
> > + */
> 
> I totally understand that libbtrfsutils needs extra error numbers, but I
> didn't see similar practice, would you mind to give some existing
> example of such >0 error usage in other projects?
> (Just curious)

Sure, pthreads returns 0 on success, positive errnos on failure. libcurl
also uses positive error codes 
(https://curl.haxx.se/libcurl/c/libcurl-errors.html).
Those are just the first two I checked.

> Normally people would expect error values < 0 to indicate errors, just
> like glibc system call wrappers, which always return -1 to indicate errors.
> 
> > +enum btrfs_util_error {
> > +   BTRFS_UTIL_OK,
> > +   BTRFS_UTIL_ERROR_STOP_ITERATION,
> > +   BTRFS_UTIL_ERROR_NO_MEMORY,
> 
> Not sure if this is duplicated with -ENOMEM errno.
> 
> From my understanding, these extra numbers should be used to indicate
> extra error not definied in generic errno.h.
> 
> For NOT_BTRFS and NOT_SUBVOLUME they makes sense, but for NO_MEMORY, I'm
> really not sure.

So sometimes we return an errno, sometimes an enum btrfs_util_error? And
then we have to make sure to avoid collisions between the enum values
and errno values? That seems clunky.

Thanks for taking a look.

> > +   BTRFS_UTIL_ERROR_INVALID_ARGUMENT,
> > +   BTRFS_UTIL_ERROR_NOT_BTRFS,
> > +   BTRFS_UTIL_ERROR_NOT_SUBVOLUME,
> > +   BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND,
> > +   BTRFS_UTIL_ERROR_OPEN_FAILED,
> > +   BTRFS_UTIL_ERROR_RMDIR_FAILED,
> > +   BTRFS_UTIL_ERROR_UNLINK_FAILED,
> > +   BTRFS_UTIL_ERROR_STAT_FAILED,
> > +   BTRFS_UTIL_ERROR_STATFS_FAILED,
> > +   BTRFS_UTIL_ERROR_SEARCH_FAILED,
> > +   BTRFS_UTIL_ERROR_INO_LOOKUP_FAILED,
> > +   BTRFS_UTIL_ERROR_SUBVOL_GETFLAGS_FAILED,
> > +   

Re: [PATCH 02/26] Add libbtrfsutil

2018-01-28 Thread Qu Wenruo


On 2018年01月27日 02:40, Omar Sandoval wrote:
> From: Omar Sandoval 
> 
> Currently, users wishing to manage Btrfs filesystems programatically
> have to shell out to btrfs-progs and parse the output. This isn't ideal.
> The goal of libbtrfsutil is to provide a library version of as many of
> the operations of btrfs-progs as possible and to migrate btrfs-progs to
> use it.
> 
> Rather than simply refactoring the existing btrfs-progs code, the code
> has to be written from scratch for a couple of reasons:
> 
> * A lot of the btrfs-progs code was not designed with a nice library API
>   in mind in terms of reusability, naming, and error reporting.
> * libbtrfsutil is licensed under the LGPL, whereas btrfs-progs is under
>   the GPL, which makes it dubious to directly copy or move the code.
> 
> Eventually, most of the low-level btrfs-progs code should either live in
> libbtrfsutil or the shared kernel/userspace filesystem code, and
> btrfs-progs will just be the CLI wrapper.
> 
> This first commit just includes the build system changes, license,
> README, and error reporting helper.
> 
> Signed-off-by: Omar Sandoval 
> ---
>  .gitignore  |   2 +
>  Makefile|  47 +--
>  libbtrfsutil/COPYING| 674 
> 
>  libbtrfsutil/COPYING.LESSER | 165 +++
>  libbtrfsutil/README.md  |  32 +++
>  libbtrfsutil/btrfsutil.h|  72 +
>  libbtrfsutil/errors.c   |  55 
>  7 files changed, 1031 insertions(+), 16 deletions(-)
>  create mode 100644 libbtrfsutil/COPYING
>  create mode 100644 libbtrfsutil/COPYING.LESSER
>  create mode 100644 libbtrfsutil/README.md
>  create mode 100644 libbtrfsutil/btrfsutil.h
>  create mode 100644 libbtrfsutil/errors.c
> 
> diff --git a/.gitignore b/.gitignore
> index 8e607f6e..272d53e4 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -43,6 +43,8 @@ libbtrfs.so.0.1
>  library-test
>  library-test-static
>  /fssum
> +/libbtrfsutil.so*
> +/libbtrfsutil.a
>  
>  /tests/*-tests-results.txt
>  /tests/test-console.txt
> diff --git a/Makefile b/Makefile
> index 6369e8f4..062f7f3c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -73,6 +73,7 @@ CFLAGS = $(SUBST_CFLAGS) \
>-fPIC \
>-I$(TOPDIR) \
>-I$(TOPDIR)/kernel-lib \
> +  -I$(TOPDIR)/libbtrfsutil \
>$(EXTRAWARN_CFLAGS) \
>$(DEBUG_CFLAGS_INTERNAL) \
>$(EXTRA_CFLAGS)
> @@ -121,12 +122,14 @@ libbtrfs_headers = send-stream.h send-utils.h send.h 
> kernel-lib/rbtree.h btrfs-l
>  kernel-lib/crc32c.h kernel-lib/list.h kerncompat.h \
>  kernel-lib/radix-tree.h kernel-lib/sizes.h kernel-lib/raid56.h \
>  extent-cache.h extent_io.h ioctl.h ctree.h btrfsck.h version.h
> +libbtrfsutil_version := 0.1
> +libbtrfsutil_objects = libbtrfsutil/errors.o
>  convert_objects = convert/main.o convert/common.o convert/source-fs.o \
> convert/source-ext2.o convert/source-reiserfs.o
>  mkfs_objects = mkfs/main.o mkfs/common.o
>  image_objects = image/main.o image/sanitize.o
>  all_objects = $(objects) $(cmds_objects) $(libbtrfs_objects) 
> $(convert_objects) \
> -   $(mkfs_objects) $(image_objects)
> +   $(mkfs_objects) $(image_objects) $(libbtrfsutil_objects)
>  
>  TESTS = fsck-tests.sh convert-tests.sh
>  
> @@ -248,10 +251,10 @@ static_convert_objects = $(patsubst %.o, %.static.o, 
> $(convert_objects))
>  static_mkfs_objects = $(patsubst %.o, %.static.o, $(mkfs_objects))
>  static_image_objects = $(patsubst %.o, %.static.o, $(image_objects))
>  
> -libs_shared = libbtrfs.so.0.1
> -libs_static = libbtrfs.a
> +libs_shared = libbtrfs.so.0.1 libbtrfsutil.so.$(libbtrfsutil_version)
> +libs_static = libbtrfs.a libbtrfsutil.a
>  libs = $(libs_shared) $(libs_static)
> -lib_links = libbtrfs.so.0 libbtrfs.so
> +lib_links = libbtrfs.so.0 libbtrfs.so libbtrfsutil.so.0 libbtrfsutil.so
>  headers = $(libbtrfs_headers)
>  
>  # make C=1 to enable sparse
> @@ -289,7 +292,7 @@ endif
>   $(Q)$(CC) $(STATIC_CFLAGS) -c $< -o $@ $($(subst 
> -,_,$(@:%.static.o=%)-cflags)) \
>   $($(subst -,_,btrfs-$(@:%/$(notdir $@)=%)-cflags))
>  
> -all: $(progs) libbtrfs $(BUILDDIRS)
> +all: $(progs) $(libs) $(lib_links) $(BUILDDIRS)
>  $(SUBDIRS): $(BUILDDIRS)
>  $(BUILDDIRS):
>   @echo "Making all in $(patsubst build-%,%,$@)"
> @@ -353,20 +356,31 @@ kernel-lib/tables.c:
>   @echo "[TABLE]  $@"
>   $(Q)./mktables > $@ || ($(RM) -f $@ && exit 1)
>  
> -libbtrfs: $(libs_shared) $(lib_links)
> +libbtrfs.so.0.1: $(libbtrfs_objects)
> + @echo "[LD] $@"
> + $(Q)$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LIBBTRFS_LIBS) \
> + -shared -Wl,-soname,libbtrfs.so.0 -o $@
> +
> +libbtrfs.a: $(libbtrfs_objects)
> + @echo "[AR] $@"
> + $(Q)$(AR) cr $@ $^
> +
> +libbtrfs.so.0 libbtrfs.so: libbtrfs.so.0.1
> + @echo "[LN] $@"
> + $(Q)$(LN_S) -f $< $@
>  
> -$(libs_shared): $(libbtrfs_objects) 

[PATCH 02/26] Add libbtrfsutil

2018-01-26 Thread Omar Sandoval
From: Omar Sandoval 

Currently, users wishing to manage Btrfs filesystems programatically
have to shell out to btrfs-progs and parse the output. This isn't ideal.
The goal of libbtrfsutil is to provide a library version of as many of
the operations of btrfs-progs as possible and to migrate btrfs-progs to
use it.

Rather than simply refactoring the existing btrfs-progs code, the code
has to be written from scratch for a couple of reasons:

* A lot of the btrfs-progs code was not designed with a nice library API
  in mind in terms of reusability, naming, and error reporting.
* libbtrfsutil is licensed under the LGPL, whereas btrfs-progs is under
  the GPL, which makes it dubious to directly copy or move the code.

Eventually, most of the low-level btrfs-progs code should either live in
libbtrfsutil or the shared kernel/userspace filesystem code, and
btrfs-progs will just be the CLI wrapper.

This first commit just includes the build system changes, license,
README, and error reporting helper.

Signed-off-by: Omar Sandoval 
---
 .gitignore  |   2 +
 Makefile|  47 +--
 libbtrfsutil/COPYING| 674 
 libbtrfsutil/COPYING.LESSER | 165 +++
 libbtrfsutil/README.md  |  32 +++
 libbtrfsutil/btrfsutil.h|  72 +
 libbtrfsutil/errors.c   |  55 
 7 files changed, 1031 insertions(+), 16 deletions(-)
 create mode 100644 libbtrfsutil/COPYING
 create mode 100644 libbtrfsutil/COPYING.LESSER
 create mode 100644 libbtrfsutil/README.md
 create mode 100644 libbtrfsutil/btrfsutil.h
 create mode 100644 libbtrfsutil/errors.c

diff --git a/.gitignore b/.gitignore
index 8e607f6e..272d53e4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -43,6 +43,8 @@ libbtrfs.so.0.1
 library-test
 library-test-static
 /fssum
+/libbtrfsutil.so*
+/libbtrfsutil.a
 
 /tests/*-tests-results.txt
 /tests/test-console.txt
diff --git a/Makefile b/Makefile
index 6369e8f4..062f7f3c 100644
--- a/Makefile
+++ b/Makefile
@@ -73,6 +73,7 @@ CFLAGS = $(SUBST_CFLAGS) \
 -fPIC \
 -I$(TOPDIR) \
 -I$(TOPDIR)/kernel-lib \
+-I$(TOPDIR)/libbtrfsutil \
 $(EXTRAWARN_CFLAGS) \
 $(DEBUG_CFLAGS_INTERNAL) \
 $(EXTRA_CFLAGS)
@@ -121,12 +122,14 @@ libbtrfs_headers = send-stream.h send-utils.h send.h 
kernel-lib/rbtree.h btrfs-l
   kernel-lib/crc32c.h kernel-lib/list.h kerncompat.h \
   kernel-lib/radix-tree.h kernel-lib/sizes.h kernel-lib/raid56.h \
   extent-cache.h extent_io.h ioctl.h ctree.h btrfsck.h version.h
+libbtrfsutil_version := 0.1
+libbtrfsutil_objects = libbtrfsutil/errors.o
 convert_objects = convert/main.o convert/common.o convert/source-fs.o \
  convert/source-ext2.o convert/source-reiserfs.o
 mkfs_objects = mkfs/main.o mkfs/common.o
 image_objects = image/main.o image/sanitize.o
 all_objects = $(objects) $(cmds_objects) $(libbtrfs_objects) 
$(convert_objects) \
- $(mkfs_objects) $(image_objects)
+ $(mkfs_objects) $(image_objects) $(libbtrfsutil_objects)
 
 TESTS = fsck-tests.sh convert-tests.sh
 
@@ -248,10 +251,10 @@ static_convert_objects = $(patsubst %.o, %.static.o, 
$(convert_objects))
 static_mkfs_objects = $(patsubst %.o, %.static.o, $(mkfs_objects))
 static_image_objects = $(patsubst %.o, %.static.o, $(image_objects))
 
-libs_shared = libbtrfs.so.0.1
-libs_static = libbtrfs.a
+libs_shared = libbtrfs.so.0.1 libbtrfsutil.so.$(libbtrfsutil_version)
+libs_static = libbtrfs.a libbtrfsutil.a
 libs = $(libs_shared) $(libs_static)
-lib_links = libbtrfs.so.0 libbtrfs.so
+lib_links = libbtrfs.so.0 libbtrfs.so libbtrfsutil.so.0 libbtrfsutil.so
 headers = $(libbtrfs_headers)
 
 # make C=1 to enable sparse
@@ -289,7 +292,7 @@ endif
$(Q)$(CC) $(STATIC_CFLAGS) -c $< -o $@ $($(subst 
-,_,$(@:%.static.o=%)-cflags)) \
$($(subst -,_,btrfs-$(@:%/$(notdir $@)=%)-cflags))
 
-all: $(progs) libbtrfs $(BUILDDIRS)
+all: $(progs) $(libs) $(lib_links) $(BUILDDIRS)
 $(SUBDIRS): $(BUILDDIRS)
 $(BUILDDIRS):
@echo "Making all in $(patsubst build-%,%,$@)"
@@ -353,20 +356,31 @@ kernel-lib/tables.c:
@echo "[TABLE]  $@"
$(Q)./mktables > $@ || ($(RM) -f $@ && exit 1)
 
-libbtrfs: $(libs_shared) $(lib_links)
+libbtrfs.so.0.1: $(libbtrfs_objects)
+   @echo "[LD] $@"
+   $(Q)$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LIBBTRFS_LIBS) \
+   -shared -Wl,-soname,libbtrfs.so.0 -o $@
+
+libbtrfs.a: $(libbtrfs_objects)
+   @echo "[AR] $@"
+   $(Q)$(AR) cr $@ $^
+
+libbtrfs.so.0 libbtrfs.so: libbtrfs.so.0.1
+   @echo "[LN] $@"
+   $(Q)$(LN_S) -f $< $@
 
-$(libs_shared): $(libbtrfs_objects) $(lib_links) send.h
+libbtrfsutil.so.$(libbtrfsutil_version): $(libbtrfsutil_objects)
@echo "[LD] $@"
-   $(Q)$(CC) $(CFLAGS) $(libbtrfs_objects) $(LDFLAGS) $(LIBBTRFS_LIBS) \
-   -shared -Wl,-soname,libbtrfs.so.0 -o