[PATCH] btrfs-progs: add option to run balance as daemon

[PATCH] btrfs-progs: add option to run balance as daemon

[Austin S. Hemmelgarn]

Currently, balance operations are run synchronously in the foreground.
This is nice for interactive management, but is kind of crappy when you
start looking at automation and similar things.

This patch adds an option to `btrfs balance start` to tell it to
daemonize prior to running the balance operation, thus allowing us to
preform balances asynchronously.  The two biggest use cases I have for
this are starting a balance on a remote server without establishing a
full shell session, and being able to background the balance in a
recovery shell (which usually has no job control) so I can still get
progress information.

Because it simply daemonizes prior to calling the balance ioctl, this
doesn't actually need any kernel support.

Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
---
This works as is, but there are two specific things I would love to
eventually fix but don't have the time to fix right now:
* There is no way to get any feedback from the balance operation.
* Because of how everything works, trying to start a new balance with
  --background while one iw already running won't return an error but
  won't queue or start a new balance either.

The first one is more a utility item than anything else, and probably
would not be hard to add.  Ideally, it should be output to a user
specified file, and this should work even for a normal foreground balance.

The second is very much a UX issue, but can't be easily sovled without
doing some creative process monitoring from the parrent processes.

 Documentation/btrfs-balance.asciidoc |  2 ++
 cmds-balance.c                       | 43 +++++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-balance.asciidoc b/Documentation/btrfs-balance.asciidoc
index 7df40b9..f487dbb 100644
--- a/Documentation/btrfs-balance.asciidoc
+++ b/Documentation/btrfs-balance.asciidoc
@@ -85,6 +85,8 @@ act on system chunks (requires '-f'), see `FILTERS` section for details about 'f
 be verbose and print balance filter arguments
 -f::::
 force reducing of metadata integrity, eg. when going from 'raid1' to 'single'
+--background::::
+run the balance operation asynchronously in the background
 
 *status* [-v] <path>::
 Show status of running or paused balance.
diff --git a/cmds-balance.c b/cmds-balance.c
index 708bbf4..66169b7 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -20,6 +20,9 @@
 #include <unistd.h>
 #include <getopt.h>
 #include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 #include <errno.h>
 
 #include "kerncompat.h"
@@ -510,6 +513,7 @@ static const char * const cmd_balance_start_usage[] = {
 	"-v             be verbose",
 	"-f             force reducing of metadata integrity",
 	"--full-balance do not print warning and do not delay start",
+	"--background   run the balance as a background process",
 	NULL
 };
 
@@ -520,6 +524,7 @@ static int cmd_balance_start(int argc, char **argv)
 						&args.meta, NULL };
 	int force = 0;
 	int verbose = 0;
+	int background = 0;
 	unsigned start_flags = 0;
 	int i;
 
@@ -527,7 +532,8 @@ static int cmd_balance_start(int argc, char **argv)
 
 	optind = 1;
 	while (1) {
-		enum { GETOPT_VAL_FULL_BALANCE = 256 };
+		enum { GETOPT_VAL_FULL_BALANCE = 256,
+			GETOPT_VAL_BACKGROUND = 257 };
 		static const struct option longopts[] = {
 			{ "data", optional_argument, NULL, 'd'},
 			{ "metadata", optional_argument, NULL, 'm' },
@@ -536,6 +542,8 @@ static int cmd_balance_start(int argc, char **argv)
 			{ "verbose", no_argument, NULL, 'v' },
 			{ "full-balance", no_argument, NULL,
 				GETOPT_VAL_FULL_BALANCE },
+			{ "background", no_argument, NULL,
+				GETOPT_VAL_BACKGROUND },
 			{ NULL, 0, NULL, 0 }
 		};
 
@@ -574,6 +582,9 @@ static int cmd_balance_start(int argc, char **argv)
 		case GETOPT_VAL_FULL_BALANCE:
 			start_flags |= BALANCE_START_NOWARN;
 			break;
+		case GETOPT_VAL_BACKGROUND:
+			background = 1;
+			break;
 		default:
 			usage(cmd_balance_start_usage);
 		}
@@ -626,6 +637,36 @@ static int cmd_balance_start(int argc, char **argv)
 		args.flags |= BTRFS_BALANCE_FORCE;
 	if (verbose)
 		dump_ioctl_balance_args(&args);
