| Message ID | 1280855966-11297-1-git-send-email-miguel.filho@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 3, 2010 at 5:19 PM, Miguel Di Ciurcio Filho <miguel.filho@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.filho@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 > > >
On Tue, Aug 3, 2010 at 5:55 PM, Blue Swirl <blauwirbel@gmail.com> wrote: >> + 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? > Didn't know that. It does the job, but there is some code inside qemu_uuid_parse() that I'm not familiar with: int qemu_uuid_parse(const char *str, uint8_t *uuid) { ... #ifdef TARGET_I386 smbios_add_field(1, offsetof(struct smbios_type_1, uuid), 16, uuid); #endif return 0; } I see that there are more uuid functions and spread in the code: - block/vdi.c: typedef unsigned char uuid_t[16]; void uuid_generate(uuid_t out); int uuid_is_null(const uuid_t uu); void uuid_unparse(const uuid_t uu, char *out); - monitor.c: do_info_uuid() does the conversion manually too, instead of using unparse/parse. I think it would be better to cleanup this stuff and have a common code shaded. Regards, Miguel
On Tue, Aug 3, 2010 at 2:19 PM, Miguel Di Ciurcio Filho <miguel.filho@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. > Bump. Just a friendly reminder of this RFC, now that everybody is back from the KVM Forum :-D > 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.filho@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> > > /* 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); > + 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 > >
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> /* 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); + 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; }
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.filho@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(-)