On Thu, Jan 06, 2011 at 02:27:14PM +0100, Jiri Denemark wrote:
As different snapshot methods may require different input data, new
diskSnapshot XML element is introduced to describe disk snapshot method:
<diskSnapshot method='qemu|lvm|btrfs|...'>
<!-- required data depending on the method -->
</diskSnapshot>
For qemu method, the element can be very simple:
<diskSnapshot method='qemu'/>
For lvm, logical volume device needs to be specified in case it is not the
same as disk to be snapshotted:
<diskSnapshot method='lvm'>
<device path='/dev/vg/lv4'/>
</diskSnapshot>
Enterprise storage would need their own ways of identifying volumes to be
snapshotted.
The diskSnapshot element can be used inside disk element in domain XML:
<domain ...>
...
<devices>
...
<disk ...>
...
<diskSnapshot ...>
...
</diskSnapshot>
</disk>
</devices>
</domain>
The "disk" part of "diskSnapshot" element name may seem to be
redundant but I
wanted to avoid confusion with snapshot XML element used for domain snapshots.
I think it is rather odd to include the <diskSnapshot> metadata in
the main <domain> XML itself. The <diskSnapshot> data may well only
be valid for a single snapshot operation - eg if you're doing snapshots
using QCow2 backing stores, then the path inside the <diskSnapshot>
surely has to change every time in invoke the API. This doesn't really
feel like guest configuration data, it is just a set of parameters for
an API and thus should always be passed into the API when invoked.
Existing virDomainUpdateDevice API can be used to alter snapshot
method on
existing disk devices.
Again I don't see the point in doing this extra work, when you can just
pass the data straight into virDomainDiskSnapshotCreate when you need
to run it.
To create a snapshot of a disk, the following API is introduced:
int
virDomainDiskSnapshotCreate(virDomainPtr domain,
const char *disk,
const char *method,
const char *name,
char **modifiedDisk,
char **backupSource,
unsigned int flags);
@domain
pointer to domain object
@disk
XML definition of the disk to snapshot
@method
optional <diskSnapshot> XML element overriding the one from <disk>
element; if none of them is specified, default method according to disk
type (and pool) is used; e.g., qcow disk => qemu method; logical volume in
LVM pool => lvm method
This I'd consider mandatory
@name
snapshot name
What is the 'name' in this context ? In case of QCow2, AFAICT, the only
identifier
that really matters would be the filename, which is surely part of the
<diskSnapshot>
XML passed into @method. Likewise for non-QCow@ types, wouldn't @method already have
the unique identifiers you need ?
@modifiedDisk
place where to store modified XML description of the disk which the domain
is now using; NULL is stored if domain is still using the original disk
(snapshot was created separately)
@backupSource
place where to store 'source' element from disk XML describing the disk
which can be used to take backups of 'disk' (i.e., read-only and immutable
snapshot); it might either be the same as provided in 'disk' or something
else (depending on method/implementation used); e.g., for qemu method, the
element describes previous disk source; lvm creates a new device for
snapshot and keeps writing into the original device
At which point neither of these really have any good reason to
exist.
@flags
OR'ed set of flags:
- VIR_DOMAIN_DISK_SNAPSHOT_QUIESCE_REQUIRED -- if no guest agent is
running/answering requests for consistent disk state, fail the API;
otherwise, the snapshot will be done regardless
If the guest has multiple disks, then apps will need to iterate over
each disk, invoking virDomainDiskSnapshotCreate for each one. If they
do that, then there's a reasonable chance they'll want to do the
quiesce once upfront, so all disks are consistent wrt each other.
This kind of suggests we want a virDomainQuiesceStorage() and
virDomainUnquiesceStorage() API pair.
I have a slight feeling that the API is a bit over-engineered but
I'm not
entirely sure if it can be simplified and still provide the flexibility and
future-compatibility. I have this feeling especially about backupSource output
parameter which might possibly replaced with a simple char * (eventually
returned directly by the API instead of int) containing file/device path.
Another think which is not strictly needed is modifiedDisk. The caller can ask
for domain XML and look the device there if needed but that would be quite
complicated. Thus returning it from this API seemed useful and logical too,
since the API is possibly changing disk XML and it make sense to return the
changes.
Deleting/merging snapshots previously created by virDomainDiskSnapshotCreate
is not covered by this proposal and will need to be added in the future to
complete disk snapshot support.
As long as the new files created via this are all within scope of a storage
pool, then they can be deleted that way. Merging snapshots is also probably
something that'd probably want to be done via the storage pool APIs.
Regards,
Daniel