[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits() |
Date: |
Thu, 12 Dec 2013 07:17:31 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1 |
Am 11.12.2013 22:08, schrieb Kevin Wolf:
> 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)
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> block.c | 21 ++++++++++++
> block/iscsi.c | 87
> ++++++++++++++++++++++++++++-------------------
> block/qcow2.c | 11 +++++-
> block/qed.c | 11 +++++-
> block/vmdk.c | 22 +++++++++---
> include/block/block_int.h | 2 ++
> 6 files changed, 113 insertions(+), 41 deletions(-)
>
> diff --git a/block.c b/block.c
> index 13f001a..c32d856 100644
> --- a/block.c
> +++ b/block.c
> @@ -479,6 +479,21 @@ int bdrv_create_file(const char* filename,
> QEMUOptionParameter *options,
> return ret;
> }
>
> +static int bdrv_refresh_limits(BlockDriverState *bs)
> +{
> + BlockDriver *drv = bs->drv;
> +
> + memset(&bs->bl, 0, sizeof(bs->bl));
> +
> + if (!drv) {
> + return 0;
> + } else if (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 +848,8 @@ static int bdrv_open_common(BlockDriverState *bs,
> BlockDriverState *file,
> goto free_and_fail;
> }
>
> + bdrv_refresh_limits(bs);
> +
> #ifndef _WIN32
> if (bs->is_temporary) {
> assert(bs->filename[0] != '\0');
> @@ -1018,6 +1035,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;
> }
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 829d444..f3ded8c 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1425,6 +1425,56 @@ static int iscsi_open(BlockDriverState *bs, QDict
> *options, int flags,
> task = NULL;
> }
>
> +#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> + /* Set up a timer for sending out iSCSI NOPs */
> + iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> iscsi_nop_timed_event, iscsilun);
> + timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> NOP_INTERVAL);
> +#endif
> +
> +out:
> + qemu_opts_del(opts);
> + if (initiator_name != NULL) {
> + g_free(initiator_name);
> + }
> + if (iscsi_url != NULL) {
> + iscsi_destroy_url(iscsi_url);
> + }
> + if (task != NULL) {
> + scsi_free_scsi_task(task);
> + }
> +
> + if (ret) {
> + if (iscsi != NULL) {
> + iscsi_destroy_context(iscsi);
> + }
> + memset(iscsilun, 0, sizeof(IscsiLun));
> + }
> + return ret;
> +}
> +
> +static void iscsi_close(BlockDriverState *bs)
> +{
> + IscsiLun *iscsilun = bs->opaque;
> + struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> + if (iscsilun->nop_timer) {
> + timer_del(iscsilun->nop_timer);
> + timer_free(iscsilun->nop_timer);
> + }
> + qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL);
> + iscsi_destroy_context(iscsi);
> + g_free(iscsilun->zeroblock);
> + memset(iscsilun, 0, sizeof(IscsiLun));
> +}
> +
> +static int iscsi_refresh_limits(BlockDriverState *bs)
> +{
> + IscsiLun *iscsilun = bs->opaque;
> + struct scsi_task *task = NULL;
> + int ret;
> +
> + memset(&bs->bl, 0, sizeof(bs->bl));
> +
you do that memset in bdrv_refresh_limits already.
> if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) {
> struct scsi_inquiry_block_limits *inq_bl;
> task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1,
> @@ -1462,48 +1512,14 @@ static int iscsi_open(BlockDriverState *bs, QDict
> *options, int flags,
iscsi_do_inquiry does a sync call to request the pages from the target.
you can only do that here if you can guarantee that iscsi_refresh_limits will
be called with no pending requests in-flight otherwise this will end in a mess.
otherwise i would propose to leave the inquiry call ins iscsi_open and just fill
the bs->bl struc tin iscsi_refresh_limits. the problem is if we make this async
we have to handle all possible I/O callbacks whiile in iscsi_refresh_limits.
i do not see how an iscsi target can change its limits while a target is
mounted.
Peter
> iscsilun);
> }
>
> -#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> - /* Set up a timer for sending out iSCSI NOPs */
> - iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> iscsi_nop_timed_event, iscsilun);
> - timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> NOP_INTERVAL);
> -#endif
> -
> + ret = 0;
> out:
> - qemu_opts_del(opts);
> - if (initiator_name != NULL) {
> - g_free(initiator_name);
> - }
> - if (iscsi_url != NULL) {
> - iscsi_destroy_url(iscsi_url);
> - }
> if (task != NULL) {
> scsi_free_scsi_task(task);
> }
> -
> - if (ret) {
> - if (iscsi != NULL) {
> - iscsi_destroy_context(iscsi);
> - }
> - memset(iscsilun, 0, sizeof(IscsiLun));
> - }
> return ret;
> }
>
> -static void iscsi_close(BlockDriverState *bs)
> -{
> - IscsiLun *iscsilun = bs->opaque;
> - struct iscsi_context *iscsi = iscsilun->iscsi;
> -
> - if (iscsilun->nop_timer) {
> - timer_del(iscsilun->nop_timer);
> - timer_free(iscsilun->nop_timer);
> - }
> - qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL);
> - iscsi_destroy_context(iscsi);
> - g_free(iscsilun->zeroblock);
> - memset(iscsilun, 0, sizeof(IscsiLun));
> -}
> -
> static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
> {
> IscsiLun *iscsilun = bs->opaque;
> @@ -1616,6 +1632,7 @@ static BlockDriver bdrv_iscsi = {
> .bdrv_getlength = iscsi_getlength,
> .bdrv_get_info = iscsi_get_info,
> .bdrv_truncate = iscsi_truncate,
> + .bdrv_refresh_limits = iscsi_refresh_limits,
>
> #if defined(LIBISCSI_FEATURE_IOVECTOR)
> .bdrv_co_get_block_status = iscsi_co_get_block_status,
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f29aa88..f40355c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -718,7 +718,6 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
> }
>
> qemu_opts_del(opts);
> - bs->bl.write_zeroes_alignment = s->cluster_sectors;
>
> 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;
> }
>
> +static int qcow2_refresh_limits(BlockDriverState *bs)
> +{
> + BDRVQcowState *s = bs->opaque;
> +
> + bs->bl.write_zeroes_alignment = s->cluster_sectors;
> +
> + return 0;
> +}
> +
> static int qcow2_set_key(BlockDriverState *bs, const char *key)
> {
> BDRVQcowState *s = bs->opaque;
> @@ -2268,6 +2276,7 @@ static BlockDriver bdrv_qcow2 = {
>
> .bdrv_change_backing_file = qcow2_change_backing_file,
>
> + .bdrv_refresh_limits = qcow2_refresh_limits,
> .bdrv_invalidate_cache = qcow2_invalidate_cache,
>
> .create_options = 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, QDict
> *options, int flags,
> }
> }
>
> - bs->bl.write_zeroes_alignment = s->header.cluster_size >>
> BDRV_SECTOR_BITS;
> s->need_check_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> qed_need_check_timer_cb, s);
>
> @@ -507,6 +506,15 @@ out:
> return ret;
> }
>
> +static int bdrv_qed_refresh_limits(BlockDriverState *bs)
> +{
> + BDRVQEDState *s = bs->opaque;
> +
> + bs->bl.write_zeroes_alignment = s->header.cluster_size >>
> BDRV_SECTOR_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 = {
> .bdrv_truncate = bdrv_qed_truncate,
> .bdrv_getlength = bdrv_qed_getlength,
> .bdrv_get_info = bdrv_qed_get_info,
> + .bdrv_refresh_limits = bdrv_qed_refresh_limits,
> .bdrv_change_backing_file = bdrv_qed_change_backing_file,
> .bdrv_invalidate_cache = bdrv_qed_invalidate_cache,
> .bdrv_check = bdrv_qed_check,
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 0734bc2..f8a387f 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -428,10 +428,6 @@ static int vmdk_add_extent(BlockDriverState *bs,
> extent->l2_size = l2_size;
> extent->cluster_sectors = flat ? sectors : cluster_sectors;
>
> - if (!flat) {
> - bs->bl.write_zeroes_alignment =
> - MAX(bs->bl.write_zeroes_alignment, cluster_sectors);
> - }
> if (s->num_extents > 1) {
> extent->end_sector = (*(extent - 1)).end_sector + extent->sectors;
> } else {
> @@ -886,6 +882,23 @@ fail:
> return ret;
> }
>
> +
> +static int vmdk_refresh_limits(BlockDriverState *bs)
> +{
> + BDRVVmdkState *s = bs->opaque;
> + int i;
> +
> + for (i = 0; i < s->num_extents; i++) {
> + if (!s->extents[i].flat) {
> + bs->bl.write_zeroes_alignment =
> + 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,
> @@ -1971,6 +1984,7 @@ static BlockDriver bdrv_vmdk = {
> .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
> .bdrv_has_zero_init = vmdk_has_zero_init,
> .bdrv_get_specific_info = vmdk_get_specific_info,
> + .bdrv_refresh_limits = vmdk_refresh_limits,
>
> .create_options = vmdk_create_options,
> };
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 8b132d7..c49fa6b 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);
>
> + int (*bdrv_refresh_limits)(BlockDriverState *bs);
> +
> /*
> * Returns 1 if newly created images are guaranteed to contain only
> * zeros, 0 otherwise.
- Re: [Qemu-devel] [PATCH 05/22] block: Detect unaligned length in bdrv_qiov_is_aligned(), (continued)
- Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits(),
Peter Lieven <=
[Qemu-devel] [PATCH 03/22] block: Update BlockLimits when they might have changed, Kevin Wolf, 2013/12/11
[Qemu-devel] [PATCH 08/22] raw: Probe required direct I/O alignment, Kevin Wolf, 2013/12/11
[Qemu-devel] [PATCH 09/22] block: Introduce bdrv_aligned_preadv(), Kevin Wolf, 2013/12/11
[Qemu-devel] [PATCH 07/22] block: rename buffer_alignment to guest_block_size, Kevin Wolf, 2013/12/11
[Qemu-devel] [PATCH 10/22] block: Introduce bdrv_co_do_preadv(), Kevin Wolf, 2013/12/11
[Qemu-devel] [PATCH 11/22] block: Introduce bdrv_aligned_pwritev(), Kevin Wolf, 2013/12/11
[Qemu-devel] [PATCH 12/22] block: write: Handle COR dependency after I/O throttling, Kevin Wolf, 2013/12/11