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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 16EA0713F3 for ; Thu, 10 Jun 2021 14:30:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F37A22F220 for ; Thu, 10 Jun 2021 14:30:14 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id C30102F213 for ; Thu, 10 Jun 2021 14:30:12 +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 97AC646748 for ; Thu, 10 Jun 2021 14:30:12 +0200 (CEST) Date: Thu, 10 Jun 2021 14:30:05 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20210609131852.167416-1-w.bumiller@proxmox.com> <20210609131852.167416-5-w.bumiller@proxmox.com> In-Reply-To: <20210609131852.167416-5-w.bumiller@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1623327878.g3rwwrxepu.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.896 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 KAM_SHORT 0.001 Use of a URL Shortener for very short URL SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, storage.pm, gnu.org, pvesm.pm] Subject: Re: [pve-devel] [PATCH storage 3/4] update import/export storage API 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: Thu, 10 Jun 2021 12:30:45 -0000 On June 9, 2021 3:18 pm, Wolfgang Bumiller wrote: > This bumps APIVER, but also APIAGE, see below. >=20 > The import methods (volume_import, volume_import_formats): > This additionally gets the '$snapshot' parameter which is > already present on the export side as an informational > piece to know which of the snapshots is the *current* one. > The current "disk" state will be set to this snapshot. > This, too, is required for our btrfs implementation. > `volume_import_formats` can obviously not make much > *use* of this parameter, but it'll still be useful to know > that the information is actually available in the import > call, so its presence will be checked in the btrfs > implementation. >=20 > Currently this is intended to be used for btrfs send/recv > support, which in theory could also get additional metadata > similar to how we do the "tar+size" format, however, we > currently only really use this within this repository in > storage_migrate() which has this information readily > available anyway. >=20 > On the export side (volume_export, volume_export_formats): > The `$with_snapshots` option can now also be an ordered > array of snapshots to include, as a hint for storages > which need this. (As of the next commit this is only > btrfs, and only when also specifying a base snapshot, > which is a case we can currently not run into except on > the command line interface.) > The current providers of the `with_snapshot` option will > still treat it as a boolean (since eg. for ZFS you cannot > really "skip" snapshots AFAIK). > This is mainly intended for storages which do not have a > strong association between snapshots and the originals, or > an ordering (eg. btrfs and lvm-thin allow creating > arbitrary snapshot trees, and with btrfs you can even > create a "circular" connection between subvolumes, also we > could consider reflink based copies snapshots on xfs in > the future maybe?) >=20 > While `import_formats` and `export_formats` now take the same > parameters, I appended the `$snapshot` parameter to the end of > `import_formats` to the end for compatibility. since we are currently in a window where we can do breaking changes, we=20 could think about breaking for the sake of consistency. do we have any=20 other cleanups planned that would cause us to reset APIAGE? > Therefore we also bump APIAGE, as apart from the btrfs > storage, none of our other storages need the new parameter. >=20 > Signed-off-by: Wolfgang Bumiller > --- > PVE/CLI/pvesm.pm | 23 +++++++++++++++++++++-- > PVE/Storage.pm | 48 ++++++++++++++++++++++++++++++++---------------- > 2 files changed, 53 insertions(+), 18 deletions(-) >=20 > diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm > index d28f1ba..b22f759 100755 > --- a/PVE/CLI/pvesm.pm > +++ b/PVE/CLI/pvesm.pm > @@ -276,12 +276,23 @@ __PACKAGE__->register_method ({ > optional =3D> 1, > default =3D> 0, > }, > + 'snapshot-list' =3D> { > + description =3D> "Ordered list of snapshots to transfer", > + type =3D> 'string', > + format =3D> 'string-list', > + optional =3D> 1, > + }, > }, > }, > returns =3D> { type =3D> 'null' }, > code =3D> sub { > my ($param) =3D @_; > =20 > + my $with_snapshots =3D $param->{'with-snapshots'}; > + if (defined(my $list =3D $param->{'snapshot-list'})) { > + $with_snapshots =3D PVE::Tools::split_list($list); > + } > + > my $filename =3D $param->{filename}; > =20 > my $outfh; > @@ -295,7 +306,7 @@ __PACKAGE__->register_method ({ > eval { > my $cfg =3D PVE::Storage::config(); > PVE::Storage::volume_export($cfg, $outfh, $param->{volume}, $param-= >{format}, > - $param->{snapshot}, $param->{base}, $param->{'with-snapshots'}); > + $param->{snapshot}, $param->{base}, $with_snapshots); > }; > my $err =3D $@; > if ($filename ne '-') { > @@ -361,6 +372,13 @@ __PACKAGE__->register_method ({ > optional =3D> 1, > default =3D> 0, > }, > + snapshot =3D> { > + description =3D> "The current-state snapshot if the stream contains sn= apshots", > + type =3D> 'string', > + pattern =3D> qr/[a-z0-9_\-]{1,40}/i, > + maxLength =3D> 40, > + optional =3D> 1, > + }, > }, > }, > returns =3D> { type =3D> 'string' }, > @@ -436,7 +454,8 @@ __PACKAGE__->register_method ({ > my $volume =3D $param->{volume}; > my $delete =3D $param->{'delete-snapshot'}; > my $imported_volid =3D PVE::Storage::volume_import($cfg, $infh, $volume= , $param->{format}, > - $param->{base}, $param->{'with-snapshots'}, $param->{'allow-rename'= }); > + $param->{base}, $param->{'with-snapshots'}, $param->{'allow-rename'= }, > + $param->{snapshot}); > PVE::Storage::volume_snapshot_delete($cfg, $imported_volid, $delete) > if defined($delete); > return $imported_volid; > diff --git a/PVE/Storage.pm b/PVE/Storage.pm > index f9f8d16..15fcedf 100755 > --- a/PVE/Storage.pm > +++ b/PVE/Storage.pm > @@ -41,11 +41,11 @@ use PVE::Storage::PBSPlugin; > use PVE::Storage::BTRFSPlugin; > =20 > # Storage API version. Increment it on changes in storage API interface. > -use constant APIVER =3D> 8; > +use constant APIVER =3D> 9; > # Age is the number of versions we're backward compatible with. > # This is like having 'current=3DAPIVER' and age=3D'APIAGE' in libtool, > # see https://www.gnu.org/software/libtool/manual/html_node/Libtool-vers= ioning.html > -use constant APIAGE =3D> 7; > +use constant APIAGE =3D> 8; > =20 > # load standard plugins > PVE::Storage::DirPlugin->register(); > @@ -716,7 +716,8 @@ sub storage_migrate { > my $send =3D ['pvesm', 'export', $volid, $format, '-', '-with-snapsh= ots', $with_snapshots]; > my $recv =3D [@$ssh, '--', 'pvesm', 'import', $target_volid, $format= , $import_fn, '-with-snapshots', $with_snapshots]; > if (defined($snapshot)) { > - push @$send, '-snapshot', $snapshot > + push @$send, '-snapshot', $snapshot; > + push @$recv, '-snapshot', $snapshot; this one breaks migration using storage_migrate and replication from=20 updated to non-updated nodes.. we already introduced a mechanism to=20 handle this (`pvesm apiinfo`) the last time we introduced a new=20 parameter/breaking change on the receiving side (`--allow-rename`),=20 which should likely be used here to stay compatible in both directions. > } > if ($migration_snapshot) { > push @$recv, '-delete-snapshot', $snapshot; > @@ -1716,7 +1717,7 @@ sub prune_mark_backup_group { > } > } > =20 > -sub volume_export { > +sub volume_export : prototype($$$$$$$) { > my ($cfg, $fh, $volid, $format, $snapshot, $base_snapshot, $with_sna= pshots) =3D @_; > =20 > my ($storeid, $volname) =3D parse_volume_id($volid, 1); > @@ -1727,18 +1728,27 @@ sub volume_export { > $snapshot, $base_snapshot, $with_snaps= hots); > } > =20 > -sub volume_import { > - my ($cfg, $fh, $volid, $format, $base_snapshot, $with_snapshots, $al= low_rename) =3D @_; > +sub volume_import : prototype($$$$$$$$) { > + my ($cfg, $fh, $volid, $format, $base_snapshot, $with_snapshots, $al= low_rename, $snapshot) =3D @_; > =20 > my ($storeid, $volname) =3D parse_volume_id($volid, 1); > die "cannot import into volume '$volid'\n" if !$storeid; > my $scfg =3D storage_config($cfg, $storeid); > my $plugin =3D PVE::Storage::Plugin->lookup($scfg->{type}); > - return $plugin->volume_import($scfg, $storeid, $fh, $volname, $forma= t, > - $base_snapshot, $with_snapshots, $allo= w_rename) // $volid; > -} > - > -sub volume_export_formats { > + return $plugin->volume_import( > + $scfg, > + $storeid, > + $fh, > + $volname, > + $format, > + $base_snapshot, > + $with_snapshots, > + $allow_rename, > + $snapshot, > + ) // $volid; > +} > + > +sub volume_export_formats : prototype($$$$$) { > my ($cfg, $volid, $snapshot, $base_snapshot, $with_snapshots) =3D @_= ; > =20 > my ($storeid, $volname) =3D parse_volume_id($volid, 1); > @@ -1750,21 +1760,27 @@ sub volume_export_formats { > $with_snapshots); > } > =20 > -sub volume_import_formats { > - my ($cfg, $volid, $base_snapshot, $with_snapshots) =3D @_; > +sub volume_import_formats : prototype($$$$$) { > + my ($cfg, $volid, $base_snapshot, $with_snapshots, $snapshot) =3D @_= ; > =20 > my ($storeid, $volname) =3D parse_volume_id($volid, 1); > return if !$storeid; > my $scfg =3D storage_config($cfg, $storeid); > my $plugin =3D PVE::Storage::Plugin->lookup($scfg->{type}); > - return $plugin->volume_import_formats($scfg, $storeid, $volname, > - $base_snapshot, $with_snapshot= s); > + return $plugin->volume_import_formats( > + $scfg, > + $storeid, > + $volname, > + $base_snapshot, > + $with_snapshots, > + $snapshot, > + ); > } > =20 > sub volume_transfer_formats { > my ($cfg, $src_volid, $dst_volid, $snapshot, $base_snapshot, $with_s= napshots) =3D @_; > my @export_formats =3D volume_export_formats($cfg, $src_volid, $snap= shot, $base_snapshot, $with_snapshots); > - my @import_formats =3D volume_import_formats($cfg, $dst_volid, $base= _snapshot, $with_snapshots); > + my @import_formats =3D volume_import_formats($cfg, $dst_volid, $base= _snapshot, $with_snapshots, $snapshot); > my %import_hash =3D map { $_ =3D> 1 } @import_formats; > my @common =3D grep { $import_hash{$_} } @export_formats; > return @common; > --=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