+	if (background) {
+		switch (fork()) {
+		case (-1):
+			error("Unable to fork to run balance in background");
+			return 1;
+			break;
+		case (0):
+			setsid();
+			switch(fork()) {
+				case (-1):
+					error("Unable to fork to run balance in background");
+					exit(1);
+					break;
+				case (0):
+					chdir("/");
+					close(0);
+					close(1);
+					close(2);
+					open("/dev/null", O_RDONLY);
+					open("/dev/null", O_WRONLY);
+					open("/dev/null", O_WRONLY);
+					break;
+				default:
+					exit(0);
+			}
+			break;
+		default:
+			exit(0);
+		}
+	}
 
 	return do_balance(argv[optind], &args, start_flags);
 }
-- 
2.9.0

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Satoru Takeuchi]

On 2016/06/22 0:16, Austin S. Hemmelgarn wrote:
> Currently, balance operations are run synchronously in the foreground.
> This is nice for interactive management, but is kind of crappy when you
> start looking at automation and similar things.
> 
> This patch adds an option to `btrfs balance start` to tell it to
> daemonize prior to running the balance operation, thus allowing us to
> preform balances asynchronously.  The two biggest use cases I have for
> this are starting a balance on a remote server without establishing a
> full shell session, and being able to background the balance in a
> recovery shell (which usually has no job control) so I can still get
> progress information.
> 
> Because it simply daemonizes prior to calling the balance ioctl, this
> doesn't actually need any kernel support.
> 
> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> ---
> This works as is, but there are two specific things I would love to
> eventually fix but don't have the time to fix right now:
> * There is no way to get any feedback from the balance operation.
> * Because of how everything works, trying to start a new balance with
>   --background while one iw already running won't return an error but
>   won't queue or start a new balance either.
> 
> The first one is more a utility item than anything else, and probably
> would not be hard to add.  Ideally, it should be output to a user
> specified file, and this should work even for a normal foreground balance.
> 
> The second is very much a UX issue, but can't be easily sovled without
> doing some creative process monitoring from the parrent processes.
> 
>  Documentation/btrfs-balance.asciidoc |  2 ++
>  cmds-balance.c                       | 43 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/btrfs-balance.asciidoc b/Documentation/btrfs-balance.asciidoc
> index 7df40b9..f487dbb 100644
> --- a/Documentation/btrfs-balance.asciidoc
> +++ b/Documentation/btrfs-balance.asciidoc
> @@ -85,6 +85,8 @@ act on system chunks (requires '-f'), see `FILTERS` section for details about 'f
>  be verbose and print balance filter arguments
>  -f::::
>  force reducing of metadata integrity, eg. when going from 'raid1' to 'single'
> +--background::::
> +run the balance operation asynchronously in the background
>  
>  *status* [-v] <path>::
>  Show status of running or paused balance.
> diff --git a/cmds-balance.c b/cmds-balance.c
> index 708bbf4..66169b7 100644
> --- a/cmds-balance.c
> +++ b/cmds-balance.c
> @@ -20,6 +20,9 @@
>  #include <unistd.h>
>  #include <getopt.h>
>  #include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
>  #include <errno.h>
>  
>  #include "kerncompat.h"
> @@ -510,6 +513,7 @@ static const char * const cmd_balance_start_usage[] = {
>  	"-v             be verbose",
>  	"-f             force reducing of metadata integrity",
>  	"--full-balance do not print warning and do not delay start",
> +	"--background   run the balance as a background process",
>  	NULL
>  };
>  
> @@ -520,6 +524,7 @@ static int cmd_balance_start(int argc, char **argv)
>  						&args.meta, NULL };
>  	int force = 0;
>  	int verbose = 0;
> +	int background = 0;
>  	unsigned start_flags = 0;
>  	int i;
>  
> @@ -527,7 +532,8 @@ static int cmd_balance_start(int argc, char **argv)
>  
>  	optind = 1;
>  	while (1) {
> -		enum { GETOPT_VAL_FULL_BALANCE = 256 };
> +		enum { GETOPT_VAL_FULL_BALANCE = 256,
> +			GETOPT_VAL_BACKGROUND = 257 };
>  		static const struct option longopts[] = {
>  			{ "data", optional_argument, NULL, 'd'},
>  			{ "metadata", optional_argument, NULL, 'm' },
> @@ -536,6 +542,8 @@ static int cmd_balance_start(int argc, char **argv)
>  			{ "verbose", no_argument, NULL, 'v' },
>  			{ "full-balance", no_argument, NULL,
>  				GETOPT_VAL_FULL_BALANCE },
> +			{ "background", no_argument, NULL,
> +				GETOPT_VAL_BACKGROUND },
>  			{ NULL, 0, NULL, 0 }
>  		};
>  
> @@ -574,6 +582,9 @@ static int cmd_balance_start(int argc, char **argv)
>  		case GETOPT_VAL_FULL_BALANCE:
>  			start_flags |= BALANCE_START_NOWARN;
>  			break;
> +		case GETOPT_VAL_BACKGROUND:
> +			background = 1;
> +			break;
>  		default:
>  			usage(cmd_balance_start_usage);
>  		}
> @@ -626,6 +637,36 @@ static int cmd_balance_start(int argc, char **argv)
>  		args.flags |= BTRFS_BALANCE_FORCE;
>  	if (verbose)
>  		dump_ioctl_balance_args(&args);
> +	if (background) {
> +		switch (fork()) {
> +		case (-1):
> +			error("Unable to fork to run balance in background");
> +			return 1;
> +			break;
> +		case (0):
> +			setsid();
> +			switch(fork()) {
> +				case (-1):
> +					error("Unable to fork to run balance in background");
> +					exit(1);
> +					break;
> +				case (0):
> +					chdir("/");

You should check the return value of chdir(). Otherwise
we get the following warning message at the build time.

=================================
cmds-balance.c: In function 'cmd_balance_start':
cmds-balance.c:654:6: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result]
      chdir("/");
      ^
=================================

I found this warning message at
integration-20160704(2355a7e5dcdf122d1924).

Thanks,
Satoru

> +					close(0);
> +					close(1);
> +					close(2);
> +					open("/dev/null", O_RDONLY);
> +					open("/dev/null", O_WRONLY);
> +					open("/dev/null", O_WRONLY);
> +					break;
> +				default:
> +					exit(0);
> +			}
> +			break;
> +		default:
> +			exit(0);
> +		}
> +	}
>  
>  	return do_balance(argv[optind], &args, start_flags);
>  }
> 

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[David Sterba]

