On Tue, Aug 3, 2010 at 5:19 PM, Miguel Di Ciurcio Filho
<miguel.fi...@gmail.com> wrote:
> As discussed on today's call, here is a prototype to support snapshots
> inherantance in qcow2 and to use uuid as identification mechanism.
>
> Bugs/Limitations:
> * 'info snapshots' output is huge
> Displaying one item per line seams cumbersome. Maybe we should have two
> commands,
> like lvscan and lvdisplay. One very simple, the other one show full
> information.
>
> * Better distinguish ids and names
> I would like to suggest split functions like bdrv_snapshot_find() to something
> more strict, like bdrv_snapshot_find_by_id() and bdrv_snapshot_find_by_name()
>
> Signed-off-by: Miguel Di Ciurcio Filho <miguel.fi...@gmail.com>
> ---
> block.c | 8 +++---
> block.h | 1 +
> block/qcow2-snapshot.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++++-
> block/qcow2.h | 1 +
> savevm.c | 17 ++++++++++++++
> 5 files changed, 80 insertions(+), 5 deletions(-)
>
> diff --git a/block.c b/block.c
> index 452ae94..f305744 100644
> --- a/block.c
> +++ b/block.c
> @@ -1941,8 +1941,8 @@ char *bdrv_snapshot_dump(char *buf, int buf_size,
> QEMUSnapshotInfo *sn)
>
> if (!sn) {
> snprintf(buf, buf_size,
> - "%-10s%-20s%7s%20s%15s",
> - "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
> + "%-37s%-37s%-20s%7s%20s%15s",
> + "ID", "PARENT ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
> } else {
> ti = sn->date_sec;
> #ifdef _WIN32
> @@ -1962,8 +1962,8 @@ char *bdrv_snapshot_dump(char *buf, int buf_size,
> QEMUSnapshotInfo *sn)
> (int)(secs % 60),
> (int)((sn->vm_clock_nsec / 1000000) % 1000));
> snprintf(buf, buf_size,
> - "%-10s%-20s%7s%20s%15s",
> - sn->id_str, sn->name,
> + "%-37s%-37s%-20s%7s%20s%15s",
> + sn->id_str, sn->parent_id_str, sn->name,
> get_human_readable_size(buf1, sizeof(buf1),
> sn->vm_state_size),
> date_buf,
> clock_buf);
> diff --git a/block.h b/block.h
> index db131a3..e3c040a 100644
> --- a/block.h
> +++ b/block.h
> @@ -25,6 +25,7 @@ typedef struct QEMUSnapshotInfo {
> uint32_t date_sec; /* UTC date of the snapshot */
> uint32_t date_nsec;
> uint64_t vm_clock_nsec; /* VM clock relative to boot */
> + char parent_id_str[128];
> } QEMUSnapshotInfo;
>
> #define BDRV_O_RDWR 0x0002
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 6228612..97877dd 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -54,6 +54,7 @@ void qcow2_free_snapshots(BlockDriverState *bs)
> for(i = 0; i < s->nb_snapshots; i++) {
> qemu_free(s->snapshots[i].name);
> qemu_free(s->snapshots[i].id_str);
> + qemu_free(s->snapshots[i].parent_id_str);
> }
> qemu_free(s->snapshots);
> s->snapshots = NULL;
> @@ -69,6 +70,8 @@ int qcow2_read_snapshots(BlockDriverState *bs)
> int64_t offset;
> uint32_t extra_data_size;
>
> + extra_data_size = 0;
> +
> if (!s->nb_snapshots) {
> s->snapshots = NULL;
> s->snapshots_size = 0;
> @@ -94,7 +97,15 @@ int qcow2_read_snapshots(BlockDriverState *bs)
> id_str_size = be16_to_cpu(h.id_str_size);
> name_size = be16_to_cpu(h.name_size);
>
> - offset += extra_data_size;
> + if (extra_data_size > 0) {
> + sn->parent_id_str = qemu_malloc(extra_data_size);
> + if (bdrv_pread(bs->file, offset, sn->parent_id_str,
> extra_data_size) != extra_data_size) {
> + goto fail;
> + }
> +
> + offset += extra_data_size;
> + sn->parent_id_str[extra_data_size] = '\0';
> + }
>
> sn->id_str = qemu_malloc(id_str_size + 1);
> if (bdrv_pread(bs->file, offset, sn->id_str, id_str_size) !=
> id_str_size)
> @@ -125,6 +136,9 @@ static int qcow_write_snapshots(BlockDriverState *bs)
> uint64_t data64;
> uint32_t data32;
> int64_t offset, snapshots_offset;
> + uint32_t extra_data_size;
> +
> + extra_data_size = 0;
>
> /* compute the size of the snapshots */
> offset = 0;
> @@ -132,6 +146,9 @@ static int qcow_write_snapshots(BlockDriverState *bs)
> sn = s->snapshots + i;
> offset = align_offset(offset, 8);
> offset += sizeof(h);
> + if (sn->parent_id_str) {
> + offset += strlen(sn->parent_id_str);
> + }
> offset += strlen(sn->id_str);
> offset += strlen(sn->name);
> }
> @@ -153,6 +170,11 @@ static int qcow_write_snapshots(BlockDriverState *bs)
> h.date_nsec = cpu_to_be32(sn->date_nsec);
> h.vm_clock_nsec = cpu_to_be64(sn->vm_clock_nsec);
>
> + if (sn->parent_id_str) {
> + extra_data_size = strlen(sn->parent_id_str);
> + h.extra_data_size = cpu_to_be32(extra_data_size);
> + }
> +
> id_str_size = strlen(sn->id_str);
> name_size = strlen(sn->name);
> h.id_str_size = cpu_to_be16(id_str_size);
> @@ -161,6 +183,15 @@ static int qcow_write_snapshots(BlockDriverState *bs)
> if (bdrv_pwrite_sync(bs->file, offset, &h, sizeof(h)) < 0)
> goto fail;
> offset += sizeof(h);
> +
> + if (sn->parent_id_str) {
> + if (bdrv_pwrite_sync(bs->file, offset, sn->parent_id_str,
> extra_data_size) < 0) {
> + goto fail;
> + }
> +
> + offset += extra_data_size;
> + }
> +
> if (bdrv_pwrite_sync(bs->file, offset, sn->id_str, id_str_size) < 0)
> goto fail;
> offset += id_str_size;
> @@ -257,6 +288,9 @@ int qcow2_snapshot_create(BlockDriverState *bs,
> QEMUSnapshotInfo *sn_info)
> sn->name = qemu_strdup(sn_info->name);
> if (!sn->name)
> goto fail;
> +
> + sn->parent_id_str = qemu_strdup(sn_info->parent_id_str);
> +
> sn->vm_state_size = sn_info->vm_state_size;
> sn->date_sec = sn_info->date_sec;
> sn->date_nsec = sn_info->date_nsec;
> @@ -352,6 +386,19 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char
> *snapshot_id)
> return -EIO;
> }
>
> +static void qcow2_update_parent_id_references(BlockDriverState *bs,
> QCowSnapshot *sn)
> +{
> + BDRVQcowState *s = bs->opaque;
> + int i;
> +
> + for(i = 0; i < s->nb_snapshots; i++) {
> + if (!strcmp(s->snapshots[i].parent_id_str, sn->id_str)) {
> + pstrcpy(s->snapshots[i].parent_id_str, strlen(sn->parent_id_str),
> + sn->parent_id_str);
> + }
> + }
> +}
> +
> int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
> {
> BDRVQcowState *s = bs->opaque;
> @@ -372,7 +419,10 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const
> char *snapshot_id)
> return ret;
> qcow2_free_clusters(bs, sn->l1_table_offset, sn->l1_size *
> sizeof(uint64_t));
>
> + qcow2_update_parent_id_references(bs, sn);
> +
> qemu_free(sn->id_str);
> + qemu_free(sn->parent_id_str);
> qemu_free(sn->name);
> memmove(sn, sn + 1, (s->nb_snapshots - snapshot_index - 1) * sizeof(*sn));
> s->nb_snapshots--;
> @@ -407,6 +457,12 @@ int qcow2_snapshot_list(BlockDriverState *bs,
> QEMUSnapshotInfo **psn_tab)
> sn->id_str);
> pstrcpy(sn_info->name, sizeof(sn_info->name),
> sn->name);
> +
> + if (sn->parent_id_str) {
> + pstrcpy(sn_info->parent_id_str, sizeof(sn_info->parent_id_str),
> + sn->parent_id_str);
> + }
> +
> sn_info->vm_state_size = sn->vm_state_size;
> sn_info->date_sec = sn->date_sec;
> sn_info->date_nsec = sn->date_nsec;
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 3ff162e..d40e6bf 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -76,6 +76,7 @@ typedef struct QCowSnapshot {
> uint32_t date_sec;
> uint32_t date_nsec;
> uint64_t vm_clock_nsec;
> + char *parent_id_str;
> } QCowSnapshot;
>
> typedef struct BDRVQcowState {
> diff --git a/savevm.c b/savevm.c
> index 4c0e5d3..6ed839b 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -28,6 +28,7 @@
> #include <errno.h>
> #include <sys/time.h>
> #include <zlib.h>
> +#include <uuid/uuid.h>
This header is not available on all systems.
>
> /* Needed early for CONFIG_BSD etc. */
> #include "config-host.h"
> @@ -86,6 +87,8 @@
>
> #define SELF_ANNOUNCE_ROUNDS 5
>
> +static char current_snapshot_id[128];
> +
> #ifndef ETH_P_RARP
> #define ETH_P_RARP 0x8035
> #endif
> @@ -1797,6 +1800,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> QEMUFile *f;
> int saved_vm_running;
> uint32_t vm_state_size;
> + uuid_t uuid_buf;
> #ifdef _WIN32
> struct _timeb tb;
> #else
> @@ -1841,6 +1845,16 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> }
> }
>
> + if (strlen(current_snapshot_id) > 0) {
> + pstrcpy(sn->parent_id_str, sizeof(sn->parent_id_str),
> current_snapshot_id);
> + } else {
> + pstrcpy(sn->parent_id_str, sizeof(sn->parent_id_str),
> "00000000-0000-0000-0000-000000000000");
> + }
> +
> + uuid_generate(uuid_buf);
> + uuid_unparse(uuid_buf, sn->id_str);
We have qemu_uuid which is defined with command line switch, can you use that?
> + pstrcpy(current_snapshot_id, sizeof(current_snapshot_id), sn->id_str);
> +
> /* fill auxiliary fields */
> #ifdef _WIN32
> _ftime(&tb);
> @@ -1969,6 +1983,9 @@ int load_vmstate(const char *name)
> error_report("Error %d while loading VM state", ret);
> return ret;
> }
> +
> + pstrcpy(current_snapshot_id, sizeof(current_snapshot_id), sn.id_str);
> +
> return 0;
> }
>
> --
> 1.7.1
>
>
>
