On Jan 20, 2010, at 6:54 AM, Daniel P. Berrange wrote:
> On Sun, Jan 17, 2010 at 12:31:07PM -0500, Philip Jameson wrote:
>> A little while ago I submitted a patch to add a snapshot API, but
>> I was informed that it got somewhat lost in the list. I have attached
>> a patch that implements this API at least for the qemu driver, and
>> it also added another function for screenshots, just because it
>> usually is nice to get a view of the screen with the snapshot.
>
> Could you resubmit the virDomainTakeScreenshot() code as a separate
> patch. That is quite a straightforward API addition, so we shouldn't
> let snapshot discussions delay that.
>
Will do, just need to separate it out from the other code.
>> +int virDomainTakeSnapshot (virDomainPtr domain,
>> + const char *name);
>> +int virDomainRestoreSnapshot(virDomainPtr domain,
>> + const char *name);
>> +int virDomainDeleteSnapshot (virDomainPtr domain,
>> + const char *name);
>> +int virDomainListSnapshots (virDomainPtr domain,
>> + char** sslist, int listsize);
>
> So thinking about things in terms of VMWare/VirtualBox capabilities, we
> probably need to expand these APIs a little further to
>
>
> typedef struct _virDomainSnapshot virDomainSnapshot;
>
> /* Take a snapshot of the current VM state */
> virDomainSnapshotPtr virDomainTakeSnapshot(virDomainPtr domain,
> const char *name,
> unsigned int flags);
>
> /* Query all snapshots know for a VM */
> int virDomainListSnapshotNames(virDomainPtr domain,
> char** names, int nameslen);
>
> /* Get a handle to a named snapshot */
> virDomainSnapshotPtr virDomainSnapshotLookupByName(virDomainPtr domain,
> const char *name);
>
> /* Set this snapshot as the current one for a domain, to be
> used next time domain is started */
> int virDomainSnapshotActivate(virDomainSnapshotPtr snapshot,
> unsigned int flags);
>
> /* Delete a snapshot */
> enum {
> VIR_DOMAIN_SNAPSHOT_DELETE_MERGE,
> VIR_DOMAIN_SNAPSHOT_DELETE_DISCARD,
> };
> int virDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
> unsigned int flags);
>
I think that's probably a good modification to allow for storing more information on the snapshots, however, did you remove the restore function, or is that what you're wanting the 'virDomainSnapshotActivate' to be? If so, that doesn't make as much sense at least for qemu, as it can do restoration of snapshots while online. If this was intended to replace the restore function, I suppose online/immediate restoration could be one of the flags.
>
> For XML to describe the metadata, I think we want to try something
> like
>
> <domainsnapshot>
> <name>XYZ</name>
> <creationdate>...</creationdate>
> <description>
> ....blah....
> </description>
> <state>RUNNING</state>
> <domain>
> <uuid>XXXXX-XXXX-XXXX-XXXX-XXXXXXXXX</uuid>
> </domain>
> <parent>
> <name>ABC</name>
> </parent>
> </domainsnapshot>
>
That's probably a fair layout. The only thing I might add is that for qemu/kvm support, since it is stored on the first block device, I believe, we would need to keep track of any shuffling of drives. So, maybe add a <blockdev>/path/to/hd</blockdev> tag so we know whether you can actually restore a snapshot at the time.
Philip Jameson
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list