Re: [PATCH 04/26] libbtrfsutil: add btrfs_util_is_subvolume() and btrfs_util_subvolume_id()

[Omar Sandoval]

On Thu, Feb 01, 2018 at 05:28:28PM +0100, David Sterba wrote:
> On Tue, Jan 30, 2018 at 08:54:08AM +0200, Nikolay Borisov wrote:
> > 
> > 
> > On 29.01.2018 23:43, Omar Sandoval wrote:
> > > On Mon, Jan 29, 2018 at 12:24:26PM +0200, Nikolay Borisov wrote:
> > >> On 26.01.2018 20:40, Omar Sandoval wrote:
> > > [snip]
> > >>> +/*
> > >>> + * This intentionally duplicates btrfs_util_f_is_subvolume() instead 
> > >>> of opening
> > >>> + * a file descriptor and calling it, because fstat() and fstatfs() 
> > >>> don't accept
> > >>> + * file descriptors opened with O_PATH on old kernels (before v3.6 and 
> > >>> before
> > >>> + * v3.12, respectively), but stat() and statfs() can be called on a 
> > >>> path that
> > >>> + * the user doesn't have read or write permissions to.
> > >>> + */
> > >>> +__attribute__((visibility("default")))
> > >>
> > >> Why do we need to explicitly set the attribute visibility to default,
> > >> isn't it implicitly default already?
> > > 
> > > Ah, I forgot to add -fvisibility=hidden to the build rule when I ported
> > > this to the btrfs-progs Makefile, that's why that's there. I'll add
> > > -fvisibility=hidden to the Makefile.
> > 
> > Right, it could be a good idea to hide the visibility attribute behind
> > an eloquent macro i.e. (PUBLIC|LIBRARY)_FUNC or some such.
> 
> Macro would be better (but is not needed for the initial version).
> 
> Alternatively the library .sym file can externally track the exported
> symbols and also track versioning.

I'll add a macro for this, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/26] libbtrfsutil: add btrfs_util_is_subvolume() and btrfs_util_subvolume_id()

[David Sterba]

On Tue, Jan 30, 2018 at 08:54:08AM +0200, Nikolay Borisov wrote:
> 
> 
> On 29.01.2018 23:43, Omar Sandoval wrote:
> > On Mon, Jan 29, 2018 at 12:24:26PM +0200, Nikolay Borisov wrote:
> >> On 26.01.2018 20:40, Omar Sandoval wrote:
> > [snip]
> >>> +/*
> >>> + * This intentionally duplicates btrfs_util_f_is_subvolume() instead of 
> >>> opening
> >>> + * a file descriptor and calling it, because fstat() and fstatfs() don't 
> >>> accept
> >>> + * file descriptors opened with O_PATH on old kernels (before v3.6 and 
> >>> before
> >>> + * v3.12, respectively), but stat() and statfs() can be called on a path 
> >>> that
> >>> + * the user doesn't have read or write permissions to.
> >>> + */
> >>> +__attribute__((visibility("default")))
> >>
> >> Why do we need to explicitly set the attribute visibility to default,
> >> isn't it implicitly default already?
> > 
> > Ah, I forgot to add -fvisibility=hidden to the build rule when I ported
> > this to the btrfs-progs Makefile, that's why that's there. I'll add
> > -fvisibility=hidden to the Makefile.
> 
> Right, it could be a good idea to hide the visibility attribute behind
> an eloquent macro i.e. (PUBLIC|LIBRARY)_FUNC or some such.

Macro would be better (but is not needed for the initial version).

Alternatively the library .sym file can externally track the exported
symbols and also track versioning.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/26] libbtrfsutil: add btrfs_util_is_subvolume() and btrfs_util_subvolume_id()

[Nikolay Borisov]

