[Libguestfs] [PATCH v3.1 1/9] uuid: add support to change uuid of btrfs partition

[Chen, Hanxiao]

> -----Original Message-----
> From: libguestfs-bounces at redhat.com [mailto:libguestfs-bounces at redhat.com] On
> Behalf Of Pino Toscano
> Sent: Friday, June 26, 2015 8:12 PM
> To: libguestfs at redhat.com
> Subject: Re: [Libguestfs] [PATCH v3.1 1/9] uuid: add support to change uuid of btrfs
> partition
> 
> In data venerdì 26 giugno 2015 17:35:36, Chen Hanxiao ha scritto:
> > btrfs-progs v4.1 add support to change uuid of btrfs fs.
> >
> > Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> > ---
> > v3.1: fix a typo
> > v3: set errno as ENOTSUP when btrfstune -u is not available
> > v2: put btrfs operation back to daemon/btrfs.c
> >     move tests to tests/btrfs
> >
> >  daemon/btrfs.c                 | 60 ++++++++++++++++++++++++++++++++++++++++++
> >  daemon/daemon.h                |  1 +
> >  daemon/uuids.c                 |  6 ++---
> >  tests/btrfs/test-btrfs-misc.pl | 17 ++++++++++++
> >  4 files changed, 80 insertions(+), 4 deletions(-)
> >
> > diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> > index 20e5e6b..a69c512 100644
> > --- a/daemon/btrfs.c
> > +++ b/daemon/btrfs.c
> > @@ -790,6 +790,44 @@ do_btrfs_device_delete (char *const *devices, const char
> *fs)
> >    return 0;
> >  }
> >
> > +
> > +/* btrfstune command add two new options
> > + * -U UUID      change fsid to UUID
> > + * -u           change fsid, use a random one
> > + * since v4.1
> > + * We could check wheter 'btrfstune' support
> > + * '-u' and '-U UUID' option by checking the output of
> > + * 'btrfstune' command.
> > + */
> > +static int
> > +test_btrfstune_uuid_opt (void)
> > +{
> > +  static int result = -1;
> > +  if (result != -1)
> > +    return result;
> > +
> > +  CLEANUP_FREE char *err = NULL;
> > +
> > +  int r = commandr (NULL, &err, str_btrfstune, NULL);
> > +
> > +  if (r == -1) {
> > +    reply_with_error ("btrfstune: %s", err);
> > +    return -1;
> > +  }
> > +
> > +  /* FIXME currently btrfstune do not support '--help'.
> 
> So --help should be passed anyway to the btrfstune invocation, so with
> newer versions hopefully there will not be complaints about it anymore.

Fine.

> 
[snip]
> > diff --git a/tests/btrfs/test-btrfs-misc.pl b/tests/btrfs/test-btrfs-misc.pl
> > index 2fe7c59..b47caab 100755
> > --- a/tests/btrfs/test-btrfs-misc.pl
> > +++ b/tests/btrfs/test-btrfs-misc.pl
> > @@ -20,6 +20,7 @@
> >
> >  use strict;
> >  use warnings;
> > +use Errno;
> >
> >  use Sys::Guestfs;
> >
> > @@ -47,5 +48,21 @@ my $label = $g->vfs_label ("/dev/sda1");
> >  die "unexpected label: expecting 'newlabel' but got '$label'"
> >      unless $label eq "newlabel";
> >
> > +# Setting btrfs UUID
> > +eval {
> > +    $g->set_uuid ("/dev/sda1", "12345678-1234-1234-1234-123456789012");
> > +};
> > +# FIXME: ignore ESRCH
> 
> What is ESRCH about?

The problem is every time I got a ESRCH here.
I have no idea about this.

Can we just ignore it by:
} elsif ($err == Errno::ESRCH()) {
    ;
} else {
   die $@;
}

> 
> > +my $err = $g->last_errno ();
> > +
> > +if ($err == 0) {
> > +    my $uuid = $g->vfs_uuid ("/dev/sda1");
> > +    die "unexpected uuid expecting
> > +     '12345678-1234-1234-1234-123456789012' but got '$uuid'"
> > +    unless $uuid eq "12345678-1234-1234-1234-123456789012";
> > +} elsif ($err == Errno::ENOTSUP()) {
> > +    warn "$0: skipping test for btrfs UUID change feature is not available\n";
> > +}
> 
> IIRC 'warn' prints the newline at the end already.
> 
> This is missing to check when $err is a valid error.
> 

Can we just:
} else {
   die $@;
}

or some hints?

Regards,
- Chen