[libvirt] [RFC]: Snapshot API

[Chris Lalancette]

On 03/24/2010 05:51 AM, Daniel Veillard wrote:
>>> /* 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);
>>
>> This delayed semantic cannot be implemented for ESX. ESX can revert to
>> a snapshot immediately only. I think the same holds true for
>> VirtualBox.
>>
>> Maybe I misunterstand this. What should happen if you call activate on
>> a running domain?
> 
>   I guess we really need to use the flags there, and check for the flag
> semantic in the driver.
>   In ESX we will need a VIR_DOMAIN_SNAPSHOT_ACTIVATE_NOW flag to be passed
>   In QEmu we will need a VIR_DOMAIN_SNAPSHOT_ACTIVATE_RESTART to be passed
> 
>  Or we split that as two different entry points as the semantic is
> fairly different. This may be a bit cleaner, for example if you think of
> a virsh command,
>   virsh snap_activate --now foo snap1
>   virsh snap_activate --restart foo snap1
> 
> where the command without flags would be rejected because there is no
> common semantic, versus something like
> 
>   virsh snap_use foo snap1
>   virsh snap_restart foo snap1
> 
> where using snap_use on qemu would raise a unsupported or snap_restart
> on ESX.
> 
>   Semantic is so different, we can't use it without some flags or
> have 
>   virsh snap_activate foo snap1
> fail while
>   virsh snap_activate --restart foo snap1
> would work, that's disturbing.
> 
>   So I really think we need 2 different entry point
> 
>   virDomainSnapshotActivateNow
> 
>   virDomainSnapshotActivateLater
> 
> for example (and still with flags)

Yeah, this is the weak point of the whole thing.  I actually
think that I agree with Dan and Mattias now that we just need
"virDomainSnapshotCreateWithSnapshot" API that starts this
domain now against this particular snapshot.  That way you
don't have the (potentially confusing) semantic of activating
a snapshot, and then it taking effect on the next reboot.

> 
>>> int virDomainSnapshotFree(virDomainSnapshotPtr snapshot);
> 
>   Shouldn't we add something like virDomainSnapshotDestroy() too
> assuming the semantic of Free is just to discard the libvirt object
> but not remove the actual data.
 
Right, virDomainSnapshotFree() is just freeing up the libvirt object,
not removing the data.  The API that removes the data is the contended
virDomainSnapshotDelete/virDomainSnapshotDeactivate... etc discussion.
The name is honestly not that important at the moment; I'll probably
leave it as Deactivate in my current patches and then we can come up
with a final name once I post the patches.

-- 
Chris Lalancette