<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<blockquote>
<blockquote type="cite">+ } else {
<br>
+ die "can't snapshot";
<br>
</blockquote>
why 'can't snapshot'?! <br>
</blockquote>
<br>
If the storeid can not be parsed (undef)!<br>
<br>
<blockquote>
<blockquote type="cite">- } elsif ($volid =~ m|^(/.+)$|
&& -e $volid) {
<br>
- die "snapshot rollback device is not possible";
<br>
- } else {
<br>
- die "can't snapshot";
<br>
</blockquote>
<br>
Why do you remove those checks? <br>
</blockquote>
Because I checked them before in QemuServer.pm!<br>
should I double check it?
<blockquote>
<blockquote type="cite"> - # abort rollback if snapshot is not
the latest
<br>
- my $recentsnap = $class->zfs_get_latest_snapshot($scfg,
$volname);
<br>
- if ($snap ne $recentsnap) {
<br>
- die "cannot rollback, more recent snapshots exist\n";
<br>
- }
<br>
-
<br>
</blockquote>
<br>
You simple remove that test (but do not call
volume_rollback_is_possibe)? <br>
</blockquote>
I call it in QemuServer.pm.<br>
should I double check it?<br>
<br>
<div class="moz-cite-prefix">On 02/12/2015 06:57 AM, Dietmar Maurer
wrote:<br>
</div>
<blockquote cite="mid:54DC40BF.6000401@proxmox.com" type="cite">On
02/11/2015 01:33 PM, Wolfgang Link wrote:
<br>
<blockquote type="cite">add methode volume_rollback_is_possibel
and refactor
<br>
Improve error handling
<br>
If snapshot is not revetable catch it before vm will lock and
shutdown.
<br>
This is the case if zfs has an younger snapshot.
<br>
</blockquote>
Please use a spell checker to fix spelling errors.
<br>
<br>
possibel => possible
<br>
revetable => ?
<br>
<br>
<blockquote type="cite">
<br>
Signed-off-by: Wolfgang Link <a class="moz-txt-link-rfc2396E" href="mailto:w.link@proxmox.com"><w.link@proxmox.com></a>
<br>
---
<br>
PVE/Storage.pm | 19 +++++++++++++++----
<br>
PVE/Storage/Plugin.pm | 6 ++++++
<br>
PVE/Storage/ZFSPlugin.pm | 6 ------
<br>
PVE/Storage/ZFSPoolPlugin.pm | 11 ++++++++---
<br>
4 files changed, 29 insertions(+), 13 deletions(-)
<br>
<br>
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
<br>
index 8711710..c4b9a68 100755
<br>
--- a/PVE/Storage.pm
<br>
+++ b/PVE/Storage.pm
<br>
@@ -144,6 +144,21 @@ sub volume_resize {
<br>
}
<br>
}
<br>
+sub volume_rollback_is_possibel {
<br>
+ my ($cfg, $volid, $snap) = @_;
<br>
+
<br>
+ my ($storeid, $volname) = parse_volume_id($volid, 1);
<br>
+ if ($storeid) {
<br>
+ my $scfg = storage_config($cfg, $storeid);
<br>
+ my $plugin =
PVE::Storage::Plugin->lookup($scfg->{type});
<br>
+ return $plugin->volume_rollback_is_possibe($scfg,
$storeid, $volname, $snap);
<br>
</blockquote>
<br>
volume_rollback_is_possibe => volume_rollback_is_possible
<br>
<br>
<br>
<blockquote type="cite">+ } elsif ($volid =~ m|^(/.+)$|
&& -e $volid) {
<br>
+ die "snapshot rollback device is not possible";
<br>
</blockquote>
<br>
We can improve those error messages, for example:
<br>
<br>
die "rollback device '$volid' is not possible\n";
<br>
<br>
<br>
<blockquote type="cite">+ } else {
<br>
+ die "can't snapshot";
<br>
</blockquote>
why 'can't snapshot'?!
<br>
<br>
<blockquote type="cite">+ }
<br>
+}
<br>
+
<br>
sub volume_snapshot {
<br>
my ($cfg, $volid, $snap, $running) = @_;
<br>
@@ -167,10 +182,6 @@ sub volume_snapshot_rollback {
<br>
my $scfg = storage_config($cfg, $storeid);
<br>
my $plugin =
PVE::Storage::Plugin->lookup($scfg->{type});
<br>
return $plugin->volume_snapshot_rollback($scfg,
$storeid, $volname, $snap);
<br>
- } elsif ($volid =~ m|^(/.+)$| && -e $volid) {
<br>
- die "snapshot rollback device is not possible";
<br>
- } else {
<br>
- die "can't snapshot";
<br>
</blockquote>
<br>
Why do you remove those checks?
<br>
<blockquote type="cite"> }
<br>
}
<br>
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
<br>
index 15c23d4..4549f9f 100644
<br>
--- a/PVE/Storage/Plugin.pm
<br>
+++ b/PVE/Storage/Plugin.pm
<br>
@@ -656,6 +656,12 @@ sub volume_snapshot {
<br>
return undef;
<br>
}
<br>
+sub volume_rollback_is_possibe {
<br>
+ my ($class, $scfg, $storeid, $volname, $snap) = @_;
<br>
+
<br>
+ return 1;
<br>
+}
<br>
+
<br>
sub volume_snapshot_rollback {
<br>
my ($class, $scfg, $storeid, $volname, $snap) = @_;
<br>
diff --git a/PVE/Storage/ZFSPlugin.pm
b/PVE/Storage/ZFSPlugin.pm
<br>
index 77394b9..f020985 100644
<br>
--- a/PVE/Storage/ZFSPlugin.pm
<br>
+++ b/PVE/Storage/ZFSPlugin.pm
<br>
@@ -310,12 +310,6 @@ sub volume_resize {
<br>
sub volume_snapshot_rollback {
<br>
my ($class, $scfg, $storeid, $volname, $snap) = @_;
<br>
- # abort rollback if snapshot is not the latest
<br>
- my $recentsnap = $class->zfs_get_latest_snapshot($scfg,
$volname);
<br>
- if ($snap ne $recentsnap) {
<br>
- die "cannot rollback, more recent snapshots exist\n";
<br>
- }
<br>
-
<br>
</blockquote>
<br>
You simple remove that test (but do not call
volume_rollback_is_possibe)?
<br>
<blockquote type="cite"> $class->zfs_delete_lu($scfg,
$volname);
<br>
$class->zfs_request($class, $scfg, undef, 'rollback',
"$scfg->{pool}/$volname\@$snap");
<br>
diff --git a/PVE/Storage/ZFSPoolPlugin.pm
b/PVE/Storage/ZFSPoolPlugin.pm
<br>
index 754f29f..0d38012 100644
<br>
--- a/PVE/Storage/ZFSPoolPlugin.pm
<br>
+++ b/PVE/Storage/ZFSPoolPlugin.pm
<br>
@@ -411,13 +411,18 @@ sub volume_snapshot_delete {
<br>
sub volume_snapshot_rollback {
<br>
my ($class, $scfg, $storeid, $volname, $snap) = @_;
<br>
- # abort rollback if snapshot is not the latest
<br>
+ zfs_request($class, $scfg, undef, 'rollback',
"$scfg->{pool}/$volname\@$snap");
<br>
+}
<br>
+
<br>
+sub volume_rollback_is_possibe {
<br>
+ my ($class, $scfg, $storeid, $volname, $snap) = @_;
<br>
+
<br>
my $recentsnap = $class->zfs_get_latest_snapshot($scfg,
$volname);
<br>
if ($snap ne $recentsnap) {
<br>
- die "cannot rollback, more recent snapshots exist\n";
<br>
+ die "cannot rollback, more recent snapshots exist\n";
<br>
}
<br>
- zfs_request($class, $scfg, undef, 'rollback',
"$scfg->{pool}/$volname\@$snap");
<br>
+ return 1;
<br>
}
<br>
sub activate_storage {
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>