On 29.01.2018 23:43, Omar Sandoval wrote:
> On Mon, Jan 29, 2018 at 12:24:26PM +0200, Nikolay Borisov wrote:
>> On 26.01.2018 20:40, Omar Sandoval wrote:
> [snip]
>>> +/*
>>> + * This intentionally duplicates btrfs_util_f_is_subvolume() instead of 
>>> opening
>>> + * a file descriptor and calling it, because fstat() and fstatfs() don't 
>>> accept
>>> + * file descriptors opened with O_PATH on old kernels (before v3.6 and 
>>> before
>>> + * v3.12, respectively), but stat() and statfs() can be called on a path 
>>> that
>>> + * the user doesn't have read or write permissions to.
>>> + */
>>> +__attribute__((visibility("default")))
>>
>> Why do we need to explicitly set the attribute visibility to default,
>> isn't it implicitly default already?
> 
> Ah, I forgot to add -fvisibility=hidden to the build rule when I ported
> this to the btrfs-progs Makefile, that's why that's there. I'll add
> -fvisibility=hidden to the Makefile.

Right, it could be a good idea to hide the visibility attribute behind
an eloquent macro i.e. (PUBLIC|LIBRARY)_FUNC or some such.

> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/26] libbtrfsutil: add btrfs_util_is_subvolume() and btrfs_util_subvolume_id()

[Omar Sandoval]

On Mon, Jan 29, 2018 at 12:24:26PM +0200, Nikolay Borisov wrote:
> On 26.01.2018 20:40, Omar Sandoval wrote:
[snip]
> > +/*
> > + * This intentionally duplicates btrfs_util_f_is_subvolume() instead of 
> > opening
> > + * a file descriptor and calling it, because fstat() and fstatfs() don't 
> > accept
> > + * file descriptors opened with O_PATH on old kernels (before v3.6 and 
> > before
> > + * v3.12, respectively), but stat() and statfs() can be called on a path 
> > that
> > + * the user doesn't have read or write permissions to.
> > + */
> > +__attribute__((visibility("default")))
> 
> Why do we need to explicitly set the attribute visibility to default,
> isn't it implicitly default already?

Ah, I forgot to add -fvisibility=hidden to the build rule when I ported
this to the btrfs-progs Makefile, that's why that's there. I'll add
-fvisibility=hidden to the Makefile.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/26] libbtrfsutil: add btrfs_util_is_subvolume() and btrfs_util_subvolume_id()

[Nikolay Borisov]

