9:16 a.m.
All these tests fail with current GIT, but are intended to work
with Eric's snapshot series applied.
The first test in this list, however, does not pass.
Eric's current tree forbids calling 'Destroy' on a transient
guest which has snapshots present. IMHO this is wrong, because
the guest may itself exit at any time, which leaves us in the
same state as is 'Destroy' had been called wrt snapshots.
IMHO, we should be allowed to call 'virDomainDestroy' on a
transient guest with snapshots, and then later 'virDomainCreateXML'
to re-create the guest and still have access to the snapshots
In other words, just because a transient guest is not currently
present, does not mean we should forget about its snapshots as
a management app can re-create it again at any time.
Forbidding virDomainUndefine with snapshots is more reasonable,
but I think its still possible to argue that it should be allowed
and that a later virDomainDefine with the same name+uuid should
resurrect any previous snapshots.
9:16 a.m.
New subject: [libvirt] [PATCH 1/3] Add test for transient domain snapshot management
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The test case validates the core lifecycle operations on
persistent domains with snapshots. A transient domain
should allow snapshots to be created while running. It
should be possible to destory and restart the domain
and still have snapshots present.
---
scripts/domain-snapshot/050-transient-snapshot.t | 97 ++++++++++++++++++++++
1 files changed, 97 insertions(+), 0 deletions(-)
create mode 100644 scripts/domain-snapshot/050-transient-snapshot.t
diff --git a/scripts/domain-snapshot/050-transient-snapshot.t
b/scripts/domain-snapshot/050-transient-snapshot.t
new file mode 100644
index 0000000..a314248
--- /dev/null
+++ b/scripts/domain-snapshot/050-transient-snapshot.t
@@ -0,0 +1,97 @@
+# -*- perl -*-
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#
+# This program is free software; You can redistribute it and/or modify
+# it under the GNU General Public License as published by the Free
+# Software Foundation; either version 2, or (at your option) any
+# later version
+#
+# The file "LICENSE" distributed along with this file provides full
+# details of the terms and conditions
+#
+
+=pod
+
+=head1 NAME
+
+domain/050-transient-snapshot.t - lifecycle of transient domains with snapshots
+
+=head1 DESCRIPTION
+
+The test case validates the core lifecycle operations on
+persistent domains with snapshots. A transient domain
+should allow snapshots to be created while running. It
+should be possible to destory and restart the domain
+and still have snapshots present.
+
+=cut
+
+use strict;
+use warnings;
+
+use Test::More tests => 12;
+use Test::Exception;
+use Sys::Virt::TCK;
+use Sys::Virt::TCK::DomainSnapshotBuilder;
+
+my $tck = Sys::Virt::TCK->new();
+my $conn = eval { $tck->setup(); };
+BAIL_OUT "failed to setup test harness: $@" if $@;
+END { $tck->cleanup if $tck; }
+
+my $poolxml = $tck->generic_pool("dir")->as_xml;
+
+diag "Defining transient storage pool";
+my $pool;
+ok_pool(sub { $pool = $conn->define_storage_pool($poolxml) }, "define transient
storage pool");
+lives_ok(sub { $pool->build(0) }, "built storage pool");
+lives_ok(sub { $pool->create }, "started storage pool");
+
+my $volxml = $tck->generic_volume("tck", "qcow2",
1024*1024*50)->as_xml;
+my $vol;
+ok_volume(sub { $vol = $pool->create_volume($volxml) }, "create qcow2
volume");
+
+my $volpath = xpath($vol, "string(/volume/target/path)");
+diag "Vol path $volpath";
+my $domb = $tck->generic_domain("tck");
+$domb->{disks} = [];
+my $domxml = $domb->disk(type => "file", src => $volpath, dst =>
"hdb",
+ format => {name => "qemu", type =>
"qcow2"})->as_xml;
+
+diag "Creating a new transient domain";
+my $dom;
+ok_domain(sub { $dom = $conn->create_domain($domxml) }, "created transient domain
object");
+
+
+
+my $ssxml = Sys::Virt::TCK::DomainSnapshotBuilder->new(name =>
"ss1")->as_xml;
+
+my $domss;
+diag "Taking a snapshot";
+ok_domain_snapshot(sub { $domss = $dom->create_snapshot($ssxml)}, "created domain
snapshot");
+
+lives_ok(sub { $dom->destroy }, "destroyed domain");
+
+diag "Re-creating the domain";
+ok_domain(sub { $dom = $conn->create_domain($domxml) }, "created transient domain
object");
+
+diag "Looking up snapshot";
+$domss = undef;
+lives_ok(sub { $domss = $dom->get_snapshot_by_name("ss1"); }, "snapshot
still exists");
+
+diag "Deleting snapshot";
+lives_ok(sub { $domss->delete }, "deleted snapshot");
+
+diag "Destroying the transient domain";
+lives_ok(sub { $dom->destroy }, "destroyed domain");
+
+diag "Checking that transient domain has gone away";
+ok_error(sub { $conn->get_domain_by_name("tck") }, "NO_DOMAIN error
raised from missing domain",
+ Sys::Virt::Error::ERR_NO_DOMAIN);
+
+$vol->delete;
+$pool->destroy;
+$pool->delete;
+
+# end
--
1.7.6
10:01 a.m.
New subject: [libvirt] [PATCH 1/3] Add test for transient domain snapshot management
On 08/26/2011 08:16 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange(a)redhat.com>
The test case validates the core lifecycle operations on
persistent domains with snapshots. A transient domain
should allow snapshots to be created while running. It
should be possible to destory and restart the domain
s/destory/destroy
and still have snapshots present.
See my reply to 0/3. I think this test is making an invalid assumption.
If we leave stale snapshot metadata behind, then create a new domain
with different uuid but same name, then we are setting ourselves up for
confusion. We _have_ to clean up the metadata at some point in time,
either when a new domain is created whose name happens to match stale
metadata from a previous domain, or when the last reference to a
transient domain disappears. But if we also make it possible to
reinstate metadata, via VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, then the
app managing a transient domain will be able to reinstate everything
needed to be able to safely revert back to the snapshot storage (when
the prior transient domain shut down, it was only the metadata that was
lost, not the actual storage).
At any rate, thanks for making me think about this, and I guess I'd
better get CREATE_REDEFINE working sooner rather than later.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:16 a.m.
New subject: [libvirt] [PATCH 2/3] Add test for persistent domain snapshot management
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The test case validates the core lifecycle operations on
persistent domains with snapshots. A persistent domain
should allow snapshots to be created & deleted while both
running and shutoff. It should forbid undefine of the
domain when it has snapshots present.
---
scripts/domain-snapshot/080-persistent-snapshot.t | 112 +++++++++++++++++++++
1 files changed, 112 insertions(+), 0 deletions(-)
create mode 100644 scripts/domain-snapshot/080-persistent-snapshot.t
diff --git a/scripts/domain-snapshot/080-persistent-snapshot.t
b/scripts/domain-snapshot/080-persistent-snapshot.t
new file mode 100644
index 0000000..4064003
--- /dev/null
+++ b/scripts/domain-snapshot/080-persistent-snapshot.t
@@ -0,0 +1,112 @@
+# -*- perl -*-
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#
+# This program is free software; You can redistribute it and/or modify
+# it under the GNU General Public License as published by the Free
+# Software Foundation; either version 2, or (at your option) any
+# later version
+#
+# The file "LICENSE" distributed along with this file provides full
+# details of the terms and conditions
+#
+
+=pod
+
+=head1 NAME
+
+domain/080-persistent-snapshot.t - lifecycle of persistent domains with snapshots
+
+=head1 DESCRIPTION
+
+The test case validates the core lifecycle operations on
+persistent domains with snapshots. A persistent domain
+should allow snapshots to be created & deleted while both
+running and shutoff. It should forbid undefine of the
+domain when it has snapshots present.
+
+=cut
+
+use strict;
+use warnings;
+
+use Test::More tests => 18;
+use Test::Exception;
+use Sys::Virt::TCK;
+use Sys::Virt::TCK::DomainSnapshotBuilder;
+
+my $tck = Sys::Virt::TCK->new();
+my $conn = eval { $tck->setup(); };
+BAIL_OUT "failed to setup test harness: $@" if $@;
+END { $tck->cleanup if $tck; }
+
+my $poolxml = $tck->generic_pool("dir")->as_xml;
+
+diag "Defining transient storage pool";
+my $pool;
+ok_pool(sub { $pool = $conn->define_storage_pool($poolxml) }, "define transient
storage pool");
+lives_ok(sub { $pool->build(0) }, "built storage pool");
+lives_ok(sub { $pool->create }, "started storage pool");
+
+my $volxml = $tck->generic_volume("tck", "qcow2",
1024*1024*50)->as_xml;
+my $vol;
+ok_volume(sub { $vol = $pool->create_volume($volxml) }, "create qcow2
volume");
+
+my $volpath = xpath($vol, "string(/volume/target/path)");
+diag "Vol path $volpath";
+my $domb = $tck->generic_domain("tck");
+$domb->{disks} = [];
+my $domxml = $domb->disk(type => "file", src => $volpath, dst =>
"hdb",
+ format => {name => "qemu", type =>
"qcow2"})->as_xml;
+
+diag "Defining a new persistent domain";
+my $dom;
+ok_domain(sub { $dom = $conn->define_domain($domxml) }, "defined persistent
domain object");
+
+my $ss1xml = Sys::Virt::TCK::DomainSnapshotBuilder->new(name =>
"ss1")->as_xml;
+my $ss2xml = Sys::Virt::TCK::DomainSnapshotBuilder->new(name =>
"ss2")->as_xml;
+
+my $domss1;
+my $domss2;
+diag "Taking a snapshot while inactive";
+ok_domain_snapshot(sub { $domss2 = $dom->create_snapshot($ss1xml)}, "created
domain snapshot ss1");
+
+diag "Starting domain";
+lives_ok(sub { $dom->create }, "started domain");
+
+diag "Taking a snapshot while active";
+ok_domain_snapshot(sub { $domss2 = $dom->create_snapshot($ss2xml)}, "created
domain snapshot ss2");
+
+lives_ok(sub { $dom->destroy }, "destroyed domain");
+
+diag "Re-creating the domain";
+lives_ok(sub { $dom->create }, "recreated persistent domain object");
+
+diag "Looking up snapshot";
+$domss1 = undef;
+lives_ok(sub { $domss1 = $dom->get_snapshot_by_name("ss1"); },
"snapshot ss1 still exists");
+lives_ok(sub { $domss2 = $dom->get_snapshot_by_name("ss2"); },
"snapshot ss2 still exists");
+
+diag "Deleting snapshot ss1";
+lives_ok(sub { $domss1->delete }, "deleted snapshot ss1");
+
+diag "Destroying the transient domain";
+lives_ok(sub { $dom->destroy }, "destroyed domain");
+
+diag "Trying to undefine with a snapshot present";
+ok_error(sub { $dom->undefine }, Sys::Virt::Error::ERR_OPERATION_INVALID);
+
+diag "Deleting snapshot ss2";
+lives_ok(sub { $domss2->delete }, "deleted snapshot ss2");
+
+lives_ok(sub { $dom->undefine }, "undefined persistent domain");
+
+diag "Checking that transient domain has gone away";
+ok_error(sub { $conn->get_domain_by_name("tck") }, "NO_DOMAIN error
raised from missing domain",
+ Sys::Virt::Error::ERR_NO_DOMAIN);
+
+$vol->delete;
+$pool->destroy;
+$pool->delete;
+
+# end
--
1.7.6
10:26 a.m.
New subject: [libvirt] [PATCH 2/3] Add test for persistent domain snapshot management
On 08/26/2011 08:16 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange(a)redhat.com>
The test case validates the core lifecycle operations on
persistent domains with snapshots. A persistent domain
should allow snapshots to be created& deleted while both
running and shutoff.
This test is good, but could use more coverage - you need to test the
following 9 transitions, to be sure we covered everything (because my
series proved we had bugs in multiple of these transitions):
current -> snapshot state
------------------
shutoff -> shutoff
shutoff -> paused (0.9.4 buggy - it temporarily ran the vcpus, rather
than restoring to the exact state where the paused snapshot was created)
shutoff -> running
paused -> shutoff (0.9.4 buggy with newer qemu, because it tried to
use qemu -loadvm with no vm state)
paused -> paused
paused -> running (0.9.4 buggy - it left the restored domain paused)
running -> shutoff (0.9.4 buggy with newer qemu, because it tried to
use qemu -loadvm with no vm state)
running -> paused
running -> running
It becomes 12 transitions when you add disk snapshots into the series,
and once I implement VIR_DOMAIN_SNAPSHOT_REVERT_{START,PAUSED,FORCE}, we
have even more combinations.
It should forbid undefine of the
domain when it has snapshots present.
---
scripts/domain-snapshot/080-persistent-snapshot.t | 112 +++++++++++++++++++++
1 files changed, 112 insertions(+), 0 deletions(-)
create mode 100644 scripts/domain-snapshot/080-persistent-snapshot.t
+my $domss1;
+my $domss2;
+diag "Taking a snapshot while inactive";
+ok_domain_snapshot(sub { $domss2 = $dom->create_snapshot($ss1xml)}, "created
domain snapshot ss1");
+
+diag "Starting domain";
+lives_ok(sub { $dom->create }, "started domain");
+
+diag "Taking a snapshot while active";
+ok_domain_snapshot(sub { $domss2 = $dom->create_snapshot($ss2xml)}, "created
domain snapshot ss2");
This only creates 2 of the 3 needed snapshots to fully exercise the
possible 9 transitions.
Also, I don't see any tests of reverting to a snapshot state - you
appear to be testing the management of creation and deletion of
snapshots, but not reversion.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:16 a.m.
New subject: [libvirt] [PATCH 3/3] Add test for recursive deletion of snapshot lists
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The test case validates it recursive deletion of snapshots
cleans up all children and leaves no orphans behind.
---
.../domain-snapshot/100-delete-snapshot-children.t | 101 ++++++++++++++++++++
1 files changed, 101 insertions(+), 0 deletions(-)
create mode 100644 scripts/domain-snapshot/100-delete-snapshot-children.t
diff --git a/scripts/domain-snapshot/100-delete-snapshot-children.t
b/scripts/domain-snapshot/100-delete-snapshot-children.t
new file mode 100644
index 0000000..3ebf2b9
--- /dev/null
+++ b/scripts/domain-snapshot/100-delete-snapshot-children.t
@@ -0,0 +1,101 @@
+# -*- perl -*-
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#
+# This program is free software; You can redistribute it and/or modify
+# it under the GNU General Public License as published by the Free
+# Software Foundation; either version 2, or (at your option) any
+# later version
+#
+# The file "LICENSE" distributed along with this file provides full
+# details of the terms and conditions
+#
+
+=pod
+
+=head1 NAME
+
+domain/100-delete-snapshot-children.t - recursive deletion of snapshot children
+
+=head1 DESCRIPTION
+
+The test case validates it recursive deletion of snapshots
+cleans up all children and leaves no orphans behind.
+
+=cut
+
+use strict;
+use warnings;
+
+use Test::More tests => 29;
+use Test::Exception;
+use Sys::Virt::TCK;
+use Sys::Virt::TCK::DomainSnapshotBuilder;
+
+my $tck = Sys::Virt::TCK->new();
+my $conn = eval { $tck->setup(); };
+BAIL_OUT "failed to setup test harness: $@" if $@;
+END { $tck->cleanup if $tck; }
+
+my $poolxml = $tck->generic_pool("dir")->as_xml;
+
+diag "Defining transient storage pool";
+my $pool;
+ok_pool(sub { $pool = $conn->define_storage_pool($poolxml) }, "define transient
storage pool");
+lives_ok(sub { $pool->build(0) }, "built storage pool");
+lives_ok(sub { $pool->create }, "started storage pool");
+
+my $volxml = $tck->generic_volume("tck", "qcow2",
1024*1024*50)->as_xml;
+my $vol;
+ok_volume(sub { $vol = $pool->create_volume($volxml) }, "create qcow2
volume");
+
+my $volpath = xpath($vol, "string(/volume/target/path)");
+diag "Vol path $volpath";
+my $domb = $tck->generic_domain("tck");
+$domb->{disks} = [];
+my $domxml = $domb->disk(type => "file", src => $volpath, dst =>
"hdb",
+ format => {name => "qemu", type =>
"qcow2"})->as_xml;
+
+diag "Creating a new transient domain";
+my $dom;
+ok_domain(sub { $dom = $conn->create_domain($domxml) }, "created transient domain
object");
+
+
+my $ss;
+for (my $i = 0 ; $i < 8 ; $i++) {
+ my $ssxml = Sys::Virt::TCK::DomainSnapshotBuilder->new(name =>
"ss$i")->as_xml;
+ ok_domain_snapshot(sub { $ss = $dom->create_snapshot($ssxml) }, "created domain
snapshot ss$i");
+}
+
+lives_ok(sub { $ss = $dom->get_snapshot_by_name("ss4") }, "snapshot ss4
exists");
+lives_ok(sub { $ss->delete(Sys::Virt::DomainSnapshot::DELETE_CHILDREN) },
"deleted all snapshots from ss4 onwards");
+
+for (my $i = 0 ; $i < 8 ; $i++) {
+ my $ss;
+ eval { $ss = $dom->get_snapshot_by_name("ss$i") };
+
+ if ($i < 4) {
+ ok_domain_snapshot(sub { $ss }, "snapshot ss$i still exists");
+ } else {
+ ok(!defined $ss, "snapshot ss$i has been deleted");
+ }
+}
+
+for (my $i = 0 ; $i < 4 ; $i++) {
+ my $ss;
+ lives_ok(sub { $ss = $dom->get_snapshot_by_name("ss$i"); $ss->delete },
"deleted snapshot ss$i");
+}
+
+
+diag "Destroying the transient domain";
+lives_ok(sub { $dom->destroy }, "destroyed domain");
+
+diag "Checking that transient domain has gone away";
+ok_error(sub { $conn->get_domain_by_name("tck") }, "NO_DOMAIN error
raised from missing domain",
+ Sys::Virt::Error::ERR_NO_DOMAIN);
+
+$vol->delete;
+$pool->destroy;
+$pool->delete;
+
+# end
--
1.7.6
10:31 a.m.
New subject: [libvirt] [PATCH 3/3] Add test for recursive deletion of snapshot lists
On 08/26/2011 08:16 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange(a)redhat.com>
The test case validates it recursive deletion of snapshots
cleans up all children and leaves no orphans behind.
---
.../domain-snapshot/100-delete-snapshot-children.t | 101 ++++++++++++++++++++
1 files changed, 101 insertions(+), 0 deletions(-)
create mode 100644 scripts/domain-snapshot/100-delete-snapshot-children.t
+
+The test case validates it recursive deletion of snapshots
+cleans up all children and leaves no orphans behind.
Good, but incomplete. You also need to test handling of the current
snapshot in relation to deletion of children. In the notation where *
represents the current snapshot, and using 'parent -> child', if I have:
s1 -> s2 -> s3 -> s4 -> *s5
then delete s2 alone, I should have:
s1 -> s3 -> s4 -> *s5
then if I delete s3 and children, I should have:
*s1
That is, I think your test should also be probing the name of the
current snapshot along each phase of the test, to ensure that the
current snapshot properly migrates up the tree to the parent of the
snapshot subtree that just got deleted, since that was also buggy in 0.9.4.
+diag "Creating a new transient domain";
+my $dom;
+ok_domain(sub { $dom = $conn->create_domain($domxml) }, "created transient
domain object");
system checkpoints of running domains are a lot slower than those of
inactive persistent guests (that is, 'qemu savevm' takes a lot longer
than 'qemu-img snapshot -c'), if that matters at all to your test.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:57 a.m.
On 08/26/2011 08:16 AM, Daniel P. Berrange wrote:
All these tests fail with current GIT, but are intended to work
with Eric's snapshot series applied.
The first test in this list, however, does not pass.
Eric's current tree forbids calling 'Destroy' on a transient
guest which has snapshots present. IMHO this is wrong, because
the guest may itself exit at any time, which leaves us in the
same state as is 'Destroy' had been called wrt snapshots.
Hmm, interesting point.
With managed save, we don't have that problem - the only way to have a
managed save image is with a persistent def, so we can safely forbid
undefine of a domain with managed save state.
IMHO, we should be allowed to call 'virDomainDestroy' on a
transient guest with snapshots, and then later 'virDomainCreateXML'
to re-create the guest and still have access to the snapshots
But leaving the snapshot metadata files behind is problematic. If you
create a new domain with the same name but a different uuid, then those
existing metadata files _will_ cause problems with the domain definition.
In other words, just because a transient guest is not currently
present, does not mean we should forget about its snapshots as
a management app can re-create it again at any time.
How about this compromise?
I already documented in my RFC the desire to add a new flag,
virDomainSnapshotCreateXML(, VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE), which
will allow a management app to recreate a metadata file. I haven't yet
implemented it, but think that it will solve this problem, as follows:
Libvirt should not ever forbid destroy of a domain with snapshots.
However, when a transient guest shuts down, then part of the cleanup
that libvirt does for that guest is to remove all existing snapshot
metadata (the snapshots themselves continue to exist, but the libvirt
hierarchy of creation date, domain xml, and parent relationships is gone).
On destroy, either the domain is persistent (so the snapshots will still
be useful from the inactive guest), or the domain is transient (so the
management app has to be aware of the ramifications). But the
management app _already_ has to track domain xml independently if it
plans on recreating a new transient guest from the same disk images, so
it is not too much of a stretch to assume that the management app can
also track snapshot xml. At which point, if the management app wants to
recreate a new transient app with the same snapshot metadata, then it
simply calls virDomainSnapshotCreateXML(,
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) for each saved snapshot that it
wants to revive in libvirt's tracking.
Thus, snapshot metadata is never left behind to confuse the creation of
a new guest (persistent domains fail to undefine until the snapshots are
cleaned up, and transient domains are cleaned up automatically when the
last reference to the domain disappears), but can easily be restored and
thus tracked externally.
However, this means that your first test would need tweaking - the test
itself would have to create the snapshot, capture the snapshots xml,
destroy the domain, then create a new domain with the same name and
uuid, restore the snapshot, then revert to the snapshot, all to prove
that redefining a transient domain does not lose the actual snapshot
data, just the metadata that tracked the snapshot.
There's also some FIXMEs in libvirt about the notion of extracting the
name of all internal snapshots directly from qcow2 files, and exposing
those to the user with minimal information. qcow2 lacks parent and
domain xml information, so the regenerated snapshot xml would be weak
and reverting to it would require the use of
VIR_DOMAIN_SNAPSHOT_REVERT_FORCE; but making it easier to expose all
known internal snapshot names, even when libvirt does not have the
metadata behind the origin of those internal snapshots, might be useful
via a new flag to virDomainSnapshotListNames.
Forbidding virDomainUndefine with snapshots is more reasonable,
Good, I'll keep it that way, since it is consistent with managed save
images.
but I think its still possible to argue that it should be allowed
and that a later virDomainDefine with the same name+uuid should
resurrect any previous snapshots.
If resurrecting previous snapshots is important, then it should be done
with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:03 a.m.
On Fri, Aug 26, 2011 at 08:57:54AM -0600, Eric Blake wrote:
On 08/26/2011 08:16 AM, Daniel P. Berrange wrote:
>All these tests fail with current GIT, but are intended to work
>with Eric's snapshot series applied.
>
>The first test in this list, however, does not pass.
>
>Eric's current tree forbids calling 'Destroy' on a transient
>guest which has snapshots present. IMHO this is wrong, because
>the guest may itself exit at any time, which leaves us in the
>same state as is 'Destroy' had been called wrt snapshots.
Hmm, interesting point.
With managed save, we don't have that problem - the only way to have
a managed save image is with a persistent def, so we can safely
forbid undefine of a domain with managed save state.
>
>IMHO, we should be allowed to call 'virDomainDestroy' on a
>transient guest with snapshots, and then later 'virDomainCreateXML'
>to re-create the guest and still have access to the snapshots
But leaving the snapshot metadata files behind is problematic. If
you create a new domain with the same name but a different uuid,
then those existing metadata files _will_ cause problems with the
domain definition.
Surely this simply means that the snapshot metadata files shold
be recording the orignal domain UUID, so that we can safely
ignore/discard them if the guest is re-defined with the same
name but new uuid ?
>In other words, just because a transient guest is not currently
>present, does not mean we should forget about its snapshots as
>a management app can re-create it again at any time.
How about this compromise?
I already documented in my RFC the desire to add a new flag,
virDomainSnapshotCreateXML(, VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE),
which will allow a management app to recreate a metadata file. I
haven't yet implemented it, but think that it will solve this
problem, as follows:
Libvirt should not ever forbid destroy of a domain with snapshots.
However, when a transient guest shuts down, then part of the cleanup
that libvirt does for that guest is to remove all existing snapshot
metadata (the snapshots themselves continue to exist, but the
libvirt hierarchy of creation date, domain xml, and parent
relationships is gone).
On destroy, either the domain is persistent (so the snapshots will
still be useful from the inactive guest), or the domain is transient
(so the management app has to be aware of the ramifications). But
the management app _already_ has to track domain xml independently
if it plans on recreating a new transient guest from the same disk
images, so it is not too much of a stretch to assume that the
management app can also track snapshot xml. At which point, if the
management app wants to recreate a new transient app with the same
snapshot metadata, then it simply calls virDomainSnapshotCreateXML(,
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) for each saved snapshot that it
wants to revive in libvirt's tracking.
Thus, snapshot metadata is never left behind to confuse the creation
of a new guest (persistent domains fail to undefine until the
snapshots are cleaned up, and transient domains are cleaned up
automatically when the last reference to the domain disappears), but
can easily be restored and thus tracked externally.
However, this means that your first test would need tweaking - the
test itself would have to create the snapshot, capture the snapshots
xml, destroy the domain, then create a new domain with the same name
and uuid, restore the snapshot, then revert to the snapshot, all to
prove that redefining a transient domain does not lose the actual
snapshot data, just the metadata that tracked the snapshot.
There's also some FIXMEs in libvirt about the notion of extracting
the name of all internal snapshots directly from qcow2 files, and
exposing those to the user with minimal information. qcow2 lacks
parent and domain xml information, so the regenerated snapshot xml
would be weak and reverting to it would require the use of
VIR_DOMAIN_SNAPSHOT_REVERT_FORCE; but making it easier to expose all
known internal snapshot names, even when libvirt does not have the
metadata behind the origin of those internal snapshots, might be
useful via a new flag to virDomainSnapshotListNames.
>
>Forbidding virDomainUndefine with snapshots is more reasonable,
Good, I'll keep it that way, since it is consistent with managed
save images.
>but I think its still possible to argue that it should be allowed
>and that a later virDomainDefine with the same name+uuid should
>resurrect any previous snapshots.
If resurrecting previous snapshots is important, then it should be
done with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE.
This is all workable, but is it overkill compared to just validating
the original VM uuid against the new UUID when loading metdata ?
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:15 a.m.
On 08/26/2011 09:03 AM, Daniel P. Berrange wrote:
>> IMHO, we should be allowed to call 'virDomainDestroy'
on a
>> transient guest with snapshots, and then later 'virDomainCreateXML'
>> to re-create the guest and still have access to the snapshots
>
> But leaving the snapshot metadata files behind is problematic. If
> you create a new domain with the same name but a different uuid,
> then those existing metadata files _will_ cause problems with the
> domain definition.
Surely this simply means that the snapshot metadata files shold
be recording the orignal domain UUID,
They are. Even before my series, the original UUID was the _only_ thing
about the domain that it was storing (my series fixes things to store
the entire <domain> xml, which is needed for proper reversion to a
snapshot, but at least old existing snapshots can already be recognized).
so that we can safely
ignore/discard them if the guest is re-defined with the same
name but new uuid ?
We have two options for when to nuke stale metadata - at the point when
the domain goes down, or at the point when a new domain is created whose
name differs from what was stored in the stale metadata.
I guess you are leaning towards the latter - if a new domain is created
with the right uuid, then the snapshots are already available; but if
created with the same name but a new uuid, then we finally know to clean
up the mess.
But I'm not convinced that it scales well - if you create lots of
transient domains, all with snapshots, but never reuse the same name,
then the metadata directory keeps growing without bounds even though you
never have that many domains at any given point of time. Besides, since
the only way to use libvirt to delete snapshot metadata is via a
virDomainSnapshotPtr object, but the only way to get at a snapshot is
via a virDomainPtr object, I think it is risky to leave stale metadata
in libvirt's directory trees that _cannot_ be removed by any existing
libvirt API.
Cleaning at the time the transient domain goes away is nicer for
scalability. It means that nothing will be stored in
/var/lib/libvirt/qemu/ unless it can also be accessed via a current
virDomainPtr. It is also nicer for migration - remember, in my RFC, I
mentioned how snapshots and migration interact - the only sane way is
for the destination to start from a blank state, then recreate the same
snapshot hierarchy as was present on the source alongside the migration,
then remove the snapshot metadata on the source. If we keep snapshot
metadata around even after a transient domain is no longer running on a
host, then it becomes harder to start from a clean slate when migrating
a new guest by that name onto the host.
> If resurrecting previous snapshots is important, then it should
be
> done with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE.
This is all workable, but is it overkill compared to just validating
the original VM uuid against the new UUID when loading metdata ?
I hope that we can come to agreement on whether or not making the
management app of the transient domain have to track snapshot metadata
counts as overkill or not.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:22 a.m.
On Fri, Aug 26, 2011 at 09:15:43AM -0600, Eric Blake wrote:
On 08/26/2011 09:03 AM, Daniel P. Berrange wrote:
>>>IMHO, we should be allowed to call 'virDomainDestroy' on a
>>>transient guest with snapshots, and then later 'virDomainCreateXML'
>>>to re-create the guest and still have access to the snapshots
>>
>>But leaving the snapshot metadata files behind is problematic. If
>>you create a new domain with the same name but a different uuid,
>>then those existing metadata files _will_ cause problems with the
>>domain definition.
>
>Surely this simply means that the snapshot metadata files shold
>be recording the orignal domain UUID,
They are. Even before my series, the original UUID was the _only_
thing about the domain that it was storing (my series fixes things
to store the entire <domain> xml, which is needed for proper
reversion to a snapshot, but at least old existing snapshots can
already be recognized).
>so that we can safely
>ignore/discard them if the guest is re-defined with the same
>name but new uuid ?
We have two options for when to nuke stale metadata - at the point
when the domain goes down, or at the point when a new domain is
created whose name differs from what was stored in the stale
metadata.
I guess you are leaning towards the latter - if a new domain is
created with the right uuid, then the snapshots are already
available; but if created with the same name but a new uuid, then we
finally know to clean up the mess.
But I'm not convinced that it scales well - if you create lots of
transient domains, all with snapshots, but never reuse the same
name, then the metadata directory keeps growing without bounds even
though you never have that many domains at any given point of time.
Besides, since the only way to use libvirt to delete snapshot
metadata is via a virDomainSnapshotPtr object, but the only way to
get at a snapshot is via a virDomainPtr object, I think it is risky
to leave stale metadata in libvirt's directory trees that _cannot_
be removed by any existing libvirt API.
Cleaning at the time the transient domain goes away is nicer for
scalability. It means that nothing will be stored in
/var/lib/libvirt/qemu/ unless it can also be accessed via a current
virDomainPtr. It is also nicer for migration - remember, in my RFC,
I mentioned how snapshots and migration interact - the only sane way
is for the destination to start from a blank state, then recreate
the same snapshot hierarchy as was present on the source alongside
the migration, then remove the snapshot metadata on the source. If
we keep snapshot metadata around even after a transient domain is no
longer running on a host, then it becomes harder to start from a
clean slate when migrating a new guest by that name onto the host.
>>If resurrecting previous snapshots is important, then it should be
>>done with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE.
>
>This is all workable, but is it overkill compared to just validating
>the original VM uuid against the new UUID when loading metdata ?
I hope that we can come to agreement on whether or not making the
management app of the transient domain have to track snapshot
metadata counts as overkill or not.
I don't have a strong feeling on it, as long as we have a way to
get back the snapshots when recreating a transient guest. So lets
go with your suggestion.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4871
days inactive
4871
days old
10 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake