[pve-devel] [PATCH container] Only add actual volumes to volid_list

[Fabian Grünbichler]

> Wolfgang Bumiller <w.bumiller at proxmox.com> hat am 4. Februar 2016 um 11:21
> geschrieben:
> 
> 
> On Thu, Feb 04, 2016 at 11:08:05AM +0100, Fabian Grünbichler wrote:
> > skip /dev and bind mounts, otherwise stop backups will
> > fail in parse_volume_id.
> > ---
> >  src/PVE/VZDump/LXC.pm | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
> > index dddf17e..fda37c9 100644
> > --- a/src/PVE/VZDump/LXC.pm
> > +++ b/src/PVE/VZDump/LXC.pm
> > @@ -132,6 +132,7 @@ sub prepare {
> >      my ($id_map, $rootuid, $rootgid) = PVE::LXC::parse_id_maps($conf);
> >      $task->{userns_cmd} = PVE::LXC::userns_command($id_map);
> >  
> > +    my $volid_list = [];
> >      PVE::LXC::foreach_mountpoint($conf, sub {
> >  	my ($name, $data) = @_;
> >  	my $volid = $data->{volume};
> > @@ -146,8 +147,9 @@ sub prepare {
> >  	}
> >  
> >  	push @$disks, $data;
> > +	push @$volid_list, $volid
> > +	    if $type eq 'volume';
> >      });
> > -    my $volid_list = [map { $_->{volume} } @$disks];
> >  
> >      if ($mode eq 'snapshot') {
> >  	if (!PVE::LXC::has_feature('vzdump', $conf, $storage_cfg)) {
> > @@ -219,6 +221,7 @@ sub snapshot {
> >  	if !($conf->{snapshots} && $conf->{snapshots}->{vzdump});
> >  
> >      my $disks = $task->{disks};
> > +    #todo: reevaluate bind/dev mount handling when implementing snapshots
> > for mps
> >      my $volid_list = [map { $_->{volume} } @$disks];
> 
> We could store the volid list from prepare() in $task->{volids} and
> reuse it here.
> (Doesn't this otherwise currently try to activate_volume() on /dev and
> bindmounts?)
> 

Yes we could, but  this code will only be called if disks has only one element,
which has passed the LXC::has_feature() check:
- only rootfs and mps with backup == 1 are included in disks
- if we try to snapshot with any mps with backup == 1 LXC::has_feature() in
prepare() errors out before snapshot() is called
Therefore, the only way to reach snapshot() is with $disks containing exactly
one entry, the rootfs. 

If rootfs is a bind or /dev mount (if that is even supported by other code
paths?), LXC::has_feature() fails before snapshot() is called.

Since extending snapshot support to multiple volumes/mountpoints will probably
require bigger changes anyway, I figured this should be okay for now?