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

[Dietmar Maurer]

applied

___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

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

[Fabian Grünbichler]

> Wolfgang Bumiller  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?

___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel