All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Denis V. Lunev" <den@openvz.org>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/1] qcow2: handle cluster leak happening with a guest TRIM command
Date: Mon, 22 May 2017 06:37:21 -0500	[thread overview]
Message-ID: <705cdc34-b45d-5f56-3fb2-f8a67c4d2b9f@redhat.com> (raw)
In-Reply-To: <1495376506-13227-1-git-send-email-den@openvz.org>

[-- Attachment #1: Type: text/plain, Size: 1754 bytes --]

On 05/21/2017 09:21 AM, Denis V. Lunev wrote:
>   qemu-img create -f qcow2 1.img 64G
>   qemu-io -c "write -P 0x32 0 64k" 1.img
> results in
>   324 -rw-r--r-- 1 den den 393216 May 21 16:48 1.img
> Subsequent
>   qemu-io -c "write -z 0 64k" 1.img
>   qemu-io -c "write -P 0x32 0 64k" 1.img
> results in
>   388 -rw-r--r-- 1 den den 458752 May 21 16:50 1.img
> which looks like we have 1 cluster leaked.
> 
> Indeed, qcow2_co_pwrite_zeroes calls qcow2_zero_clusters/zero_single_l2,
> which does not update refcount for the host cluster and keep the offset
> as used. Later on handle_copied() does not take into account
> QCOW2_CLUSTER_ZERO type of the cluster.

NACK.  We've already fixed this upstream, at commit 564a6b693.

$ ./qemu-img create -f qcow2 1.img 64g
Formatting '1.img', fmt=qcow2 size=68719476736 encryption=off
cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ ./qemu-io -c 'w -P 0x32 0 64k' 1.img
wrote 65536/65536 bytes at offset 0
64 KiB, 1 ops; 0.0410 sec (1.524 MiB/sec and 24.3902 ops/sec)
$ ls -l 1.img
-rw-r--r--. 1 eblake eblake 393216 May 22 06:35 1.img
$ ./qemu-io -c 'w -z 0 64k' 1.img
wrote 65536/65536 bytes at offset 0
64 KiB, 1 ops; 0.0055 sec (11.345 MiB/sec and 181.5211 ops/sec)
$ ./qemu-io -c 'w -P 0x32 0 64k' 1.img
wrote 65536/65536 bytes at offset 0
64 KiB, 1 ops; 0.0160 sec (3.903 MiB/sec and 62.4415 ops/sec)
$ ls -l 1.img
-rw-r--r--. 1 eblake eblake 393216 May 22 06:35 1.img


As it is, your patch doesn't apply to master.  And even if it did, your
patch breaks semantics - we CANNOT discard clusters that must read back
as zeroes.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

  reply	other threads:[~2017-05-22 11:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-21 14:21 [Qemu-devel] [PATCH 1/1] qcow2: handle cluster leak happening with a guest TRIM command Denis V. Lunev
2017-05-22 11:37 ` Eric Blake [this message]
2017-05-22 11:56   ` Denis V. Lunev
2017-05-22 13:02     ` Eric Blake
2017-05-23 14:52       ` Denis V. Lunev

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=705cdc34-b45d-5f56-3fb2-f8a67c4d2b9f@redhat.com \
    --to=eblake@redhat.com \
    --cc=den@openvz.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --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.