On Mon, Jul 11, 2016 at 10:44:30AM +0900, Satoru Takeuchi wrote:
> > +					chdir("/");
> 
> You should check the return value of chdir(). Otherwise
> we get the following warning message at the build time.

Can we actually fail to change directory to '/' ? Otherwise I think the
warning should be silenced so the build is clean.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Austin S. Hemmelgarn]

On 2016-07-26 13:07, David Sterba wrote:
> On Mon, Jul 11, 2016 at 10:44:30AM +0900, Satoru Takeuchi wrote:
>>> +					chdir("/");
>>
>> You should check the return value of chdir(). Otherwise
>> we get the following warning message at the build time.
>
> Can we actually fail to change directory to '/' ? Otherwise I think the
> warning should be silenced so the build is clean.
It is theoretically possible, but in practice it means you either have 
failing hardware or your kernel is broken somehow, both of which should 
be causing all kinds of other issues, and thus we don't really gain 
anything from trying to catch the error.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Tomasz Torcz]

On Tue, Jun 21, 2016 at 11:16:59AM -0400, Austin S. Hemmelgarn wrote:
> Currently, balance operations are run synchronously in the foreground.
> This is nice for interactive management, but is kind of crappy when you
> start looking at automation and similar things.
 
  It can be done with simplest systemd unit file:
btrfs-balance@.service:
---
[Unit]
Description=btrfs balance for %I

[Service]
ExecStart=/usr/bin/btrfs balance start %I
ExecStop=/usr/bin/btrfs balance cancel %I
---

  It automates quite nicely and needs no additional code.

-- 
Tomasz Torcz                "Funeral in the morning, IDE hacking
xmpp: zdzichubg@chrome.pl    in the afternoon and evening." - Alan Cox

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Austin S. Hemmelgarn]

On 2016-07-11 03:26, Tomasz Torcz wrote:
> On Tue, Jun 21, 2016 at 11:16:59AM -0400, Austin S. Hemmelgarn wrote:
>> Currently, balance operations are run synchronously in the foreground.
>> This is nice for interactive management, but is kind of crappy when you
>> start looking at automation and similar things.
>
>   It can be done with simplest systemd unit file:
> btrfs-balance@.service:
> ---
> [Unit]
> Description=btrfs balance for %I
>
> [Service]
> ExecStart=/usr/bin/btrfs balance start %I
> ExecStop=/usr/bin/btrfs balance cancel %I
> ---
>
>   It automates quite nicely and needs no additional code.
>
It's also entirely dependent on a couple of things:
1. You're running systemd (not everyone is, I'm certainly not).
2. You're only dealing with the local system.

