From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.gruenbichler@proxmox.com>
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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
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 <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=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 <w.bumiller@proxmox.com>
> ---
> 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