On 26.01.2018 20:40, Omar Sandoval wrote:
> From: Omar Sandoval 
> 
> These are the most trivial helpers in the library and will be used to
> implement several of the more involved functions.
> 
> Signed-off-by: Omar Sandoval 
> ---
>  Makefile|   2 +-
>  libbtrfsutil/btrfsutil.h|  33 +++
>  libbtrfsutil/python/btrfsutilpy.h   |   3 +
>  libbtrfsutil/python/module.c|  12 +++
>  libbtrfsutil/python/setup.py|   1 +
>  libbtrfsutil/python/subvolume.c |  73 +++
>  libbtrfsutil/python/tests/__init__.py   |  66 ++
>  libbtrfsutil/python/tests/test_subvolume.py |  57 
>  libbtrfsutil/subvolume.c| 137 
> 
>  9 files changed, 383 insertions(+), 1 deletion(-)
>  create mode 100644 libbtrfsutil/python/subvolume.c
>  create mode 100644 libbtrfsutil/python/tests/test_subvolume.py
>  create mode 100644 libbtrfsutil/subvolume.c
> 
> diff --git a/Makefile b/Makefile
> index 02b03e81..48a558a9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -123,7 +123,7 @@ libbtrfs_headers = send-stream.h send-utils.h send.h 
> kernel-lib/rbtree.h btrfs-l
>  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
> +libbtrfsutil_objects = libbtrfsutil/errors.o libbtrfsutil/subvolume.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
> diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
> index fe1091ca..dff6599d 100644
> --- a/libbtrfsutil/btrfsutil.h
> +++ b/libbtrfsutil/btrfsutil.h
> @@ -20,6 +20,8 @@
>  #ifndef BTRFS_UTIL_H
>  #define BTRFS_UTIL_H
>  
> +#include 
> +
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
> @@ -65,6 +67,37 @@ enum btrfs_util_error {
>   */
>  const char *btrfs_util_strerror(enum btrfs_util_error err);
>  
> +/**
> + * btrfs_util_is_subvolume() - Return whether a given path is a Btrfs 
> subvolume.
> + * @path: Path to check.
> + *
> + * Return: %BTRFS_UTIL_OK if @path is a Btrfs subvolume,
> + * %BTRFS_UTIL_ERROR_NOT_BTRFS if @path is not on a Btrfs filesystem,
> + * %BTRFS_UTIL_ERROR_NOT_SUBVOLUME if @path is not a subvolume, non-zero 
> error
> + * code on any other failure.
> + */
> +enum btrfs_util_error btrfs_util_is_subvolume(const char *path);
> +
> +/**
> + * btrfs_util_f_is_subvolume() - See btrfs_util_is_subvolume().
> + */
> +enum btrfs_util_error btrfs_util_f_is_subvolume(int fd);
> +
> +/**
> + * btrfs_util_subvolume_id() - Get the ID of the subvolume containing a path.
> + * @path: Path on a Btrfs filesystem.
> + * @id_ret: Returned subvolume ID.
> + *
> + * Return: %BTRFS_UTIL_OK on success, non-zero error code on failure.
> + */
> +enum btrfs_util_error btrfs_util_subvolume_id(const char *path,
> +   uint64_t *id_ret);
> +
> +/**
> + * btrfs_util_f_subvolume_id() - See btrfs_util_subvolume_id().
> + */
> +enum btrfs_util_error btrfs_util_f_subvolume_id(int fd, uint64_t *id_ret);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/libbtrfsutil/python/btrfsutilpy.h 
> b/libbtrfsutil/python/btrfsutilpy.h
> index 6d82f7e1..9a04fda7 100644
> --- a/libbtrfsutil/python/btrfsutilpy.h
> +++ b/libbtrfsutil/python/btrfsutilpy.h
> @@ -52,6 +52,9 @@ void SetFromBtrfsUtilErrorWithPaths(enum btrfs_util_error 
> err,
>   struct path_arg *path1,
>   struct path_arg *path2);
>  
> +PyObject *is_subvolume(PyObject *self, PyObject *args, PyObject *kwds);
> +PyObject *subvolume_id(PyObject *self, PyObject *args, PyObject *kwds);
> +
>  void add_module_constants(PyObject *m);
>  
>  #endif /* BTRFSUTILPY_H */
> diff --git a/libbtrfsutil/python/module.c b/libbtrfsutil/python/module.c
> index d7398808..d492cbc7 100644
> --- a/libbtrfsutil/python/module.c
> +++ b/libbtrfsutil/python/module.c
> @@ -132,6 +132,18 @@ void path_cleanup(struct path_arg *path)
>  }
>  
>  static PyMethodDef btrfsutil_methods[] = {
> + {"is_subvolume", (PyCFunction)is_subvolume,
> +  METH_VARARGS | METH_KEYWORDS,
> +  "is_subvolume(path) -> bool\n\n"
> +  "Get whether a file is a subvolume.\n\n"
> +  "Arguments:\n"
> +  "path -- string, bytes, path-like object, or open file descriptor"},
> + {"subvolume_id", (PyCFunction)subvolume_id,
> +  METH_VARARGS | METH_KEYWORDS,
> +  "subvolume_id(path) -> int\n\n"
> +  "Get the ID of the subvolume containing a file.\n\n"
> +  "Arguments:\n"
> +  "path -- string, bytes, path-like object, or open file descriptor"},
>   {},
>  };
>  
> diff --git a/libbtrfsutil/python/setup.py