The type of situation I'm thinking of is dealing with non-local systems. 
  For example, running something like this:
ssh user@remotehost btrfs balance start --background /
Keeping the SSH connection open for the duration of the balance has 
issues for some people (may close without keep-alive set, uses network 
bandwidth with keep-alive set, many people who are hosted have bandwidth 
quotas still), and it's extremely useful to have the option to fire and 
forget.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Tomasz Torcz]

On Mon, Jul 11, 2016 at 07:17:28AM -0400, Austin S. Hemmelgarn wrote:
> On 2016-07-11 03:26, Tomasz Torcz wrote:
> > On Tue, Jun 21, 2016 at 11:16:59AM -0400, Austin S. Hemmelgarn wrote:
> > > Currently, balance operations are run synchronously in the foreground.
> > > This is nice for interactive management, but is kind of crappy when you
> > > start looking at automation and similar things.
> > 
> >   It can be done with simplest systemd unit file:
> > btrfs-balance@.service:
> > ---
> > [Unit]
> > Description=btrfs balance for %I
> > 
> > [Service]
> > ExecStart=/usr/bin/btrfs balance start %I
> > ExecStop=/usr/bin/btrfs balance cancel %I
> > ---
> > 
> >   It automates quite nicely and needs no additional code.
> > 
> It's also entirely dependent on a couple of things:
> 1. You're running systemd (not everyone is, I'm certainly not).

  So instead of using widespread, tested code, you re-implement 
parts of it.  BTW, your patch for daemonizing does only 5 steps
out of 15 described in man 7 daemon.

> 2. You're only dealing with the local system.
> 
> The type of situation I'm thinking of is dealing with non-local systems.
> For example, running something like this:
> ssh user@remotehost btrfs balance start --background /
> Keeping the SSH connection open for the duration of the balance has issues
> for some people (may close without keep-alive set, uses network bandwidth
> with keep-alive set, many people who are hosted have bandwidth quotas
> still), and it's extremely useful to have the option to fire and forget.

  I don't get the local part.  Right now, when using above unit you can

ssh user@remotehost systemctl start btrfs-balance@-

(or even
systemctl -H user@remotehost start btrfs-balance@-)

 and balance for / runs in background on target host. With clean
environment, logs being captured, locking against multiple
startups and so on. Right now, without any additional code.

-- 
Tomasz   .. oo o.   oo o. .o   .o o. o. oo o.   ..
Torcz    .. .o .o   .o .o oo   oo .o .. .. oo   oo
o.o.o.   .o .. o.   o. o. o.   o. o. oo .. ..   o.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Austin S. Hemmelgarn]

On 2016-07-11 12:58, Tomasz Torcz wrote:
> On Mon, Jul 11, 2016 at 07:17:28AM -0400, Austin S. Hemmelgarn wrote:
>> On 2016-07-11 03:26, Tomasz Torcz wrote:
>>> On Tue, Jun 21, 2016 at 11:16:59AM -0400, Austin S. Hemmelgarn wrote:
>>>> Currently, balance operations are run synchronously in the foreground.
>>>> This is nice for interactive management, but is kind of crappy when you
>>>> start looking at automation and similar things.
>>>
>>>   It can be done with simplest systemd unit file:
>>> btrfs-balance@.service:
>>> ---
>>> [Unit]
>>> Description=btrfs balance for %I
>>>
>>> [Service]
>>> ExecStart=/usr/bin/btrfs balance start %I
>>> ExecStop=/usr/bin/btrfs balance cancel %I
>>> ---
>>>
>>>   It automates quite nicely and needs no additional code.
>>>
>> It's also entirely dependent on a couple of things:
>> 1. You're running systemd (not everyone is, I'm certainly not).
>
>   So instead of using widespread, tested code, you re-implement
> parts of it.  BTW, your patch for daemonizing does only 5 steps
> out of 15 described in man 7 daemon.
Sysvinit is more widely tested and used than systemd has ever been. 
Systemd's been around for only a few years, sysvinit has been around for 
decades.  I'm not going to add a dependency on something that's just 
popular for the moment, especially when that thing is such a point of 
contention for so many people.

