From nobody Wed Dec 17 05:35:15 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1504706890694829.9467405544169; Wed, 6 Sep 2017 07:08:10 -0700 (PDT) Received: from localhost ([::1]:36290 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dpazx-0000rE-Ko for importer@patchew.org; Wed, 06 Sep 2017 10:08:09 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50083) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dpavA-0005W1-Dm for qemu-devel@nongnu.org; Wed, 06 Sep 2017 10:03:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dpav4-0000rh-B0 for qemu-devel@nongnu.org; Wed, 06 Sep 2017 10:03:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38698) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dpauv-0000lR-9k; Wed, 06 Sep 2017 10:02:57 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4406AC04B924; Wed, 6 Sep 2017 14:02:56 +0000 (UTC) Received: from dhcp-200-186.str.redhat.com (dhcp-200-186.str.redhat.com [10.33.200.186]) by smtp.corp.redhat.com (Postfix) with ESMTP id 527C717A60; Wed, 6 Sep 2017 14:02:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4406AC04B924 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=kwolf@redhat.com From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 6 Sep 2017 16:02:37 +0200 Message-Id: <20170906140246.7326-6-kwolf@redhat.com> In-Reply-To: <20170906140246.7326-1-kwolf@redhat.com> References: <20170906140246.7326-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 06 Sep 2017 14:02:56 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 05/14] qcow: Change signature of get_cluster_offset() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Eric Blake The old signature has an ambiguous meaning for a return of 0: either no allocation was requested or necessary, or an error occurred (but any errno associated with the error is lost to the caller, which then has to assume EIO). Better is to follow the example of qcow2, by changing the signature to have a separate return value that cleanly distinguishes between failure and success, along with a parameter that cleanly holds a 64-bit value. Then update all callers. While auditing that all return paths return a negative errno (rather than -1), I also simplified places where we can pass NULL rather than a local Error that just gets thrown away. Suggested-by: Kevin Wolf Signed-off-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow.c | 123 +++++++++++++++++++++++++++++++++++--------------------= ---- 1 file changed, 73 insertions(+), 50 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index 63904a26ee..d07bef6306 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -347,19 +347,21 @@ static int qcow_reopen_prepare(BDRVReopenState *state, * 'compressed_size'. 'compressed_size' must be > 0 and < * cluster_size * - * return 0 if not allocated. + * return 0 if not allocated, 1 if *result is assigned, and negative + * errno on failure. */ -static uint64_t get_cluster_offset(BlockDriverState *bs, - uint64_t offset, int allocate, - int compressed_size, - int n_start, int n_end) +static int get_cluster_offset(BlockDriverState *bs, + uint64_t offset, int allocate, + int compressed_size, + int n_start, int n_end, uint64_t *result) { BDRVQcowState *s =3D bs->opaque; - int min_index, i, j, l1_index, l2_index; + int min_index, i, j, l1_index, l2_index, ret; uint64_t l2_offset, *l2_table, cluster_offset, tmp; uint32_t min_count; int new_l2_table; =20 + *result =3D 0; l1_index =3D offset >> (s->l2_bits + s->cluster_bits); l2_offset =3D s->l1_table[l1_index]; new_l2_table =3D 0; @@ -373,10 +375,12 @@ static uint64_t get_cluster_offset(BlockDriverState *= bs, /* update the L1 entry */ s->l1_table[l1_index] =3D l2_offset; tmp =3D cpu_to_be64(l2_offset); - if (bdrv_pwrite_sync(bs->file, - s->l1_table_offset + l1_index * sizeof(tmp), - &tmp, sizeof(tmp)) < 0) - return 0; + ret =3D bdrv_pwrite_sync(bs->file, + s->l1_table_offset + l1_index * sizeof(tmp), + &tmp, sizeof(tmp)); + if (ret < 0) { + return ret; + } new_l2_table =3D 1; } for(i =3D 0; i < L2_CACHE_SIZE; i++) { @@ -403,14 +407,17 @@ static uint64_t get_cluster_offset(BlockDriverState *= bs, l2_table =3D s->l2_cache + (min_index << s->l2_bits); if (new_l2_table) { memset(l2_table, 0, s->l2_size * sizeof(uint64_t)); - if (bdrv_pwrite_sync(bs->file, l2_offset, l2_table, - s->l2_size * sizeof(uint64_t)) < 0) - return 0; + ret =3D bdrv_pwrite_sync(bs->file, l2_offset, l2_table, + s->l2_size * sizeof(uint64_t)); + if (ret < 0) { + return ret; + } } else { - if (bdrv_pread(bs->file, l2_offset, l2_table, - s->l2_size * sizeof(uint64_t)) !=3D - s->l2_size * sizeof(uint64_t)) - return 0; + ret =3D bdrv_pread(bs->file, l2_offset, l2_table, + s->l2_size * sizeof(uint64_t)); + if (ret < 0) { + return ret; + } } s->l2_cache_offsets[min_index] =3D l2_offset; s->l2_cache_counts[min_index] =3D 1; @@ -427,16 +434,18 @@ static uint64_t get_cluster_offset(BlockDriverState *= bs, /* if the cluster is already compressed, we must decompress it in the case it is not completely overwritten */ - if (decompress_cluster(bs, cluster_offset) < 0) - return 0; + if (decompress_cluster(bs, cluster_offset) < 0) { + return -EIO; + } cluster_offset =3D bdrv_getlength(bs->file->bs); cluster_offset =3D (cluster_offset + s->cluster_size - 1) & ~(s->cluster_size - 1); /* write the cluster content */ - if (bdrv_pwrite(bs->file, cluster_offset, s->cluster_cache, - s->cluster_size) !=3D - s->cluster_size) - return -1; + ret =3D bdrv_pwrite(bs->file, cluster_offset, s->cluster_cache, + s->cluster_size); + if (ret < 0) { + return ret; + } } else { cluster_offset =3D bdrv_getlength(bs->file->bs); if (allocate =3D=3D 1) { @@ -459,13 +468,14 @@ static uint64_t get_cluster_offset(BlockDriverState *= bs, s->cluster_data, BDRV_SECTOR_SIZE, NULL) < 0) { - errno =3D EIO; - return -1; + return -EIO; + } + ret =3D bdrv_pwrite(bs->file, + cluster_offset + i * 512, + s->cluster_data, 512); + if (ret < 0) { + return ret; } - if (bdrv_pwrite(bs->file, - cluster_offset + i * 512, - s->cluster_data, 512) !=3D 512) - return -1; } } } @@ -477,23 +487,29 @@ static uint64_t get_cluster_offset(BlockDriverState *= bs, /* update L2 table */ tmp =3D cpu_to_be64(cluster_offset); l2_table[l2_index] =3D tmp; - if (bdrv_pwrite_sync(bs->file, l2_offset + l2_index * sizeof(tmp), - &tmp, sizeof(tmp)) < 0) - return 0; + ret =3D bdrv_pwrite_sync(bs->file, l2_offset + l2_index * sizeof(t= mp), + &tmp, sizeof(tmp)); + if (ret < 0) { + return ret; + } } - return cluster_offset; + *result =3D cluster_offset; + return 1; } =20 static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **= file) { BDRVQcowState *s =3D bs->opaque; - int index_in_cluster, n; + int index_in_cluster, n, ret; uint64_t cluster_offset; =20 qemu_co_mutex_lock(&s->lock); - cluster_offset =3D get_cluster_offset(bs, sector_num << 9, 0, 0, 0, 0); + ret =3D get_cluster_offset(bs, sector_num << 9, 0, 0, 0, 0, &cluster_o= ffset); qemu_co_mutex_unlock(&s->lock); + if (ret < 0) { + return ret; + } index_in_cluster =3D sector_num & (s->cluster_sectors - 1); n =3D s->cluster_sectors - index_in_cluster; if (n > nb_sectors) @@ -585,8 +601,11 @@ static coroutine_fn int qcow_co_readv(BlockDriverState= *bs, int64_t sector_num, =20 while (nb_sectors !=3D 0) { /* prepare next request */ - cluster_offset =3D get_cluster_offset(bs, sector_num << 9, - 0, 0, 0, 0); + ret =3D get_cluster_offset(bs, sector_num << 9, + 0, 0, 0, 0, &cluster_offset); + if (ret < 0) { + break; + } index_in_cluster =3D sector_num & (s->cluster_sectors - 1); n =3D s->cluster_sectors - index_in_cluster; if (n > nb_sectors) { @@ -603,7 +622,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState = *bs, int64_t sector_num, ret =3D bdrv_co_readv(bs->backing, sector_num, n, &hd_qiov= ); qemu_co_mutex_lock(&s->lock); if (ret < 0) { - goto fail; + break; } } else { /* Note: in this case, no need to wait */ @@ -612,13 +631,15 @@ static coroutine_fn int qcow_co_readv(BlockDriverStat= e *bs, int64_t sector_num, } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) { /* add AIO support for compressed blocks ? */ if (decompress_cluster(bs, cluster_offset) < 0) { - goto fail; + ret =3D -EIO; + break; } memcpy(buf, s->cluster_cache + index_in_cluster * 512, 512 * n); } else { if ((cluster_offset & 511) !=3D 0) { - goto fail; + ret =3D -EIO; + break; } hd_iov.iov_base =3D (void *)buf; hd_iov.iov_len =3D n * 512; @@ -635,7 +656,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState = *bs, int64_t sector_num, assert(s->crypto); if (qcrypto_block_decrypt(s->crypto, sector_num, buf, n * BDRV_SECTOR_SIZE, NULL) < 0)= { - goto fail; + ret =3D -EIO; + break; } } } @@ -646,7 +668,6 @@ static coroutine_fn int qcow_co_readv(BlockDriverState = *bs, int64_t sector_num, buf +=3D n * 512; } =20 -done: qemu_co_mutex_unlock(&s->lock); =20 if (qiov->niov > 1) { @@ -655,10 +676,6 @@ done: } =20 return ret; - -fail: - ret =3D -EIO; - goto done; } =20 static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t secto= r_num, @@ -697,9 +714,12 @@ static coroutine_fn int qcow_co_writev(BlockDriverStat= e *bs, int64_t sector_num, if (n > nb_sectors) { n =3D nb_sectors; } - cluster_offset =3D get_cluster_offset(bs, sector_num << 9, 1, 0, - index_in_cluster, - index_in_cluster + n); + ret =3D get_cluster_offset(bs, sector_num << 9, 1, 0, + index_in_cluster, + index_in_cluster + n, &cluster_offset); + if (ret < 0) { + break; + } if (!cluster_offset || (cluster_offset & 511) !=3D 0) { ret =3D -EIO; break; @@ -995,8 +1015,11 @@ qcow_co_pwritev_compressed(BlockDriverState *bs, uint= 64_t offset, goto success; } qemu_co_mutex_lock(&s->lock); - cluster_offset =3D get_cluster_offset(bs, offset, 2, out_len, 0, 0); + ret =3D get_cluster_offset(bs, offset, 2, out_len, 0, 0, &cluster_offs= et); qemu_co_mutex_unlock(&s->lock); + if (ret < 0) { + goto fail; + } if (cluster_offset =3D=3D 0) { ret =3D -EIO; goto fail; --=20 2.13.5