From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:51996)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbutsykin@virtuozzo.com>) id 1b7OAQ-0006oH-6c
	for qemu-devel@nongnu.org; Mon, 30 May 2016 10:27:43 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbutsykin@virtuozzo.com>) id 1b7OAK-0003pW-7X
	for qemu-devel@nongnu.org; Mon, 30 May 2016 10:27:41 -0400
Received: from mail-db5eur01on0137.outbound.protection.outlook.com
	([104.47.2.137]:35635
	helo=EUR01-DB5-obe.outbound.protection.outlook.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbutsykin@virtuozzo.com>) id 1b7OAJ-0003pQ-MJ
	for qemu-devel@nongnu.org; Mon, 30 May 2016 10:27:36 -0400
References: <1463229957-14253-1-git-send-email-den@openvz.org>
	<1463229957-14253-5-git-send-email-den@openvz.org>
	<20160527174539.GH27946@stefanha-x1.localdomain>
From: Pavel Butsykin <pbutsykin@virtuozzo.com>
Message-ID: <574C4DD0.2000507@virtuozzo.com>
Date: Mon, 30 May 2016 17:27:28 +0300
MIME-Version: 1.0
In-Reply-To: <20160527174539.GH27946@stefanha-x1.localdomain>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 04/10] qcow: add qcow_co_write_compressed
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>, "Denis V. Lunev" <den@openvz.org>
Cc: qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>, Jeff Cody <jcody@redhat.com>, Markus Armbruster <armbru@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, John Snow <jsnow@redhat.com>

On 27.05.2016 20:45, Stefan Hajnoczi wrote:
> On Sat, May 14, 2016 at 03:45:52PM +0300, Denis V. Lunev wrote:
>> +    qemu_co_mutex_lock(&s->lock);
>> +    cluster_offset = get_cluster_offset(bs, sector_num << 9, 2, out_len, 0, 0);
>> +    qemu_co_mutex_unlock(&s->lock);
>> +    if (cluster_offset == 0) {
>> +        ret = -EIO;
>> +        goto fail;
>> +    }
>> +    cluster_offset &= s->cluster_offset_mask;
>> +
>> +    iov = (struct iovec) {
>> +        .iov_base   = out_buf,
>> +        .iov_len    = out_len,
>> +    };
>> +    qemu_iovec_init_external(&hd_qiov, &iov, 1);
>> +    ret = bdrv_co_pwritev(bs->file->bs, cluster_offset, out_len, &hd_qiov, 0);
>
> Not sure if this has the same race condition as the qcow2 patch.  It
> seems that bdrv_getlength() is used to extend the file on a per-sector
> basis.  That would mean compressed data is not packed inside sectors and
> no read-write-modify race condition exists, but I haven't fully audited
> get_cluster_offset().
>

The get_cluster_offset() also doesn't allow to do multiple compressed
writes in a single cluster, because this function for all offsets
within the cluster returns the same cluster_offset. So here we just
can't write at offset in the cluster, only at the beginning of the
cluster.

> Stefan
>