As far as daemonization, I have no man-page called daemon in section 
seven, yet I have an up-to-date upstream copy of the Linux man pages. 
My guess is that this is a systemd man page, which in turn means it does 
a bunch of stuff that's only needed for system services on systemd based 
systems.  The method I used, in a slightly different form (most people 
use if instead of switch), is pretty much identical to what's been done 
since before SVR4 for daemonization.  In theory, I could use libdaemon, 
but that adds a new dependency.  In theory I could use the daemon() 
function from unistd.h, but the implementation in glibc doesn't work 
correctly (it only forks once).  This doesn't do special handling for 
systemd monitoring, because it doesn't need to.  It's not a system 
service, it's just a background process.
>
>> 2. You're only dealing with the local system.
>>
>> The type of situation I'm thinking of is dealing with non-local systems.
>> For example, running something like this:
>> ssh user@remotehost btrfs balance start --background /
>> Keeping the SSH connection open for the duration of the balance has issues
>> for some people (may close without keep-alive set, uses network bandwidth
>> with keep-alive set, many people who are hosted have bandwidth quotas
>> still), and it's extremely useful to have the option to fire and forget.
>
>   I don't get the local part.  Right now, when using above unit you can
>
> ssh user@remotehost systemctl start btrfs-balance@-
>
> (or even
> systemctl -H user@remotehost start btrfs-balance@-)
>
>  and balance for / runs in background on target host. With clean
> environment, logs being captured, locking against multiple
> startups and so on. Right now, without any additional code.
****************************************
*EXCEPT THAT NOT EVERYONE USES SYSTEMD!*
****************************************

Why do you fail to get that simple fact?

I'm not changing my init system just to add functionality that should 
already exist in btrfs-progs.  The fact that the balance ioctl is 
synchronous was a poor design choice, and we need to provide the option 
to work around that independent of what our users are running.  There's 
been enough interest from other people that it should be obvious that 
people want this _in_ btrfs-progs.  The point is to not _need_ anything 
else to do this.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Duncan]

Austin S. Hemmelgarn posted on Tue, 12 Jul 2016 08:25:24 -0400 as
excerpted:

> As far as daemonization, I have no man-page called daemon in section
> seven, yet I have an up-to-date upstream copy of the Linux man pages. My
> guess is that this is a systemd man page, which in turn means it does a
> bunch of stuff that's only needed for system services on systemd based
> systems.

Your guess would appear to be correct, as my systemd package includes a 
daemon manpage, here (gentoo).

As for the general debate, while I'm a systemd user, I definitely support 
it remaining optional, which means things need to work without it, too.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Austin S. Hemmelgarn]

