QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Alberto Garcia <berto@igalia.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Derek Su <dereksu@qnap.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org
Subject: Re: [PATCH v9 28/34] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes()
Date: Thu, 2 Jul 2020 16:28:57 +0200
Message-ID: <25384933-0ca3-3f50-c1af-4c92e8c88328@redhat.com> (raw)
In-Reply-To: <3c6140626c443f1eb443881eddcfb65b6107aff2.1593342067.git.berto@igalia.com>

[-- Attachment #1.1: Type: text/plain, Size: 3679 bytes --]

On 28.06.20 13:02, Alberto Garcia wrote:
> This works now at the subcluster level and pwrite_zeroes_alignment is
> updated accordingly.
> 
> qcow2_cluster_zeroize() is turned into qcow2_subcluster_zeroize() with
> the following changes:
> 
>    - The request can now be subcluster-aligned.
> 
>    - The cluster-aligned body of the request is still zeroized using
>      zero_in_l2_slice() as before.
> 
>    - The subcluster-aligned head and tail of the request are zeroized
>      with the new zero_l2_subclusters() function.
> 
> There is just one thing to take into account for a possible future
> improvement: compressed clusters cannot be partially zeroized so
> zero_l2_subclusters() on the head or the tail can return -ENOTSUP.
> This makes the caller repeat the *complete* request and write actual
> zeroes to disk. This is sub-optimal because
> 
>    1) if the head area was compressed we would still be able to use
>       the fast path for the body and possibly the tail.
> 
>    2) if the tail area was compressed we are writing zeroes to the
>       head and the body areas, which are already zeroized.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block/qcow2.h         |  4 +--
>  block/qcow2-cluster.c | 80 +++++++++++++++++++++++++++++++++++++++----
>  block/qcow2.c         | 27 ++++++++-------
>  3 files changed, 90 insertions(+), 21 deletions(-)

[...]

> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index deff838fe8..1641976028 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -2015,12 +2015,58 @@ static int zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
>      return nb_clusters;
>  }
>  
> -int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset,
> -                          uint64_t bytes, int flags)
> +static int zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> +                               unsigned nb_subclusters)
> +{
> +    BDRVQcow2State *s = bs->opaque;
> +    uint64_t *l2_slice;
> +    uint64_t old_l2_bitmap, l2_bitmap;
> +    int l2_index, ret, sc = offset_to_sc_index(s, offset);
> +
> +    /* For full clusters use zero_in_l2_slice() instead */
> +    assert(nb_subclusters > 0 && nb_subclusters < s->subclusters_per_cluster);
> +    assert(sc + nb_subclusters <= s->subclusters_per_cluster);

Maybe we should also assert that @offset is aligned to the subcluster size.

[...]

> diff --git a/block/qcow2.c b/block/qcow2.c
> index 86258fbc22..4edc3c72b9 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c

[...]

> @@ -4367,12 +4367,13 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>          uint64_t zero_start = QEMU_ALIGN_UP(old_length, s->cluster_size);

Can we instead align this to just subclusters?

>  
>          /*
> -         * Use zero clusters as much as we can. qcow2_cluster_zeroize()
> +         * Use zero clusters as much as we can. qcow2_subcluster_zeroize()
>           * requires a cluster-aligned start. The end may be unaligned if it is

s/cluster/subcluster/?

Max

>           * at the end of the image (which it is here).
>           */
>          if (offset > zero_start) {
> -            ret = qcow2_cluster_zeroize(bs, zero_start, offset - zero_start, 0);
> +            ret = qcow2_subcluster_zeroize(bs, zero_start, offset - zero_start,
> +                                           0);
>              if (ret < 0) {
>                  error_setg_errno(errp, -ret, "Failed to zero out new clusters");
>                  goto fail;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply index

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-28 11:02 [PATCH v9 00/34] Add subcluster allocation to qcow2 Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 01/34] qcow2: Make Qcow2AioTask store the full host offset Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 02/34] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset() Alberto Garcia
2020-06-30 10:19   ` Max Reitz
2020-06-30 10:27     ` Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 03/34] qcow2: Add calculate_l2_meta() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 04/34] qcow2: Split cluster_needs_cow() out of count_cow_clusters() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 05/34] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied() Alberto Garcia
2020-06-30 10:38   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 06/34] qcow2: Add get_l2_entry() and set_l2_entry() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 07/34] qcow2: Document the Extended L2 Entries feature Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 08/34] qcow2: Add dummy has_subclusters() function Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 09/34] qcow2: Add subcluster-related fields to BDRVQcow2State Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 10/34] qcow2: Add offset_to_sc_index() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 11/34] qcow2: Add offset_into_subcluster() and size_to_subclusters() Alberto Garcia
2020-07-01 12:23   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 12/34] qcow2: Add l2_entry_size() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 13/34] qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap() Alberto Garcia
2020-07-01 12:28   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 14/34] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type() Alberto Garcia
2020-07-01 12:52   ` Max Reitz
2020-07-01 16:26     ` Alberto Garcia
2020-07-02  9:57       ` Max Reitz
2020-07-02 22:00         ` Alberto Garcia
2020-07-03  7:17           ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 15/34] qcow2: Add qcow2_get_subcluster_range_type() Alberto Garcia
2020-07-01 13:37   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 16/34] qcow2: Add qcow2_cluster_is_allocated() Alberto Garcia
2020-07-01 13:55   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 17/34] qcow2: Add cluster type parameter to qcow2_get_host_offset() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 18/34] qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_* Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 19/34] qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 20/34] qcow2: Add subcluster support to calculate_l2_meta() Alberto Garcia
2020-07-02 11:30   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 21/34] qcow2: Add subcluster support to qcow2_get_host_offset() Alberto Garcia
2020-07-02 12:46   ` Max Reitz
2020-07-02 22:04     ` Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 22/34] qcow2: Add subcluster support to zero_in_l2_slice() Alberto Garcia
2020-07-02 12:56   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 23/34] qcow2: Add subcluster support to discard_in_l2_slice() Alberto Garcia
2020-07-02 13:24   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 24/34] qcow2: Add subcluster support to check_refcounts_l2() Alberto Garcia
2020-07-02 13:32   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 25/34] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2() Alberto Garcia
2020-07-02 14:01   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 26/34] qcow2: Clear the L2 bitmap when allocating a compressed cluster Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 27/34] qcow2: Add subcluster support to handle_alloc_space() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 28/34] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes() Alberto Garcia
2020-07-02 14:28   ` Max Reitz [this message]
2020-07-02 22:40     ` Alberto Garcia
2020-07-03  7:18       ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 29/34] qcow2: Add subcluster support to qcow2_measure() Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 30/34] qcow2: Add prealloc field to QCowL2Meta Alberto Garcia
2020-07-02 14:50   ` Max Reitz
2020-07-02 14:58     ` Eric Blake
2020-07-02 15:09       ` Max Reitz
2020-07-02 23:05         ` Alberto Garcia
2020-07-03  7:22           ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 31/34] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit Alberto Garcia
2020-07-02 15:13   ` Max Reitz
2020-07-03 12:43     ` Alberto Garcia
2020-06-28 11:02 ` [PATCH v9 32/34] qcow2: Allow preallocation and backing files if extended_l2 is set Alberto Garcia
2020-07-03  7:45   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 33/34] qcow2: Assert that expand_zero_clusters_in_l1() does not support subclusters Alberto Garcia
2020-07-03  7:46   ` Max Reitz
2020-06-28 11:02 ` [PATCH v9 34/34] iotests: Add tests for qcow2 images with extended L2 entries Alberto Garcia
2020-07-03  9:49   ` Max Reitz
2020-07-03 13:06     ` Alberto Garcia
2020-07-03 13:47       ` Max Reitz
2020-07-03 15:20         ` Alberto Garcia
2020-07-06 13:57       ` Eric Blake

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=25384933-0ca3-3f50-c1af-4c92e8c88328@redhat.com \
    --to=mreitz@redhat.com \
    --cc=berto@igalia.com \
    --cc=dereksu@qnap.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /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

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git