From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46098) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4ITB-0005Be-0C for qemu-devel@nongnu.org; Fri, 17 Jan 2014 18:05:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W4IT4-0008Ri-79 for qemu-devel@nongnu.org; Fri, 17 Jan 2014 18:04:56 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:47751) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4IT3-0008R2-C0 for qemu-devel@nongnu.org; Fri, 17 Jan 2014 18:04:50 -0500 Date: Fri, 17 Jan 2014 23:39:50 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140117222653.GA7739@irqsave.net> References: <1389968119-24771-1-git-send-email-kwolf@redhat.com> <1389968119-24771-2-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1389968119-24771-2-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 01/29] block: Move initialisation of BlockLimits to bdrv_refresh_limits() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pl@kamp.de, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, xiawenc@linux.vnet.ibm.com Le Friday 17 Jan 2014 =E0 15:14:51 (+0100), Kevin Wolf a =E9crit : > This function separates filling the BlockLimits from bdrv_open(), which > allows it to call it from other operations which may change the limits > (e.g. modifications to the backing file chain or bdrv_reopen) >=20 > Signed-off-by: Kevin Wolf > Reviewed-by: Max Reitz > --- > block.c | 19 +++++++++++++++++++ > block/iscsi.c | 46 +++++++++++++++++++++++++++++----------= ------- > block/qcow2.c | 11 ++++++++++- > block/qed.c | 11 ++++++++++- > block/vmdk.c | 22 ++++++++++++++++++---- > include/block/block_int.h | 2 ++ > 6 files changed, 88 insertions(+), 23 deletions(-) >=20 > diff --git a/block.c b/block.c > index 64e7d22..99e69da 100644 > --- a/block.c > +++ b/block.c > @@ -479,6 +479,19 @@ int bdrv_create_file(const char* filename, QEMUOpt= ionParameter *options, > return ret; > } > =20 > +static int bdrv_refresh_limits(BlockDriverState *bs) > +{ > + BlockDriver *drv =3D bs->drv; > + > + memset(&bs->bl, 0, sizeof(bs->bl)); > + > + if (drv && drv->bdrv_refresh_limits) { > + return drv->bdrv_refresh_limits(bs); > + } > + > + return 0; > +} > + > /* > * Create a uniquely-named empty temporary file. > * Return 0 upon success, otherwise a negative errno value. > @@ -833,6 +846,8 @@ static int bdrv_open_common(BlockDriverState *bs, B= lockDriverState *file, > goto free_and_fail; > } > =20 > + bdrv_refresh_limits(bs); > + > #ifndef _WIN32 > if (bs->is_temporary) { > assert(bs->filename[0] !=3D '\0'); > @@ -1018,6 +1033,10 @@ int bdrv_open_backing_file(BlockDriverState *bs,= QDict *options, Error **errp) > } > pstrcpy(bs->backing_file, sizeof(bs->backing_file), > bs->backing_hd->file->filename); > + > + /* Recalculate the BlockLimits with the backing file */ > + bdrv_refresh_limits(bs); > + > return 0; > } > =20 > diff --git a/block/iscsi.c b/block/iscsi.c > index c0ea0c4..3202dc5 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1265,23 +1265,6 @@ static int iscsi_open(BlockDriverState *bs, QDic= t *options, int flags, > sizeof(struct scsi_inquiry_block_limits)); > scsi_free_scsi_task(task); > task =3D NULL; > - > - if (iscsilun->bl.max_unmap < 0xffffffff) { > - bs->bl.max_discard =3D sector_lun2qemu(iscsilun->bl.max_un= map, > - iscsilun); > - } > - bs->bl.discard_alignment =3D sector_lun2qemu(iscsilun->bl.opt_= unmap_gran, > - iscsilun); > - > - if (iscsilun->bl.max_ws_len < 0xffffffff) { > - bs->bl.max_write_zeroes =3D sector_lun2qemu(iscsilun->bl.m= ax_ws_len, > - iscsilun); > - } > - bs->bl.write_zeroes_alignment =3D sector_lun2qemu(iscsilun->bl= .opt_unmap_gran, > - iscsilun); > - > - bs->bl.opt_transfer_length =3D sector_lun2qemu(iscsilun->bl.op= t_xfer_len, > - iscsilun); > } > =20 > #if defined(LIBISCSI_FEATURE_NOP_COUNTER) > @@ -1326,6 +1309,34 @@ static void iscsi_close(BlockDriverState *bs) > memset(iscsilun, 0, sizeof(IscsiLun)); > } > =20 > +static int iscsi_refresh_limits(BlockDriverState *bs) > +{ > + IscsiLun *iscsilun =3D bs->opaque; > + > + /* We don't actually refresh here, but just return data queried in > + * iscsi_open(): iscsi targets don't change their limits. */ > + if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) { > + if (iscsilun->bl.max_unmap < 0xffffffff) { > + bs->bl.max_discard =3D sector_lun2qemu(iscsilun->bl.max_un= map, > + iscsilun); > + } > + bs->bl.discard_alignment =3D sector_lun2qemu(iscsilun->bl.opt_= unmap_gran, > + iscsilun); > + > + if (iscsilun->bl.max_ws_len < 0xffffffff) { > + bs->bl.max_write_zeroes =3D sector_lun2qemu(iscsilun->bl.m= ax_ws_len, > + iscsilun); > + } > + bs->bl.write_zeroes_alignment =3D sector_lun2qemu(iscsilun->bl= .opt_unmap_gran, > + iscsilun); > + > + bs->bl.opt_transfer_length =3D sector_lun2qemu(iscsilun->bl.op= t_xfer_len, > + iscsilun); Why is bl.opt_transfer_length filled only inside the test for unmap and w= rite same ? Is there a relationship ? > + } > + > + return 0; > +} > + > static int iscsi_truncate(BlockDriverState *bs, int64_t offset) > { > IscsiLun *iscsilun =3D bs->opaque; > @@ -1438,6 +1449,7 @@ static BlockDriver bdrv_iscsi =3D { > .bdrv_getlength =3D iscsi_getlength, > .bdrv_get_info =3D iscsi_get_info, > .bdrv_truncate =3D iscsi_truncate, > + .bdrv_refresh_limits =3D iscsi_refresh_limits, > =20 > #if defined(LIBISCSI_FEATURE_IOVECTOR) > .bdrv_co_get_block_status =3D iscsi_co_get_block_status, > diff --git a/block/qcow2.c b/block/qcow2.c > index 8ec9db1..6562994 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -718,7 +718,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *= options, int flags, > } > =20 > qemu_opts_del(opts); > - bs->bl.write_zeroes_alignment =3D s->cluster_sectors; > =20 > if (s->use_lazy_refcounts && s->qcow_version < 3) { > error_setg(errp, "Lazy refcounts require a qcow2 image with at= least " > @@ -751,6 +750,15 @@ static int qcow2_open(BlockDriverState *bs, QDict = *options, int flags, > return ret; > } > =20 > +static int qcow2_refresh_limits(BlockDriverState *bs) > +{ > + BDRVQcowState *s =3D bs->opaque; > + > + bs->bl.write_zeroes_alignment =3D s->cluster_sectors; > + > + return 0; > +} > + > static int qcow2_set_key(BlockDriverState *bs, const char *key) > { > BDRVQcowState *s =3D bs->opaque; > @@ -2268,6 +2276,7 @@ static BlockDriver bdrv_qcow2 =3D { > =20 > .bdrv_change_backing_file =3D qcow2_change_backing_file, > =20 > + .bdrv_refresh_limits =3D qcow2_refresh_limits, > .bdrv_invalidate_cache =3D qcow2_invalidate_cache, > =20 > .create_options =3D qcow2_create_options, > diff --git a/block/qed.c b/block/qed.c > index 450a1fa..d35bd1c 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -495,7 +495,6 @@ static int bdrv_qed_open(BlockDriverState *bs, QDic= t *options, int flags, > } > } > =20 > - bs->bl.write_zeroes_alignment =3D s->header.cluster_size >> BDRV_S= ECTOR_BITS; > s->need_check_timer =3D timer_new_ns(QEMU_CLOCK_VIRTUAL, > qed_need_check_timer_cb, s= ); > =20 > @@ -507,6 +506,15 @@ out: > return ret; > } > =20 > +static int bdrv_qed_refresh_limits(BlockDriverState *bs) > +{ > + BDRVQEDState *s =3D bs->opaque; > + > + bs->bl.write_zeroes_alignment =3D s->header.cluster_size >> BDRV_S= ECTOR_BITS; > + > + return 0; > +} > + > /* We have nothing to do for QED reopen, stubs just return > * success */ > static int bdrv_qed_reopen_prepare(BDRVReopenState *state, > @@ -1616,6 +1624,7 @@ static BlockDriver bdrv_qed =3D { > .bdrv_truncate =3D bdrv_qed_truncate, > .bdrv_getlength =3D bdrv_qed_getlength, > .bdrv_get_info =3D bdrv_qed_get_info, > + .bdrv_refresh_limits =3D bdrv_qed_refresh_limits, > .bdrv_change_backing_file =3D bdrv_qed_change_backing_file, > .bdrv_invalidate_cache =3D bdrv_qed_invalidate_cache, > .bdrv_check =3D bdrv_qed_check, > diff --git a/block/vmdk.c b/block/vmdk.c > index c6b60b4..e26df97 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -428,10 +428,6 @@ static int vmdk_add_extent(BlockDriverState *bs, > extent->l2_size =3D l2_size; > extent->cluster_sectors =3D flat ? sectors : cluster_sectors; > =20 > - if (!flat) { > - bs->bl.write_zeroes_alignment =3D > - MAX(bs->bl.write_zeroes_alignment, cluster_sectors); > - } > if (s->num_extents > 1) { > extent->end_sector =3D (*(extent - 1)).end_sector + extent->se= ctors; > } else { > @@ -891,6 +887,23 @@ fail: > return ret; > } > =20 > + > +static int vmdk_refresh_limits(BlockDriverState *bs) > +{ > + BDRVVmdkState *s =3D bs->opaque; > + int i; > + > + for (i =3D 0; i < s->num_extents; i++) { > + if (!s->extents[i].flat) { > + bs->bl.write_zeroes_alignment =3D > + MAX(bs->bl.write_zeroes_alignment, > + s->extents[i].cluster_sectors); > + } > + } > + > + return 0; > +} > + > static int get_whole_cluster(BlockDriverState *bs, > VmdkExtent *extent, > uint64_t cluster_offset, > @@ -2002,6 +2015,7 @@ static BlockDriver bdrv_vmdk =3D { > .bdrv_get_allocated_file_size =3D vmdk_get_allocated_file_size, > .bdrv_has_zero_init =3D vmdk_has_zero_init, > .bdrv_get_specific_info =3D vmdk_get_specific_info, > + .bdrv_refresh_limits =3D vmdk_refresh_limits, > =20 > .create_options =3D vmdk_create_options, > }; > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 2772f2f..3dfe724 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -226,6 +226,8 @@ struct BlockDriver { > int (*bdrv_debug_resume)(BlockDriverState *bs, const char *tag); > bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *= tag); > =20 > + int (*bdrv_refresh_limits)(BlockDriverState *bs); > + > /* > * Returns 1 if newly created images are guaranteed to contain onl= y > * zeros, 0 otherwise. > --=20 > 1.8.1.4 >=20 >=20