[pve-devel] [PATCH container v2] Fix #576: Cancelling move disk does not leave useless files anymore

[Wolfgang Bumiller]

> On June 12, 2019 at 7:59 AM Thomas Lamprecht <t.lamprecht at proxmox.com> wrote:
> 
> 
> On 6/6/19 11:52 AM, Dominic Jäger wrote:
> > When cancelling the move disk operation for containers the partly
> > finished destination and thus useless files now get removed.
> 
> I really do not understand the commit message, could you please
> describe here how/why that happens now?
> 
> Also, this is probably only OK as it is called through "run_unshared"
> which unshares the mount name-space, so you normally do not fuser'
> someone innocent, or?

In theory someone may have mounted it elsewhere, but that would have to be
done manually as from our code this should be prevented by config-locks AFAIK.

> @Wolfgang, are you OK with this or is there some run_command option
> comming up for reaping rsync's children? (I've some feeling we talked
> about this, but I'm not to sure anymore, sorry)

I'd say for now this is a working fixup. This same code will have to be touched
again with the run_command update anyway as for rsync we need to wait for all
child processes as well as the rsync "main" process doesn't do that, which we don't
want to have as default, so the rsync run_command call would need to pass some
option for it.

As for run_command itself - we need to plug some holes where a `die()`-ing signal
handler can cause us to skip the waitpid(), and then probably add systemd scope
support for rsync's lacking child process handling.
(The first part isn't all too bad, but not sufficient to replace this patch.)

> > 
> > Co-developed-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> > Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
> > ---
> > After a quick offline feedback there is now a v2
> > v2: System call is not a single string anymore 
> > 
> >  src/PVE/LXC.pm | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> > index 62b6b8c..4922fb0 100644
> > --- a/src/PVE/LXC.pm
> > +++ b/src/PVE/LXC.pm
> > @@ -2024,8 +2024,13 @@ my $copy_volume = sub {
> >  				 "--bwlimit=$bwlimit", "$src/", $dest]);
> >      };
> >      my $err = $@;
> > +
> > +    # Wait for rsync's children to release dest so that
> > +    # consequent file operations (umount, remove) are possible
> > +    while ((system {"fuser"} "fuser",  "-s", $dest) == 0) {sleep 1};
> > +
> >      foreach my $mount (reverse @mounted) {
> > -	eval { PVE::Tools::run_command(['/bin/umount', '--lazy', $mount], errfunc => sub{})};
> > +	eval { PVE::Tools::run_command(['/bin/umount', $mount], errfunc => sub{})};
> >  	warn "Can't umount $mount\n" if $@;
> >      }
> >  
> > 
> 
>