In data venerdì 26 dicembre 2014 16:17:45, Hu Tao ha scritto:> Signed-off-by: Hu Tao <hutao@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. 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? Another option could be making it synchronous (start -B), if it doesn't take too much time. Thanks, -- Pino Toscano
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@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.> > 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?> > 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. Regards, Hu
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@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 whenshutting 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