From: Anthony Liguori <anthony@codemonkey.ws>
To: Rob Earhart <earhart@google.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] Use snapshots from backing disks
Date: Wed, 17 Mar 2010 10:35:50 -0500 [thread overview]
Message-ID: <4BA0F6D6.9090009@codemonkey.ws> (raw)
In-Reply-To: <377f3a9b1003081713v6f39fae2sb15f80073d13215a@mail.gmail.com>
On 03/08/2010 07:13 PM, Rob Earhart wrote:
> Modify the snapshot load path to find and load snapshots contained in backing
> disks, useful when the current disk is a differencing disk.
>
> Add the source of a snapshot when listing snapshots.
>
> This should only break backwards compatibility for scenarios depending on not
> being able to load snapshots from backing disks (which doesn't seem like a
> problem), and for code which parses the snapshot list output (if any).
>
> Signed-off-by: Rob Earhart<earhart@google.com>
>
Your patch is whitespace damaged. Please use a different mailer (like
git-send-email).
Regards,
Anthony Liguori
> ---
> block.c | 10 +++++
> block.h | 2 +
> block/qcow2-snapshot.c | 105 ++++++++++++++++++++++++++++++++++++------------
> qemu-img.c | 25 +++++++-----
> savevm.c | 92 +++++++++++++++++++++++++++++-------------
> 5 files changed, 170 insertions(+), 64 deletions(-)
>
> diff --git a/block.c b/block.c
> index 31d1ba4..3588700 100644
> --- a/block.c
> +++ b/block.c
> @@ -1481,6 +1481,16 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
> }
> }
>
> +const char *bdrv_get_filename(BlockDriverState *bs)
> +{
> + return bs->filename;
> +}
> +
> +BlockDriverState* bdrv_get_backing_bdrv(BlockDriverState *bs)
> +{
> + return bs->backing_hd;
> +}
> +
> int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
> const uint8_t *buf, int nb_sectors)
> {
> diff --git a/block.h b/block.h
> index edf5704..1d04322 100644
> --- a/block.h
> +++ b/block.h
> @@ -179,6 +179,8 @@ int bdrv_get_info(BlockDriverState *bs,
> BlockDriverInfo *bdi);
> const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
> void bdrv_get_backing_filename(BlockDriverState *bs,
> char *filename, int filename_size);
> +const char *bdrv_get_filename(BlockDriverState *bs);
> +BlockDriverState* bdrv_get_backing_bdrv(BlockDriverState *bs);
> int bdrv_snapshot_create(BlockDriverState *bs,
> QEMUSnapshotInfo *sn_info);
> int bdrv_snapshot_goto(BlockDriverState *bs,
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 8ddaea2..ca06fcf 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -191,16 +191,23 @@ static int qcow_write_snapshots(BlockDriverState *bs)
> static void find_new_snapshot_id(BlockDriverState *bs,
> char *id_str, int id_str_size)
> {
> - BDRVQcowState *s = bs->opaque;
> - QCowSnapshot *sn;
> - int i, id, id_max = 0;
> + int id_max = 0;
> +
> + assert(bs);
> +
> + do {
> + BDRVQcowState *s = bs->opaque;
> + QCowSnapshot *sn;
> + int i, id;
> +
> + for(i = 0; i< s->nb_snapshots; i++) {
> + sn = s->snapshots + i;
> + id = strtoul(sn->id_str, NULL, 10);
> + if (id> id_max)
> + id_max = id;
> + }
> + } while ((bs = bdrv_get_backing_bdrv(bs)));
>
> - for(i = 0; i< s->nb_snapshots; i++) {
> - sn = s->snapshots + i;
> - id = strtoul(sn->id_str, NULL, 10);
> - if (id> id_max)
> - id_max = id;
> - }
> snprintf(id_str, id_str_size, "%d", id_max + 1);
> }
>
> @@ -316,41 +323,88 @@ int qcow2_snapshot_goto(BlockDriverState *bs,
> const char *snapshot_id)
> {
> BDRVQcowState *s = bs->opaque;
> QCowSnapshot *sn;
> - int i, snapshot_index, l1_size2;
> + int i, snapshot_index, l1_size2, ret;
>
> snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
> - if (snapshot_index< 0)
> - return -ENOENT;
> +
> + if (snapshot_index< 0) {
> + /* if there is a backing file, use it. In this case, the
> + * current image needs to be reset to be consistent
> + */
> + if (bs->backing_hd) {
> + ret = bdrv_snapshot_goto(bs->backing_hd, snapshot_id);
> + if (ret< 0) {
> + return ret;
> + }
> + /* reset the sectors of the current image (Note: there can
> + * still be snapshots, so the refcounts must be kept
> + * consistent)
> + */
> + if (qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> + s->l1_size, -1)< 0) {
> + return -EIO;
> + }
> + l1_size2 = s->l1_size * sizeof(uint64_t);
> + memset(s->l1_table, 0, l1_size2);
> + if (bdrv_pwrite(s->hd, s->l1_table_offset,
> + s->l1_table, l1_size2) != l1_size2) {
> + return -EIO;
> + }
> + return 0;
> + } else {
> + return -ENOENT;
> + }
> + }
> +
> + assert(0<= snapshot_index);
> +
> sn =&s->snapshots[snapshot_index];
>
> - if (qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> s->l1_size, -1)< 0)
> - goto fail;
> + /* if the file is read-only, all can be done by updating the
> + * in-memory L1 table. Otherwise, we'll need to do some writes.
> + */
>
> - if (qcow2_grow_l1_table(bs, sn->l1_size)< 0)
> - goto fail;
> + if (!bs->read_only) {
> + if (qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> + s->l1_size, -1)
> +< 0) {
> + return -EIO;
> + }
> + }
> +
> + if (qcow2_grow_l1_table(bs, sn->l1_size)< 0) {
> + return -EIO;
> + }
>
> s->l1_size = sn->l1_size;
> l1_size2 = s->l1_size * sizeof(uint64_t);
> /* copy the snapshot l1 table to the current l1 table */
> if (bdrv_pread(s->hd, sn->l1_table_offset,
> - s->l1_table, l1_size2) != l1_size2)
> - goto fail;
> - if (bdrv_pwrite(s->hd, s->l1_table_offset,
> - s->l1_table, l1_size2) != l1_size2)
> - goto fail;
> + s->l1_table, l1_size2) != l1_size2) {
> + return -EIO;
> + }
> + if (!bs->read_only) {
> + if (bdrv_pwrite(s->hd, s->l1_table_offset,
> + s->l1_table, l1_size2) != l1_size2) {
> + return -EIO;
> + }
> + }
> for(i = 0;i< s->l1_size; i++) {
> be64_to_cpus(&s->l1_table[i]);
> }
>
> - if (qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> s->l1_size, 1)< 0)
> - goto fail;
> + if (!bs->read_only) {
> + if (qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> + s->l1_size, 1)
> +< 0) {
> + return -EIO;
> + }
> + }
>
> #ifdef DEBUG_ALLOC
> qcow2_check_refcounts(bs);
> #endif
> return 0;
> - fail:
> - return -EIO;
> }
>
> int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
> @@ -416,4 +470,3 @@ int qcow2_snapshot_list(BlockDriverState *bs,
> QEMUSnapshotInfo **psn_tab)
> *psn_tab = sn_tab;
> return s->nb_snapshots;
> }
> -
> diff --git a/qemu-img.c b/qemu-img.c
> index 0c9f2d4..345a768 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -873,16 +873,21 @@ static void dump_snapshots(BlockDriverState *bs)
> int nb_sns, i;
> char buf[256];
>
> - nb_sns = bdrv_snapshot_list(bs,&sn_tab);
> - if (nb_sns<= 0)
> - return;
> - printf("Snapshot list:\n");
> - printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> - for(i = 0; i< nb_sns; i++) {
> - sn =&sn_tab[i];
> - printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
> - }
> - qemu_free(sn_tab);
> + assert(bs);
> +
> + do {
> + nb_sns = bdrv_snapshot_list(bs,&sn_tab);
> + if (nb_sns<= 0)
> + continue;
> + printf("Snapshot list (from %s):\n",
> + bdrv_get_filename(bs));
> + printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> + for(i = 0; i< nb_sns; i++) {
> + sn =&sn_tab[i];
> + printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
> + }
> + qemu_free(sn_tab);
> + } while ((bs = bdrv_get_backing_bdrv(bs)));
> }
>
> static int img_info(int argc, char **argv)
> diff --git a/savevm.c b/savevm.c
> index 2fd3de6..8a81c42 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1616,25 +1616,53 @@ static BlockDriverState *get_bs_snapshots(void)
> return bs;
> }
>
> +/* Find a snapshot with the given name.
> + * sn_bs is an out parameter: if NULL, only the current BlockDriverState (bs)
> + * is checked; otherwise, *sb_bs is set to point to the backing block driver
> + * state where the found snapshot resides.
> + */
> static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
> - const char *name)
> + const char *name, BlockDriverState **sn_bs)
> {
> QEMUSnapshotInfo *sn_tab, *sn;
> int nb_sns, i, ret;
>
> + assert(bs);
> +
> ret = -ENOENT;
> - nb_sns = bdrv_snapshot_list(bs,&sn_tab);
> - if (nb_sns< 0)
> - return ret;
> - for(i = 0; i< nb_sns; i++) {
> - sn =&sn_tab[i];
> - if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
> - *sn_info = *sn;
> - ret = 0;
> + do {
> + nb_sns = bdrv_snapshot_list(bs,&sn_tab);
> + if (nb_sns< 0) {
> + if (nb_sns == -ENOTSUP) {
> + /* If this device does not support snapshots,
> + * its backing file still might; continue on
> + * to the backing file.
> + */
> + continue;
> + } else {
> + return ret;
> + }
> + }
> + for (i = 0; i< nb_sns; i++) {
> + sn =&sn_tab[i];
> + if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
> + *sn_info = *sn;
> + if (sn_bs != 0) {
> + *sn_bs = bs;
> + }
> + ret = 0;
> + break;
> + }
> + }
> + qemu_free(sn_tab);
> + if (ret == 0) {
> break;
> }
> - }
> - qemu_free(sn_tab);
> + if (!sn_bs) {
> + break;
> + }
> + } while ((bs = bdrv_get_backing_bdrv(bs)));
> +
> return ret;
> }
>
> @@ -1651,7 +1679,7 @@ static int del_existing_snapshots(Monitor *mon,
> const char *name)
> QTAILQ_FOREACH(dinfo,&drives, next) {
> bs = dinfo->bdrv;
> if (bdrv_can_snapshot(bs)&&
> - bdrv_snapshot_find(bs, snapshot, name)>= 0)
> + bdrv_snapshot_find(bs, snapshot, name, NULL)>= 0)
> {
> ret = bdrv_snapshot_delete(bs, name);
> if (ret< 0) {
> @@ -1696,7 +1724,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
>
> memset(sn, 0, sizeof(*sn));
> if (name) {
> - ret = bdrv_snapshot_find(bs, old_sn, name);
> + ret = bdrv_snapshot_find(bs, old_sn, name, NULL);
> if (ret>= 0) {
> pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
> pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
> @@ -1759,7 +1787,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> int load_vmstate(Monitor *mon, const char *name)
> {
> DriveInfo *dinfo;
> - BlockDriverState *bs, *bs1;
> + BlockDriverState *bs, *bs1, *sn_bs;
> QEMUSnapshotInfo sn;
> QEMUFile *f;
> int ret;
> @@ -1804,12 +1832,12 @@ int load_vmstate(Monitor *mon, const char *name)
> }
>
> /* Don't even try to load empty VM states */
> - ret = bdrv_snapshot_find(bs,&sn, name);
> + ret = bdrv_snapshot_find(bs,&sn, name,&sn_bs);
> if ((ret>= 0)&& (sn.vm_state_size == 0))
> return -EINVAL;
>
> /* restore the VM state */
> - f = qemu_fopen_bdrv(bs, 0);
> + f = qemu_fopen_bdrv(sn_bs, 0);
> if (!f) {
> monitor_printf(mon, "Could not open VM state file\n");
> return -EINVAL;
> @@ -1876,17 +1904,25 @@ void do_info_snapshots(Monitor *mon)
> }
> monitor_printf(mon, "\n");
>
> - nb_sns = bdrv_snapshot_list(bs,&sn_tab);
> - if (nb_sns< 0) {
> - monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
> - return;
> - }
> - monitor_printf(mon, "Snapshot list (from %s):\n",
> - bdrv_get_device_name(bs));
> - monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> - for(i = 0; i< nb_sns; i++) {
> - sn =&sn_tab[i];
> - monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
> + for (; bs; bs = bdrv_get_backing_bdrv(bs)) {
> + nb_sns = bdrv_snapshot_list(bs,&sn_tab);
> + if (nb_sns< 0) {
> + monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
> + continue;
> + }
> + const char* device_name = bdrv_get_device_name(bs);
> + if (device_name&& device_name[0]) {
> + monitor_printf(mon, "Snapshot list (device %s, from %s):\n",
> + device_name, bdrv_get_filename(bs));
> + } else {
> + monitor_printf(mon, "Snapshot list (from %s):\n",
> + bdrv_get_filename(bs));
> + }
> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf,
> sizeof(buf), NULL));
> + for(i = 0; i< nb_sns; i++) {
> + sn =&sn_tab[i];
> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf,
> sizeof(buf), sn));
> + }
> + qemu_free(sn_tab);
> }
> - qemu_free(sn_tab);
> }
>
next prev parent reply other threads:[~2010-03-17 15:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-09 1:13 [Qemu-devel] [PATCH] Use snapshots from backing disks Rob Earhart
2010-03-11 13:12 ` Kevin Wolf
2010-03-18 10:37 ` Kevin Wolf
2010-03-17 15:35 ` Anthony Liguori [this message]
2010-03-17 18:16 ` Rob Earhart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BA0F6D6.9090009@codemonkey.ws \
--to=anthony@codemonkey.ws \
--cc=earhart@google.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.