[Libguestfs] [PATCH 1/5] New API: btrfs_scrub

[Pino Toscano]

On Monday 12 January 2015 11:45:02 Hu Tao wrote:
> On Wed, Jan 07, 2015 at 12:37:48PM +0100, Pino Toscano wrote:
> > In data venerdì 26 dicembre 2014 16:17:45, Hu Tao ha scritto:
> > > Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> > > ---
> > >  daemon/btrfs.c       | 32 ++++++++++++++++++++++++++++++++
> > >  generator/actions.ml | 17 +++++++++++++++++
> > >  src/MAX_PROC_NR      |  2 +-
> > >  3 files changed, 50 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> > > index 150c089..79de539 100644
> > > --- a/daemon/btrfs.c
> > > +++ b/daemon/btrfs.c
> > > @@ -1343,3 +1343,35 @@ do_btrfs_qgroup_remove (const char *src, const char *dst, const char *path)
> > >  
> > >    return 0;
> > >  }
> > > +
> > > +int
> > > +do_btrfs_scrub (const char *path)
> > > +{
> > > +  const size_t MAX_ARGS = 64;
> > > +  const char *argv[MAX_ARGS];
> > > +  size_t i = 0;
> > > +  CLEANUP_FREE char *path_buf = NULL;
> > > +  CLEANUP_FREE char *err = NULL;
> > > +  CLEANUP_FREE char *out = NULL;
> > > +  int r;
> > > +
> > > +  path_buf = sysroot_path (path);
> > > +  if (path_buf == NULL) {
> > > +    reply_with_perror ("malloc");
> > > +    return -1;
> > > +  }
> > > +
> > > +  ADD_ARG (argv, i, str_btrfs);
> > > +  ADD_ARG (argv, i, "scrub");
> > > +  ADD_ARG (argv, i, "start");
> > > +  ADD_ARG (argv, i, path_buf);
> > > +  ADD_ARG (argv, i, NULL);
> > > +
> > > +  r = commandv (&out, &err, argv);
> > > +  if (r == -1) {
> > > +    reply_with_error ("%s: %s", path, err);
> > > +    return -1;
> > > +  }
> > > +
> > > +  return 0;
> > > +}
> > 
> > Reading the btrfs documentation, it seems like this is an asynchronous
> > operation by default, so I'm not sure whether it would be useful to
> > just start it without any way to monitor its status.
> 
> btrfs balance has the same problem. I'll fix it if possible.

Thanks.

> > Also, what would it happen when you start a scrub operation on a
> > mounted filesystem, and then you either unmount it or shut down the
> > appliance while the scrubbing is still ongoing?
> 
> It probably corrupts the filesystem, but let me test it. Do we have any
> way to prevent the appliance from being shutdown if there is any async
> operation is ongoing?

I guess it depends on what "async operation" means.

All the own libguestfs communication (the commands) is sync, and
commands transferring files (upload, download, etc) can be aborted
from the library (see user_cancel) or in case of error from the
daemon side.
>From a quick glance, it seems such operations are not aborted when
shutting down, but IIRC we don't have thread safety on handles either.

Regarding all the other possible async operations (like the btrfs
scrubbing of this thread), unless libguestfs explicitly know about
them then they are implicitly interrupted/killed with the appliance
shutdown.

> > Another option could be making it synchronous (start -B), if it doesn't
> > take too much time.
> 
> I agree with you if the caller of the API doesn't care to wait.

This is the usual approach we have so far in e.g. file transfer APIs.

A better idea here would be adding support for async
operations/jobs...

-- 
Pino Toscano