Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote:
> +
> +typedef struct SNTime {
> + uint32_t date_sec; /* UTC date of the snapshot */
> + uint32_t date_nsec;
This two fields are just struct timespec, does it makes sense to use it?
> + uint64_t vm_clock_nsec; /* VM clock relative to boot */
> +} SNTime;
> +
> +typedef enum BlkSnapshotIntStep {
> + BLK_SNAPSHOT_INT_START = 0,
> + BLK_SNAPSHOT_INT_CREATED,
> + BLK_SNAPSHOT_INT_CANCELED,
> +} BlkSnapshotIntStep;
> +
> +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 */
> + QEMUSnapshotInfo sn;
> + QEMUSnapshotInfo old_sn;
> + bool old_sn_exist;
> + BlkSnapshotIntStep step;
> +} BlkSnapshotInternal;
> +
> +/* external snapshot */
> +
> +typedef enum BlkSnapshotExtStep {
> + BLK_SNAPSHOT_EXT_START = 0,
> + BLK_SNAPSHOT_EXT_CREATED,
> + BLK_SNAPSHOT_EXT_INVALIDATED,
> + BLK_SNAPSHOT_EXT_CANCELED,
> +} BlkSnapshotExtStep;
> +
> +typedef struct BlkSnapshotExternal {
> + /* caller input */
> + const char *new_image_file; /* must be set in create/delete. */
> + BlockDriverState *old_bs; /* must be set in create/delete. */
> + const char *format; /* must be set in create. */
> + /* following were used internal */
> + BlockDriverState *new_bs;
> + BlockDriver *format_drv;
> + BlkSnapshotExtStep step;
> +} BlkSnapshotExternal;
> +
> +typedef enum BlkSnapshotType {
> + BLK_SNAPSHOT_INTERNAL = 0,
> + BLK_SNAPSHOT_EXTERNAL,
> + BLK_SNAPSHOT_NOSUPPORT,
> +} BlkSnapshotType;
> +
> +/* for simple sync type params were all put here ignoring the difference of
> + different operation type as create/delete. */
> +typedef struct BlkTransactionStatesSync {
> + /* caller input */
> + BlkSnapshotType type;
> + union {
> + BlkSnapshotInternal internal;
> + BlkSnapshotExternal external;
> + };
> + bool use_existing;
> + BlkTransactionOperationSync op;
> +} BlkTransactionStatesSync;
> +
> +/* async snapshot, not supported now */
> +typedef struct BlkTransactionStatesAsync {
> + int reserved;
> +} BlkTransactionStatesAsync;
> +
> +/* Core structure for group snapshots, fill in it and then call the API. */
> +typedef struct BlkTransactionStates BlkTransactionStates;
> +
> +struct BlkTransactionStates {
> + /* caller input */
> + bool async;
Why do we have this variable? As far as I can see, we only test its
value, and set it to false. Are we missing any patch?
> + union {
> + BlkTransactionStatesSync st_sync;
> + BlkTransactionStatesSync st_async;
> + };
> + /* following were used internal */
> + Error *err;
> + int (*blk_trans_do)(BlkTransactionStates *states, Error **errp);
> + int (*blk_trans_invalid)(BlkTransactionStates *states, Error **errp);
> + int (*blk_trans_cancel)(BlkTransactionStates *states, Error **errp);
> + QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
> +};
> +
> +typedef QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionStates) \
> + BlkTransactionStatesList;
> +
> +/* API */
> +BlkTransactionStates *blk_trans_st_new(void);
> +void blk_trans_st_delete(BlkTransactionStates **p_st);
> +BlkTransactionStatesList *blk_trans_st_list_new(void);
> +void blk_trans_st_list_delete(BlkTransactionStatesList **p_list);
> +
> +/* add a request to list, request would be checked to see if it is valid,
> + return -1 when met error and states would not be queued. */
> +int add_transaction(BlkTransactionStatesList *list,
> + BlkTransactionStates *states,
> + Error **errp);
> +
> +/* 'Atomic' submit the request. In snapshot creation case, if any
> + * fail then we do not pivot any of the devices in the group, and abandon
> the
> + * snapshots
> + */
> +int submit_transaction(BlkTransactionStatesList *list, Error **errp);
> +
> +/* helper */
> +SNTime get_sn_time(void);
> +void generate_sn_name_from_time(SNTime *time, char *time_str, int size);
> #endif
