Libguestfs - Feb 2016 - Re: [PATCH] listfs: ignore the default btrfs subvolume

When listing the subvolumes of a btrfs filesystem, ignore the default
subvolume: we get the content of it when mounting the whole device
(without specifying any particular subvolume), so avoid listing it
twice.
---
 src/listfs.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/listfs.c b/src/listfs.c
index 98e74c7..de3b6f5 100644
--- a/src/listfs.c
+++ b/src/listfs.c
@@ -169,8 +169,17 @@ check_with_vfs_type (guestfs_h *g, const char *device,
struct stringsbuf *sb)
     if (vols == NULL)
       return -1;
 
+    int64_t default_volume = guestfs_btrfs_subvolume_get_default (g, device);
+
     for (size_t i = 0; i < vols->len; i++) {
       struct guestfs_btrfssubvolume *this = &vols->val[i];
+
+      /* Ignore the default subvolume.  We get it by simply mounting
+       * the whole device of this btrfs filesystem.
+       */
+      if (this->btrfssubvolume_id == (uint64_t) default_volume)
+        continue;
+
       guestfs_int_add_sprintf (g, sb,
 			       "btrfsvol:%s/%s",
 			       device, this->btrfssubvolume_path);
-- 
2.5.0

The patch does the trick for virt-builder on the images I have here. I
hope I'm not overlooking some potential side effects.

Weak ACK from me.

--
Cedric

On Thu, 2016-02-25 at 17:50 +0100, Pino Toscano wrote:
> When listing the subvolumes of a btrfs filesystem, ignore the default
> subvolume: we get the content of it when mounting the whole device
> (without specifying any particular subvolume), so avoid listing it
> twice.
> ---
>  src/listfs.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/listfs.c b/src/listfs.c
> index 98e74c7..de3b6f5 100644
> --- a/src/listfs.c
> +++ b/src/listfs.c
> @@ -169,8 +169,17 @@ check_with_vfs_type (guestfs_h *g, const char
> *device, struct stringsbuf *sb)
>      if (vols == NULL)
>        return -1;
>  
> +    int64_t default_volume = guestfs_btrfs_subvolume_get_default (g,
> device);
> +
>      for (size_t i = 0; i < vols->len; i++) {
>        struct guestfs_btrfssubvolume *this = &vols->val[i];
> +
> +      /* Ignore the default subvolume.  We get it by simply mounting
> +       * the whole device of this btrfs filesystem.
> +       */
> +      if (this->btrfssubvolume_id == (uint64_t) default_volume)
> +        continue;
> +
>        guestfs_int_add_sprintf (g, sb,
>  			       "btrfsvol:%s/%s",
>  			       device, this->btrfssubvolume_path);

On Thu, Feb 25, 2016 at 05:50:57PM +0100, Pino Toscano
wrote:
> When listing the subvolumes of a btrfs filesystem, ignore the default
> subvolume: we get the content of it when mounting the whole device
> (without specifying any particular subvolume), so avoid listing it
> twice.
> ---
>  src/listfs.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/listfs.c b/src/listfs.c
> index 98e74c7..de3b6f5 100644
> --- a/src/listfs.c
> +++ b/src/listfs.c
> @@ -169,8 +169,17 @@ check_with_vfs_type (guestfs_h *g, const char *device,
struct stringsbuf *sb)
>      if (vols == NULL)
>        return -1;
>  
> +    int64_t default_volume = guestfs_btrfs_subvolume_get_default (g,
device);
> +
>      for (size_t i = 0; i < vols->len; i++) {
>        struct guestfs_btrfssubvolume *this = &vols->val[i];
> +
> +      /* Ignore the default subvolume.  We get it by simply mounting
> +       * the whole device of this btrfs filesystem.
> +       */
> +      if (this->btrfssubvolume_id == (uint64_t) default_volume)
> +        continue;
> +
>        guestfs_int_add_sprintf (g, sb,
>  			       "btrfsvol:%s/%s",
>  			       device, this->btrfssubvolume_path);
ACK.

Was there an RHBZ# for this?  Can't seem to find it ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Thu, 2016-02-25 at 19:10 +0000, Richard W.M. Jones
wrote:
> On Thu, Feb 25, 2016 at 05:50:57PM +0100, Pino Toscano wrote:
> > When listing the subvolumes of a btrfs filesystem, ignore the
> > default
> > subvolume: we get the content of it when mounting the whole device
> > (without specifying any particular subvolume), so avoid listing it
> > twice.
> > ---
> >  src/listfs.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/src/listfs.c b/src/listfs.c
> > index 98e74c7..de3b6f5 100644
> > --- a/src/listfs.c
> > +++ b/src/listfs.c
> > @@ -169,8 +169,17 @@ check_with_vfs_type (guestfs_h *g, const char
> > *device, struct stringsbuf *sb)
> >      if (vols == NULL)
> >        return -1;
> >  
> > +    int64_t default_volume = guestfs_btrfs_subvolume_get_default
> > (g, device);
> > +
> >      for (size_t i = 0; i < vols->len; i++) {
> >        struct guestfs_btrfssubvolume *this = &vols->val[i];
> > +
> > +      /* Ignore the default subvolume.  We get it by simply
> > mounting
> > +       * the whole device of this btrfs filesystem.
> > +       */
> > +      if (this->btrfssubvolume_id == (uint64_t) default_volume)
> > +        continue;
> > +
> >        guestfs_int_add_sprintf (g, sb,
> >  			       "btrfsvol:%s/%s",
> >  			       device, this->btrfssubvolume_path);
> 
> ACK.
> 
> Was there an RHBZ# for this?  Can't seem to find it ...
I least none that I am aware of, not even a bsc# as we have an pretty
old version on openSUSE. I just got the problem with my testing of
recent releases.

--
Cedric

On Thu, Feb 25, Richard W.M. Jones wrote:
> Was there an RHBZ# for this?  Can't seem to find it ...
Maybe it helps to improve btrfs support. As stated earlier, btrfs is a
container of things instead of a plain filesystem. Its unfortunate that
the default subvolume was invented, its perhaps just there to make it
look like a filesystem.

The bug number is https://bugzilla.suse.com/show_bug.cgi?id=900346

Olaf

On Thursday 25 February 2016 19:10:39 Richard W.M. Jones
wrote:
> On Thu, Feb 25, 2016 at 05:50:57PM +0100, Pino Toscano wrote:
> > When listing the subvolumes of a btrfs filesystem, ignore the default
> > subvolume: we get the content of it when mounting the whole device
> > (without specifying any particular subvolume), so avoid listing it
> > twice.
> > ---
> >  src/listfs.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/src/listfs.c b/src/listfs.c
> > index 98e74c7..de3b6f5 100644
> > --- a/src/listfs.c
> > +++ b/src/listfs.c
> > @@ -169,8 +169,17 @@ check_with_vfs_type (guestfs_h *g, const char
*device, struct stringsbuf *sb)
> >      if (vols == NULL)
> >        return -1;
> >  
> > +    int64_t default_volume = guestfs_btrfs_subvolume_get_default (g,
device);
> > +
> >      for (size_t i = 0; i < vols->len; i++) {
> >        struct guestfs_btrfssubvolume *this = &vols->val[i];
> > +
> > +      /* Ignore the default subvolume.  We get it by simply mounting
> > +       * the whole device of this btrfs filesystem.
> > +       */
> > +      if (this->btrfssubvolume_id == (uint64_t) default_volume)
> > +        continue;
> > +
> >        guestfs_int_add_sprintf (g, sb,
> >  			       "btrfsvol:%s/%s",
> >  			       device, this->btrfssubvolume_path);
> 
> ACK.
> 
> Was there an RHBZ# for this?  Can't seem to find it ...
#824021 seems to be similar/close, although I cannot reproduce it even
without this patch (i.e. inspection works in a Fedora + btrfs layout).

-- 
Pino Toscano