The Saturday 26 Jul 2014 à 21:22:08 (+0200), Max Reitz wrote :
> The only really time-consuming operation potentially performed by
> qcow2_amend_options() is zero cluster expansion when downgrading qcow2
> images from compat=1.1 to compat=0.10, so report status of that
> operation and that operation only through the status CB.
>
> For this, approximate the progress as the number of L1 entries visited
> during the operation.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/qcow2-cluster.c | 36 ++++++++++++++++++++++++++++++++----
> block/qcow2.c | 9 ++++-----
> block/qcow2.h | 3 ++-
> 3 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 4208dc0..f8bec6f 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1548,10 +1548,17 @@ fail:
> * zero expansion (i.e., has been filled with zeroes and is referenced from
> an
> * L2 table). nb_clusters contains the total cluster count of the image file,
> * i.e., the number of bits in expanded_clusters.
> + *
> + * l1_entries and *visited_l1_entries are ued to keep track of progress for
> + * status_cb(). l1_entries contains the total number of L1 entries and
> + * *visited_l1_entries counts all visited L1 entries.
> */
> static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t
> *l1_table,
> int l1_size, uint8_t
> **expanded_clusters,
> - uint64_t *nb_clusters)
> + uint64_t *nb_clusters,
> + int64_t *visited_l1_entries,
> + int64_t l1_entries,
> + BlockDriverAmendStatusCB *status_cb)
> {
> BDRVQcowState *s = bs->opaque;
> bool is_active_l1 = (l1_table == s->l1_table);
> @@ -1571,6 +1578,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState
> *bs, uint64_t *l1_table,
>
> if (!l2_offset) {
> /* unallocated */
> + (*visited_l1_entries)++;
> continue;
> }
>
> @@ -1703,6 +1711,13 @@ static int expand_zero_clusters_in_l1(BlockDriverState
> *bs, uint64_t *l1_table,
> }
> }
> }
> +
> + (*visited_l1_entries)++;
> +
> + if (status_cb) {
> + status_cb(bs, *visited_l1_entries << (s->l2_bits +
> s->cluster_bits),
> + l1_entries << (s->l2_bits + s->cluster_bits));
Shifting is a multiplication so it keep proportionality intact.
So do we really need these shifts ?
> + }
> }
>
> ret = 0;
> @@ -1729,21 +1744,32 @@ fail:
> * allocation for pre-allocated ones). This is important for downgrading to a
> * qcow2 version which doesn't yet support metadata zero clusters.
> */
> -int qcow2_expand_zero_clusters(BlockDriverState *bs)
> +int qcow2_expand_zero_clusters(BlockDriverState *bs,
> + BlockDriverAmendStatusCB *status_cb)
> {
> BDRVQcowState *s = bs->opaque;
> uint64_t *l1_table = NULL;
> uint64_t nb_clusters;
> + int64_t l1_entries = 0, visited_l1_entries = 0;
> uint8_t *expanded_clusters;
> int ret;
> int i, j;
>
> + if (status_cb) {
> + l1_entries = s->l1_size;
> + for (i = 0; i < s->nb_snapshots; i++) {
> + l1_entries += s->snapshots[i].l1_size;
> + }
> + }
> +
> nb_clusters = size_to_clusters(s, bs->file->total_sectors *
> BDRV_SECTOR_SIZE);
> expanded_clusters = g_malloc0((nb_clusters + 7) / 8);
>
> ret = expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size,
> - &expanded_clusters, &nb_clusters);
> + &expanded_clusters, &nb_clusters,
> + &visited_l1_entries, l1_entries,
> + status_cb);
> if (ret < 0) {
> goto fail;
> }
> @@ -1777,7 +1803,9 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs)
> }
>
> ret = expand_zero_clusters_in_l1(bs, l1_table,
> s->snapshots[i].l1_size,
> - &expanded_clusters, &nb_clusters);
> + &expanded_clusters, &nb_clusters,
> + &visited_l1_entries, l1_entries,
> + status_cb);
> if (ret < 0) {
> goto fail;
> }
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 757f890..6e8c8ab 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2147,7 +2147,8 @@ static int qcow2_load_vmstate(BlockDriverState *bs,
> uint8_t *buf,
> * Downgrades an image's version. To achieve this, any incompatible features
> * have to be removed.
> */
> -static int qcow2_downgrade(BlockDriverState *bs, int target_version)
> +static int qcow2_downgrade(BlockDriverState *bs, int target_version,
> + BlockDriverAmendStatusCB *status_cb)
> {
> BDRVQcowState *s = bs->opaque;
> int current_version = s->qcow_version;
> @@ -2196,7 +2197,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int
> target_version)
> /* clearing autoclear features is trivial */
> s->autoclear_features = 0;
>
> - ret = qcow2_expand_zero_clusters(bs);
> + ret = qcow2_expand_zero_clusters(bs, status_cb);
> if (ret < 0) {
> return ret;
> }
> @@ -2224,8 +2225,6 @@ static int qcow2_amend_options(BlockDriverState *bs,
> QemuOpts *opts,
> int ret;
> QemuOptDesc *desc = opts->list->desc;
>
> - (void)status_cb;
> -
> while (desc && desc->name) {
> if (!qemu_opt_find(opts, desc->name)) {
> /* only change explicitly defined options */
> @@ -2291,7 +2290,7 @@ static int qcow2_amend_options(BlockDriverState *bs,
> QemuOpts *opts,
> return ret;
> }
> } else {
> - ret = qcow2_downgrade(bs, new_version);
> + ret = qcow2_downgrade(bs, new_version, status_cb);
> if (ret < 0) {
> return ret;
> }
> diff --git a/block/qcow2.h b/block/qcow2.h
> index b49424b..1c4f9bf 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -522,7 +522,8 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t
> offset,
> int nb_sectors, enum qcow2_discard_type type);
> int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int
> nb_sectors);
>
> -int qcow2_expand_zero_clusters(BlockDriverState *bs);
> +int qcow2_expand_zero_clusters(BlockDriverState *bs,
> + BlockDriverAmendStatusCB *status_cb);
>
> /* qcow2-snapshot.c functions */
> int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
> --
> 2.0.3
>
>
Reviewed-by: Benoit Canet <ben...@irqsave.net>
