On 12/16/2012 11:25 PM, Wenchao Xia wrote: > This patch added API to take snapshots in unified style for > both internal or external type. The core structure is based > on transaction, for that there is a qmp interface need to support > , qmp_transaction, so all operations are packed as requests. > In this way a sperate internal layer for snapshot is splitted > out from qmp layer, and now qmp can just translate the user request > and fill in internal API. Internal API use params defined inside > qemu, so other component inside qemu can use it without considering > the qmp's parameter format. >
> +typedef struct SNTime {
> + uint32_t date_sec; /* UTC date of the snapshot */
Relative to what? Seconds since Epoch? Shouldn't this be 64-bits, to
avoid wraparound problems in 2038?
> +typedef struct BlkSnapshotInternal {
> + /* caller input */
> + const char *sn_name; /* must be set in create/delete. */
> + BlockDriverState *bs; /* must be set in create/delete */
> + SNTime time; /* must be set in create. */
> + uint64_t vm_state_size; /* optional, default is 0, only valid in create.
> */
> + /* following were used internal */
Prefer present tense: The following are for internal use
> +
> +/* for simple sync type params were all put here ignoring the difference of
> + different operation type as create/delete. */
> +typedef struct BlkTransactionStatesSync {
Again, prefer present tense (avoid 'were' in comments).
> +/* async snapshot, not supported now */
> +typedef struct BlkTransactionStatesAsync {
> + int reserved;
> +} BlkTransactionStatesAsync;
Why declare a struct if we aren't supporting it yet?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
