From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id B756D750DE for ; Wed, 23 Jun 2021 14:15:00 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AD49FAA86 for ; Wed, 23 Jun 2021 14:15:00 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 62881AA78 for ; Wed, 23 Jun 2021 14:14:58 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 29AD94389E for ; Wed, 23 Jun 2021 14:14:58 +0200 (CEST) Date: Wed, 23 Jun 2021 14:14:48 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20210622121828.84178-1-w.bumiller@proxmox.com> <20210622121828.84178-4-w.bumiller@proxmox.com> In-Reply-To: <20210622121828.84178-4-w.bumiller@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1624448167.ijsofk7p3p.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.401 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URI_NOVOWEL 0.5 URI hostname has long non-vowel sequence Subject: Re: [pve-devel] [PATCH v2 storage 3/5] btrfs: add 'btrfs' import/export format X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jun 2021 12:15:00 -0000 On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote: > Signed-off-by: Wolfgang Bumiller > --- > Changes to v1: > * import api parameters reordered du to diff in patch 2/5 >=20 > PVE/CLI/pvesm.pm | 2 +- > PVE/Storage.pm | 2 +- > PVE/Storage/BTRFSPlugin.pm | 248 +++++++++++++++++++++++++++++++++++-- > 3 files changed, 240 insertions(+), 12 deletions(-) >=20 > diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm > index 4491107..668170a 100755 > --- a/PVE/CLI/pvesm.pm > +++ b/PVE/CLI/pvesm.pm > @@ -30,7 +30,7 @@ use PVE::CLIHandler; > =20 > use base qw(PVE::CLIHandler); > =20 > -my $KNOWN_EXPORT_FORMATS =3D ['raw+size', 'tar+size', 'qcow2+size', 'vmd= k+size', 'zfs']; > +my $KNOWN_EXPORT_FORMATS =3D ['raw+size', 'tar+size', 'qcow2+size', 'vmd= k+size', 'zfs', 'btrfs']; > =20 > my $nodename =3D PVE::INotify::nodename(); > =20 > diff --git a/PVE/Storage.pm b/PVE/Storage.pm > index 5ca052f..c45d339 100755 > --- a/PVE/Storage.pm > +++ b/PVE/Storage.pm > @@ -690,7 +690,7 @@ sub storage_migrate { > =20 > my $migration_snapshot; > if (!defined($snapshot)) { > - if ($scfg->{type} eq 'zfspool') { > + if ($scfg->{type} eq 'zfspool' || $scfg->{type} eq 'btrfs') { > $migration_snapshot =3D 1; > $snapshot =3D '__migration__'; > } > diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm > index 370d848..072dfe0 100644 > --- a/PVE/Storage/BTRFSPlugin.pm > +++ b/PVE/Storage/BTRFSPlugin.pm > @@ -9,8 +9,9 @@ use Fcntl qw(S_ISDIR O_WRONLY O_CREAT O_EXCL); > use File::Basename qw(dirname); > use File::Path qw(mkpath); > use IO::Dir; > +use POSIX qw(EEXIST); > =20 > -use PVE::Tools qw(run_command); > +use PVE::Tools qw(run_command dir_glob_foreach); > =20 > use PVE::Storage::DirPlugin; > =20 > @@ -612,23 +613,250 @@ sub list_images { > return $res; > } > =20 > -# For now we don't implement `btrfs send/recv` as it needs some updates = to our import/export API > -# first! > - > sub volume_export_formats { > - return PVE::Storage::DirPlugin::volume_export_formats(@_); > -} > + my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $w= ith_snapshots) =3D @_; > =20 > -sub volume_export { > - return PVE::Storage::DirPlugin::volume_export(@_); > + # We can do whatever `DirPlugin` can do. > + my @result =3D PVE::Storage::Plugin::volume_export_formats(@_); > + > + # `btrfs send` only works on snapshots: > + return @result if !defined $snapshot; > + > + # Incremental stream with snapshots is only supported if the snapsho= ts are listed (new api): > + return @result if defined($base_snapshot) && $with_snapshots && ref(= $with_snapshots) ne 'ARRAY'; > + > + # Otherwise we do also support `with_snapshots`. > + > + # Finally, `btrfs send` only works on formats where we actually use = btrfs subvolumes: > + my $format =3D ($class->parse_volname($volname))[6]; > + return @result if $format ne 'raw' && $format ne 'subvol'; > + > + return ('btrfs', @result); > } > =20 > sub volume_import_formats { > - return PVE::Storage::DirPlugin::volume_import_formats(@_); > + my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $w= ith_snapshots) =3D @_; > + > + # Same as export-formats, beware the parameter order: > + return volume_export_formats( > + $class, > + $scfg, > + $storeid, > + $volname, > + $snapshot, > + $base_snapshot, > + $with_snapshots, > + ); > +} > + > +sub volume_export { > + my ( > + $class, > + $scfg, > + $storeid, > + $fh, > + $volname, > + $format, > + $snapshot, > + $base_snapshot, > + $with_snapshots, > + ) =3D @_; > + > + if ($format ne 'btrfs') { > + return PVE::Storage::Plugin::volume_export(@_); > + } > + > + die "format 'btrfs' only works on snapshots\n" > + if !defined $snapshot; > + > + die "'btrfs' format in incremental mode requires snapshots to be lis= ted explicitly\n" > + if defined($base_snapshot) && $with_snapshots && ref($with_snapshots) n= e 'ARRAY'; > + > + my $volume_format =3D ($class->parse_volname($volname))[6]; > + > + die "btrfs-sending volumes of type $volume_format ('$volname') is no= t supported\n" > + if $volume_format ne 'raw' && $volume_format ne 'subvol'; the three checks here are the same as in volume_export, maybe they could=20 go into a single export-helper? > + > + my $path =3D $class->path($scfg, $volname, $storeid); > + > + if ($volume_format eq 'raw') { > + $path =3D raw_file_to_subvol($path); > + } > + > + my $cmd =3D ['btrfs', '-q', 'send', '-e']; > + if ($base_snapshot) { > + my $base =3D $class->path($scfg, $volname, $storeid, $base_snapshot); > + if ($volume_format eq 'raw') { > + $base =3D raw_file_to_subvol($base); > + } > + push @$cmd, '-p', $base; > + } > + push @$cmd, '--'; > + if (ref($with_snapshots) eq 'ARRAY') { > + push @$cmd, (map { "$path\@$_" } ($with_snapshots // [])->@*), $path; > + } else { > + dir_glob_foreach(dirname($path), $BTRFS_VOL_REGEX, sub { > + push @$cmd, "$path\@$_[2]" if !(defined($snapshot) && $_[2] eq $sna= pshot); > + }); > + } > + $path .=3D "\@$snapshot" if defined($snapshot); > + push @$cmd, $path; > + > + run_command($cmd, output =3D> '>&'.fileno($fh)); > + return; > } > =20 > sub volume_import { > - return PVE::Storage::DirPlugin::volume_import(@_); > + my ( > + $class, > + $scfg, > + $storeid, > + $fh, > + $volname, > + $format, > + $snapshot, > + $base_snapshot, > + $with_snapshots, > + $allow_rename, > + ) =3D @_; > + > + if ($format ne 'btrfs') { > + return PVE::Storage::Plugin::volume_import(@_); > + } > + > + die "format 'btrfs' only works on snapshots\n" > + if !defined $snapshot; > + > + my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $volume_for= mat) =3D > + $class->parse_volname($volname); > + > + die "btrfs-receiving volumes of type $volume_format ('$volname') is = not supported\n" > + if $volume_format ne 'raw' && $volume_format ne 'subvol'; > + > + if (defined($base_snapshot)) { > + my $path =3D $class->path($scfg, $volname, $storeid, $base_snapshot); > + die "base snapshot '$base_snapshot' not found - no such directory '$pat= h'\n" > + if !path_is_subvolume($path); > + } > + > + my $destination =3D $class->filesystem_path($scfg, $volname); > + if ($volume_format eq 'raw') { > + $destination =3D raw_file_to_subvol($destination); > + } > + > + if (!defined($base_snapshot) && -e $destination) { > + die "volume $volname already exists\n" if !$allow_rename; > + $volname =3D $class->find_free_diskname($storeid, $scfg, $vmid, $volume= _format, 1); > + } > + > + my $imagedir =3D $class->get_subdir($scfg, $vtype); > + $imagedir .=3D "/$vmid" if $vtype eq 'images'; > + > + my $tmppath =3D "$imagedir/recv.$vmid.tmp"; > + mkdir($imagedir); # FIXME: if $scfg->{mkdir}; > + if (!mkdir($tmppath)) { > + die "temp receive directory already exists at '$tmppath', incomplete co= ncurrent import?\n" > + if $! =3D=3D EEXIST; > + die "failed to create temporary receive directory at '$tmppath' - $!\n"= ; > + } > + > + my $dh =3D IO::Dir->new($tmppath) > + or die "failed to open temporary receive directory '$tmppath' - $!\n"; > + eval { > + run_command(['btrfs', '-q', 'receive', '-e', '--', $tmppath], input =3D= > '<&'.fileno($fh)); > + > + # Analyze the received subvolumes; > + my ($diskname, $found_snapshot, @snapshots); > + $dh->rewind; > + while (defined(my $entry =3D $dh->read)) { > + next if $entry eq '.' || $entry eq '..'; > + next if $entry !~ /^$BTRFS_VOL_REGEX$/; > + my ($cur_diskname, $cur_snapshot) =3D ($1, $2); > + > + die "send stream included a non-snapshot subvolume\n" > + if !defined($cur_snapshot); > + > + if (!defined($diskname)) { > + $diskname =3D $cur_diskname; > + } else { > + die "multiple disks contained in stream ('$diskname' vs '$cur_diskname= ')\n" > + if $diskname ne $cur_diskname; > + } > + > + if ($cur_snapshot eq $snapshot) { > + $found_snapshot =3D 1; > + } else { > + push @snapshots, $cur_snapshot; > + } > + } > + > + die "send stream did not contain the expected current snapshot '$snapsh= ot'\n" > + if !$found_snapshot; > + > + # Rotate the disk into place, first the current state: > + # Note that read-only subvolumes cannot be moved into different directo= ries, but for the > + # "current" state we also want a writable copy, so start with that: > + $class->btrfs_cmd(['property', 'set', "$tmppath/$diskname\@$snapshot", = 'ro', 'false']); > + PVE::Tools::renameat2( > + -1, > + "$tmppath/$diskname\@$snapshot", > + -1, > + $destination, > + &PVE::Tools::RENAME_NOREPLACE, > + ) or die "failed to move received snapshot '$tmppath/$diskname\@$snapsh= ot'" > + . " into place at '$destination' - $!\n"; > + > + # Now recreate the actual snapshot: > + $class->btrfs_cmd([ > + 'subvolume', > + 'snapshot', > + '-r', > + '--', > + $destination, > + "$destination\@$snapshot", > + ]); > + > + # Now go through the remaining snapshots (if any) > + foreach my $snap (@snapshots) { > + $class->btrfs_cmd(['property', 'set', "$tmppath/$diskname\@$snap", = 'ro', 'false']); > + PVE::Tools::renameat2( > + -1, > + "$tmppath/$diskname\@$snap", > + -1, > + "$destination\@$snap", > + &PVE::Tools::RENAME_NOREPLACE, > + ) or die "failed to move received snapshot '$tmppath/$diskname\@$sn= ap'" > + . " into place at '$destination\@$snap' - $!\n"; > + eval { $class->btrfs_cmd(['property', 'set', "$destination\@$snap",= 'ro', 'true']) }; > + warn "failed to make $destination\@$snap read-only - $!\n" if $@; > + } > + }; > + my $err =3D $@; > + > + eval { > + # Cleanup all the received snapshots we did not move into place, so we = can remove the temp > + # directory. > + if ($dh) { > + $dh->rewind; > + while (defined(my $entry =3D $dh->read)) { > + next if $entry eq '.' || $entry eq '..'; > + eval { $class->btrfs_cmd(['subvolume', 'delete', '--', "$tmppath/$entr= y"]) }; > + warn $@ if $@; > + } > + $dh->close; undef $dh; > + } > + if (!rmdir($tmppath)) { > + warn "failed to remove temporary directory '$tmppath' - $!\n" > + } > + }; > + warn $@ if $@; > + if ($err) { > + # clean up if the directory ended up being empty after an error > + rmdir($tmppath); > + die $err; > + } I am a bit wary about this (also w.r.t. future btrfs features) - a=20 privileged container can do quite a lot of stuff with btrfs - create snapshots from within container - set default subvolume - set properties - ... (the above is what I found in a few minutes without much BTRFS=20 experience) unprivileged containers can do the following - create new subvols - create new snapshots - set ro property (same caveat of not looking too much at what else is possible, lots of=20 stuff does not work) how do we guarantee that the send stream is not affected in a dangerous=20 fashion by an attacker inside the container? > + > + return "$storeid:$volname"; > } > =20 > 1 > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20