From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53168) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8vSy-0000zf-LC for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:13:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8vSw-0003Gu-AX for qemu-devel@nongnu.org; Fri, 03 Jun 2016 16:13:11 -0400 References: <1464974478-23598-1-git-send-email-kwolf@redhat.com> <1464974478-23598-6-git-send-email-kwolf@redhat.com> From: Eric Blake Message-ID: <5751E4CE.10806@redhat.com> Date: Fri, 3 Jun 2016 14:13:02 -0600 MIME-Version: 1.0 In-Reply-To: <1464974478-23598-6-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="VGRD1ubpvxks5cGAdoCIMr2O90FW0EPKT" Subject: Re: [Qemu-devel] [PATCH 5/5] qcow2: Implement .bdrv_co_pwritev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: jsnow@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --VGRD1ubpvxks5cGAdoCIMr2O90FW0EPKT Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/03/2016 11:21 AM, Kevin Wolf wrote: > This changes qcow2 to implement the byte-based .bdrv_co_pwritev > interface rather than the sector-based old one. >=20 > As preallocation uses the same allocation function as normal writes, an= d > the interface of that function needs to be changed, it is converted in > the same patch. >=20 > Signed-off-by: Kevin Wolf > --- > block/qcow2-cluster.c | 13 ++++---- > block/qcow2.c | 82 ++++++++++++++++++++++++-------------------= -------- > block/qcow2.h | 3 +- > trace-events | 6 ++-- > 4 files changed, 49 insertions(+), 55 deletions(-) >=20 > +++ b/block/qcow2-cluster.c > @@ -1261,7 +1261,8 @@ fail: > * Return 0 on success and -errno in error cases > */ > int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset, > - int *num, uint64_t *host_offset, QCowL2Meta **m) > + unsigned int *bytes, uint64_t *host_off= set, > + QCowL2Meta **m) > { > =20 > - *num -=3D remaining >> BDRV_SECTOR_BITS; > - assert(*num > 0); > + *bytes -=3D remaining; > + assert(*bytes > 0); The assertion is now weaker. Beforehand, num could go negative. But bytes cannot, all it can do is go to 0. If we wrap around, the assertion won't catch it. Do you want to strengthen it by also asserting that bytes < INT_MAX? > +++ b/block/qcow2.c > @@ -1544,23 +1544,20 @@ fail: > return ret; > } > =20 > -static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, > - int64_t sector_num, > - int remaining_sectors, > - QEMUIOVector *qiov) > +static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, > + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags= ) Worth consistent indentation, while touching it? > + while (bytes !=3D 0) { > =20 > l2meta =3D NULL; > =20 > trace_qcow2_writev_start_part(qemu_coroutine_self()); > - index_in_cluster =3D sector_num & (s->cluster_sectors - 1); > - cur_nr_sectors =3D remaining_sectors; > - if (bs->encrypted && > - cur_nr_sectors > > - QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors - index_in_cl= uster) { > - cur_nr_sectors =3D > - QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors - index_i= n_cluster; > + offset_in_cluster =3D offset_into_cluster(s, offset); > + cur_bytes =3D MIN(bytes, INT_MAX); > + if (bs->encrypted) { > + cur_bytes =3D MIN(cur_bytes, > + QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size > + - offset_in_cluster); Again, if the block layer learns to auto-fragment, then we should just inform the block layer about QCOW_MAX_CRYPT_CLUSTERS as our max_transfer limit. > @@ -1634,10 +1628,10 @@ static coroutine_fn int qcow2_co_writev(BlockDr= iverState *bs, > qemu_co_mutex_unlock(&s->lock); > BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO); > trace_qcow2_writev_data(qemu_coroutine_self(), > - (cluster_offset >> 9) + index_in_clust= er); > - ret =3D bdrv_co_writev(bs->file->bs, > - (cluster_offset >> 9) + index_in_cluster,= > - cur_nr_sectors, &hd_qiov); > + cluster_offset + offset_in_cluster); > + ret =3D bdrv_co_pwritev(bs->file->bs, > + cluster_offset + offset_in_cluster, > + cur_bytes, &hd_qiov, 0); s/0/flags/, even if .supported_write_flags stays 0 for now. > @@ -2008,19 +2002,19 @@ static int qcow2_change_backing_file(BlockDrive= rState *bs, > =20 > static int preallocate(BlockDriverState *bs) > { > - uint64_t nb_sectors; > + uint64_t bytes; > uint64_t offset; > uint64_t host_offset =3D 0; > - int num; > + unsigned int cur_bytes; > int ret; > QCowL2Meta *meta; > =20 > - nb_sectors =3D bdrv_nb_sectors(bs); > + bytes =3D bdrv_getlength(bs); > offset =3D 0; > =20 > - while (nb_sectors) { > - num =3D MIN(nb_sectors, INT_MAX >> BDRV_SECTOR_BITS); > - ret =3D qcow2_alloc_cluster_offset(bs, offset, &num, > + while (bytes) { > + cur_bytes =3D MIN(bytes, INT_MAX); Okay, while bdrv_co_pwritev() should (eventually) let the block layer auto-fragment, here, you are right that you have to fragment yourself since this is not being called from the block layer. > @@ -2059,7 +2053,7 @@ static int preallocate(BlockDriverState *bs) > uint8_t buf[BDRV_SECTOR_SIZE]; > memset(buf, 0, BDRV_SECTOR_SIZE); > ret =3D bdrv_write(bs->file->bs, > - (host_offset >> BDRV_SECTOR_BITS) + num - 1, > + ((host_offset + cur_bytes) >> BDRV_SECTOR_BIT= S) - 1, > buf, 1); Do we still have to call bdrv_write(), or can we convert this to a byte-based interface call? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --VGRD1ubpvxks5cGAdoCIMr2O90FW0EPKT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXUeTOAAoJEKeha0olJ0Nq6w4H/3XwXFY8pNYhe1WNl4CCcwWN wl6olC0RDSuXfOGUnD7S6wqlQJlHkvNnRSCRG4iB7QoAG2DsMqnj+7bF+eab+XaL dY2vfuxOfjeyrsEKVNbvlaLWNDlz809yCEL7EL/ioSXzgj3qHXbkUmt1igazCypu cbrnHAC+j2mcm7HnqLN7dpM4DFaxn9dMbNCtRJ67dTDWNhFSxSyKq3fyK/6pGRQc e7XpdqlSlAaKppdp1jcaA4cnLMJjhvLz7t42/G1GPFTlDqJji9FyJ0ZD3sivNpKV 7c98aQhmSiTja0hG3TbjzLs4d09WGQSKzllAN1hZp6Tk+M2oh5zdCNL1VSTG6I4= =BXYD -----END PGP SIGNATURE----- --VGRD1ubpvxks5cGAdoCIMr2O90FW0EPKT--