On 2016-07-12 11:22, Duncan wrote:
> Austin S. Hemmelgarn posted on Tue, 12 Jul 2016 08:25:24 -0400 as
> excerpted:
>
>> As far as daemonization, I have no man-page called daemon in section
>> seven, yet I have an up-to-date upstream copy of the Linux man pages. My
>> guess is that this is a systemd man page, which in turn means it does a
>> bunch of stuff that's only needed for system services on systemd based
>> systems.
>
> Your guess would appear to be correct, as my systemd package includes a
> daemon manpage, here (gentoo).
OK, in that case, what that page will list is the requirements for a 
daemon which runs under systemd.  That's _way_ more than what's needed 
for a regular daemon, and IIRC, some of the things systemd requires will 
cause issues on non-systemd systems (for those who use Gentoo or build 
their own software, this is why so many things have systemd support 
optional, many things haven't been written to choose at runtime).

For reference, the de-fact standard method of daemonization on UNIX is 
generally:
1. Perform any config parsing and similar and exit if it fails.
2. fork()
3. setsid() in the child process
4. fork() in the child process
5. chdir("/") in the new child process (this shouldn't need to have it's 
return code checked, if it fails, you almost certainly have bigger 
issues to deal with).
6. Close stdin, stdout, and stderr in the new child process
7. Open stdin, stdout, and stderr to whatever you want (usually 
/dev/null unless logging is being done).
8. Optionally provide verification from the first child that the second 
child is still running, then exit the first child process.
9. Optionally provide verification from the parent process that the 
first child exited with out indicating errors, then exit.

Item 1 is to allow for verbose error info to be returned if something 
that can be done in the foreground fails.  Items 2 and 3 set up a 
session leader process which would get re-parented to init if it's 
parent exited.  Item 4 makes sure the daemon isn't a session leader, 
which means it will never have a controlling terminal again (this is the 
bit that the glibc implementation of daemon(3) doesn't do).  Item 5 
frees up the previous working directory so that it isn't referenced by 
the daemon directly, and thus can be unmounted or removed (this isn't 
strictly necessary, but it's considered good practice).  Item 6 ensures 
that the daemon has no controlling terminal (so it doesn't get hit by 
SIGHUP or similar things accidentally). Item 7 is not 100% necessary, 
but is also considered good practice (especially if the code is only 
optionally run as a daemon).  Items 8 and 9 are generally considered 
good practice, but are less commonly used in simple daemons than other 
options.

As stated in a previous e-mail, there are two rather obvious options 
which I chose not to use here:
1. libdaemon
2. daemon(3)
Additionally, libbsd also has a daemon() function.  I chose to not use 
daemon(3) because it has issues as implemented by glibc (it skips step 4 
above, which has potentially problematic implications).  I chose not to 
use libdaemon or libbsd because those would add another dependency, 
which is not a good thing for a low-level maintenance tool.

As far as this particular implementation I did, I've got items 2-7. 
Item 1 is equivalent to command-line parsing, which is mostly done at 
this point.  I plan to do items 8 and 9, but don't really have the time 
to code them right now.
>
> As for the general debate, while I'm a systemd user, I definitely support
> it remaining optional, which means things need to work without it, too.
I'm glad to hear some one else feels this way, although I'm not 
surprised it's another Gentoo user :).

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Andrei Borzenkov]

12.07.2016 15:25, Austin S. Hemmelgarn пишет:
> 
> I'm not changing my init system just to add functionality that should
> already exist in btrfs-progs.  The fact that the balance ioctl is
> synchronous was a poor design choice, and we need to provide the option
> to work around that independent of what our users are running.  There's
> been enough interest from other people that it should be obvious that
> people want this _in_ btrfs-progs.  The point is to not _need_ anything
> else to do this.

May be I miss something obvious, but what is wrong with

nohup btrfs balance start &

nohup is definitely available on every system and so far I have been
using it in exactly such cases when I needed to leave something running
for a long time.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Austin S. Hemmelgarn]

On 2016-07-13 00:39, Andrei Borzenkov wrote:
> 12.07.2016 15:25, Austin S. Hemmelgarn пишет:
>>
>> I'm not changing my init system just to add functionality that should
>> already exist in btrfs-progs.  The fact that the balance ioctl is
>> synchronous was a poor design choice, and we need to provide the option
>> to work around that independent of what our users are running.  There's
>> been enough interest from other people that it should be obvious that
>> people want this _in_ btrfs-progs.  The point is to not _need_ anything
>> else to do this.
>
> May be I miss something obvious, but what is wrong with
>
> nohup btrfs balance start &
>
> nohup is definitely available on every system and so far I have been
> using it in exactly such cases when I needed to leave something running
> for a long time.
>
That does work, but it still falls into the category of needing external 
software to do it.  I'm trying to get something that works without any 
external software, and doesn't even depend on having a shell with job 
control.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[David Sterba]

On Tue, Jun 21, 2016 at 11:16:59AM -0400, Austin S. Hemmelgarn wrote:
> Currently, balance operations are run synchronously in the foreground.
> This is nice for interactive management, but is kind of crappy when you
> start looking at automation and similar things.

Yeah, people have been complaining about the lack of backgrounding
support over the time.

> This patch adds an option to `btrfs balance start` to tell it to
> daemonize prior to running the balance operation, thus allowing us to
> preform balances asynchronously.  The two biggest use cases I have for
> this are starting a balance on a remote server without establishing a
> full shell session, and being able to background the balance in a
> recovery shell (which usually has no job control) so I can still get
> progress information.
> 
> Because it simply daemonizes prior to calling the balance ioctl, this
> doesn't actually need any kernel support.

We could also add the kernel support, but this would need to extend the
ioctl flags. Unfortunatelly older kernels would ignore it and always let
balance run in foreground (due to lack of checks for the known flags).
So at the moment forking a process seems to be an option.

> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> ---
> This works as is, but there are two specific things I would love to
> eventually fix but don't have the time to fix right now:
> * There is no way to get any feedback from the balance operation.

Does this mean that 'btrfs balance status' is not sufficient? Or I don't
understand what you mean.

> * Because of how everything works, trying to start a new balance with
>   --background while one iw already running won't return an error but
>   won't queue or start a new balance either.
> 
> The first one is more a utility item than anything else, and probably
> would not be hard to add.  Ideally, it should be output to a user
> specified file, and this should work even for a normal foreground balance.
> 
> The second is very much a UX issue, but can't be easily sovled without
> doing some creative process monitoring from the parrent processes.

Currently, starting a second balance will return immediatelly with "in
progress" message, this is returned by the balance iotctl itself. In
this case it would be the child process and communicating it back would
need a pipe or somesuch. But, the parent can check the balance status by
itself, before calling fork, right? Unless I'm missing something, this
should address your concerns.

Re: [PATCH] btrfs-progs: add option to run balance as daemon

[Austin S. Hemmelgarn]

On 2016-07-13 12:38, David Sterba wrote:
> On Tue, Jun 21, 2016 at 11:16:59AM -0400, Austin S. Hemmelgarn wrote:
>> Currently, balance operations are run synchronously in the foreground.
>> This is nice for interactive management, but is kind of crappy when you
>> start looking at automation and similar things.
>
> Yeah, people have been complaining about the lack of backgrounding
> support over the time.
TBH, I actually understand why it was implemented the way it was in the 
first place, it's a lot easier to work with something running in the 
foreground, and having the ability to run in the background adds 
complexity, so blocking like it does currently makes sense for an 
initial implementation.  Beyond the initial implementation, I don't 
think it's bothered anyone enough that they wanted to fix it themselves.
>
>> This patch adds an option to `btrfs balance start` to tell it to
>> daemonize prior to running the balance operation, thus allowing us to
>> preform balances asynchronously.  The two biggest use cases I have for
>> this are starting a balance on a remote server without establishing a
>> full shell session, and being able to background the balance in a
>> recovery shell (which usually has no job control) so I can still get
>> progress information.
>>
>> Because it simply daemonizes prior to calling the balance ioctl, this
>> doesn't actually need any kernel support.
>
> We could also add the kernel support, but this would need to extend the
> ioctl flags. Unfortunatelly older kernels would ignore it and always let
> balance run in foreground (due to lack of checks for the known flags).
> So at the moment forking a process seems to be an option.
Ideally, I'd like to see kernel support for this too so that tools that 
want to manage it don't need to spawn a thread or fork a new process 
just to run it in the background.  That said, it's a lot quicker to 
implement in userspace, and we'd need such an implementation anyway to 
support old kernels.
>
>> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>> ---
>> This works as is, but there are two specific things I would love to
>> eventually fix but don't have the time to fix right now:
>> * There is no way to get any feedback from the balance operation.
>
> Does this mean that 'btrfs balance status' is not sufficient? Or I don't
> understand what you mean.
`btrfs balance status` calls still work.  What I was referring to is the 
lack of the usual 'Done, had to relocate X out of Y chunks.' output on 
completion.  I probably should have worded this differently in retrospect.

As mentioned below though, in an ideal situation, this would have the 
ability to log the results, but doing that the right way is more code 
than I'm willing to deal with for a first pass at this.
>
>> * Because of how everything works, trying to start a new balance with
>>   --background while one iw already running won't return an error but
>>   won't queue or start a new balance either.
>>
>> The first one is more a utility item than anything else, and probably
>> would not be hard to add.  Ideally, it should be output to a user
>> specified file, and this should work even for a normal foreground balance.
>>
>> The second is very much a UX issue, but can't be easily sovled without
>> doing some creative process monitoring from the parrent processes.
>
> Currently, starting a second balance will return immediatelly with "in
> progress" message, this is returned by the balance iotctl itself. In
> this case it would be the child process and communicating it back would
> need a pipe or somesuch. But, the parent can check the balance status by
> itself, before calling fork, right? Unless I'm missing something, this
> should address your concerns.
What I had been thinking was just having the parent wait and see that 
the child is still running after a short time (100ms maybe), or has 
exited with a 0 exit status.  I'm hesitant to add an extra ioctl just to 
check if a balance is running because of TOCTOU races, I'd rather have 
it deterministically start the balance or not based solely on the 
balance ioctl, especially since fork() is expensive enough to make such 
a race rather easy to hit).  Hopefully, I'll have some time in the near 
future to do an updated version that properly handles this.