From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:48917)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <xiecl.fnst@cn.fujitsu.com>) id 1aqYBz-0006Dd-4I
	for qemu-devel@nongnu.org; Wed, 13 Apr 2016 23:43:43 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <xiecl.fnst@cn.fujitsu.com>) id 1aqYBy-0006JF-Bx
	for qemu-devel@nongnu.org; Wed, 13 Apr 2016 23:43:43 -0400
Message-ID: <570F1294.3090601@cn.fujitsu.com>
Date: Thu, 14 Apr 2016 11:46:28 +0800
From: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
MIME-Version: 1.0
References: <1460362979-15146-1-git-send-email-xiecl.fnst@cn.fujitsu.com>
	<1460362979-15146-8-git-send-email-xiecl.fnst@cn.fujitsu.com>
	<20160413130225.GD9272@stefanha-x1.localdomain>
In-Reply-To: <20160413130225.GD9272@stefanha-x1.localdomain>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v17 7/8] Implement new driver for block
 replication
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@redhat.com>
Cc: qemu devel <qemu-devel@nongnu.org>, Fam Zheng <famz@redhat.com>, Max Reitz <mreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Dong Eddie <eddie.dong@intel.com>, Jiang Yunhong <yunhong.jiang@intel.com>, Wen Congyang <wency@cn.fujitsu.com>, qemu block <qemu-block@nongnu.org>, "Michael R. Hines" <mrhines@linux.vnet.ibm.com>, Eric Blake <eblake@redhat.com>, zhanghailiang <zhang.zhanghailiang@huawei.com>, Gonglei <arei.gonglei@huawei.com>

On 04/13/2016 09:02 PM, Stefan Hajnoczi wrote:
> On Mon, Apr 11, 2016 at 04:22:58PM +0800, Changlong Xie wrote:
>> +static coroutine_fn int replication_co_writev(BlockDriverState *bs,
>> +                                              int64_t sector_num,
>> +                                              int remaining_sectors,
>> +                                              QEMUIOVector *qiov)
>> +{
>> +    BDRVReplicationState *s = bs->opaque;
>> +    QEMUIOVector hd_qiov;
>> +    uint64_t bytes_done = 0;
>> +    BdrvChild *top = bs->file;
>> +    BdrvChild *base = s->secondary_disk;
>> +    BlockDriverState *target;
>> +    int ret, n;
>> +
>> +    ret = replication_get_io_status(s);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    if (ret == 0) {
>> +        ret = bdrv_co_writev(top->bs, sector_num,
>> +                             remaining_sectors, qiov);
>> +        return replication_return_value(s, ret);
>> +    }
>> +
>> +    /*
>> +     * Failover failed, only write to active disk if the sectors
>> +     * have already been allocated in active disk/hidden disk.
>> +     */
>> +    qemu_iovec_init(&hd_qiov, qiov->niov);
>> +    while (remaining_sectors > 0) {
>> +        ret = bdrv_is_allocated_above(top->bs, base->bs, sector_num,
>> +                                      remaining_sectors, &n);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +
>> +        qemu_iovec_reset(&hd_qiov);
>> +        qemu_iovec_concat(&hd_qiov, qiov, bytes_done, n * BDRV_SECTOR_SIZE);
>> +
>> +        target = ret ? (top->bs) : (base->bs);
>> +        ret = bdrv_co_writev(target, sector_num, n, &hd_qiov);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +
>> +        remaining_sectors -= n;
>> +        sector_num += n;
>> +        bytes_done += n * BDRV_SECTOR_SIZE;
>> +    }
>> +
>> +    return 0;
>
> qemu_iovec_destroy(&hd_qiov) is missing (also in error cases).

Yes, it's memory leak here.

Thanks
	-Xie

>