From patchwork Tue Mar 17 04:38:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Snow X-Patchwork-Id: 1256039 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=aLWx26NI; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48hL6r3lLSz9sRR for ; Tue, 17 Mar 2020 15:39:32 +1100 (AEDT) Received: from localhost ([::1]:52544 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jE40o-0001Lx-CW for incoming@patchwork.ozlabs.org; Tue, 17 Mar 2020 00:39:30 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52113) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jE40H-00014G-1m for qemu-devel@nongnu.org; Tue, 17 Mar 2020 00:38:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jE40F-0000IK-9p for qemu-devel@nongnu.org; Tue, 17 Mar 2020 00:38:56 -0400 Received: from us-smtp-delivery-74.mimecast.com ([216.205.24.74]:32304) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jE40F-0000HR-4j for qemu-devel@nongnu.org; Tue, 17 Mar 2020 00:38:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1584419934; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=W2SZloJALWARRXQrRWIcseEswJ8GCx7hIkZZF9kViZ0=; b=aLWx26NIGvBDR71V0Zg2OaL+Awh4WKEapncDjxuDeK/p8eB45XiHsRKUG3+8mI4We/Zcd4 dH74b/lMJNzbXETCSHOoEpGRM+J1V8WRyr3APewM2NQ+HLi8DMGmPdLpHzy9281wUVBiit HFL/h4+zqSyLB56fHBheVC0x2AK8Dbo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-443-gExgZIdJOzKyrnHC-kvfEQ-1; Tue, 17 Mar 2020 00:38:52 -0400 X-MC-Unique: gExgZIdJOzKyrnHC-kvfEQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3FDBB1857BE1; Tue, 17 Mar 2020 04:38:51 +0000 (UTC) Received: from probe.redhat.com (ovpn-112-191.rdu2.redhat.com [10.10.112.191]) by smtp.corp.redhat.com (Postfix) with ESMTP id C8C851036B3B; Tue, 17 Mar 2020 04:38:48 +0000 (UTC) From: John Snow To: qemu-devel@nongnu.org, Peter Maydell , jsnow@redhat.com Subject: [PULL 08/10] nbd/server: introduce NBDExtentArray Date: Tue, 17 Mar 2020 00:38:17 -0400 Message-Id: <20200317043819.20197-9-jsnow@redhat.com> In-Reply-To: <20200317043819.20197-1-jsnow@redhat.com> References: <20200317043819.20197-1-jsnow@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 216.205.24.74 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, Juan Quintela , libvir-list@redhat.com, Markus Armbruster , "Dr. David Alan Gilbert" , Stefan Hajnoczi , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Vladimir Sementsov-Ogievskiy Introduce NBDExtentArray class, to handle extents list creation in more controlled way and with fewer OUT parameters in functions. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20200205112041.6003-9-vsementsov@virtuozzo.com Signed-off-by: John Snow --- nbd/server.c | 210 +++++++++++++++++++++++++++++---------------------- 1 file changed, 118 insertions(+), 92 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 3106aaf3b4..f90bb33a75 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1909,27 +1909,98 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client, return ret; } +typedef struct NBDExtentArray { + NBDExtent *extents; + unsigned int nb_alloc; + unsigned int count; + uint64_t total_length; + bool can_add; + bool converted_to_be; +} NBDExtentArray; + +static NBDExtentArray *nbd_extent_array_new(unsigned int nb_alloc) +{ + NBDExtentArray *ea = g_new0(NBDExtentArray, 1); + + ea->nb_alloc = nb_alloc; + ea->extents = g_new(NBDExtent, nb_alloc); + ea->can_add = true; + + return ea; +} + +static void nbd_extent_array_free(NBDExtentArray *ea) +{ + g_free(ea->extents); + g_free(ea); +} +G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free); + +/* Further modifications of the array after conversion are abandoned */ +static void nbd_extent_array_convert_to_be(NBDExtentArray *ea) +{ + int i; + + assert(!ea->converted_to_be); + ea->can_add = false; + ea->converted_to_be = true; + + for (i = 0; i < ea->count; i++) { + ea->extents[i].flags = cpu_to_be32(ea->extents[i].flags); + ea->extents[i].length = cpu_to_be32(ea->extents[i].length); + } +} + /* - * Populate @extents from block status. Update @bytes to be the actual - * length encoded (which may be smaller than the original), and update - * @nb_extents to the number of extents used. - * - * Returns zero on success and -errno on bdrv_block_status_above failure. + * Add extent to NBDExtentArray. If extent can't be added (no available space), + * return -1. + * For safety, when returning -1 for the first time, .can_add is set to false, + * further call to nbd_extent_array_add() will crash. + * (to avoid the situation, when after failing to add an extent (returned -1), + * user miss this failure and add another extent, which is successfully added + * (array is full, but new extent may be squashed into the last one), then we + * have invalid array with skipped extent) */ +static int nbd_extent_array_add(NBDExtentArray *ea, + uint32_t length, uint32_t flags) +{ + assert(ea->can_add); + + if (!length) { + return 0; + } + + /* Extend previous extent if flags are the same */ + if (ea->count > 0 && flags == ea->extents[ea->count - 1].flags) { + uint64_t sum = (uint64_t)length + ea->extents[ea->count - 1].length; + + if (sum <= UINT32_MAX) { + ea->extents[ea->count - 1].length = sum; + ea->total_length += length; + return 0; + } + } + + if (ea->count >= ea->nb_alloc) { + ea->can_add = false; + return -1; + } + + ea->total_length += length; + ea->extents[ea->count] = (NBDExtent) {.length = length, .flags = flags}; + ea->count++; + + return 0; +} + static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset, - uint64_t *bytes, NBDExtent *extents, - unsigned int *nb_extents) + uint64_t bytes, NBDExtentArray *ea) { - uint64_t remaining_bytes = *bytes; - NBDExtent *extent = extents, *extents_end = extents + *nb_extents; - bool first_extent = true; - - assert(*nb_extents); - while (remaining_bytes) { + while (bytes) { uint32_t flags; int64_t num; - int ret = bdrv_block_status_above(bs, NULL, offset, remaining_bytes, - &num, NULL, NULL); + int ret = bdrv_block_status_above(bs, NULL, offset, bytes, &num, + NULL, NULL); if (ret < 0) { return ret; @@ -1938,60 +2009,37 @@ static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset, flags = (ret & BDRV_BLOCK_ALLOCATED ? 0 : NBD_STATE_HOLE) | (ret & BDRV_BLOCK_ZERO ? NBD_STATE_ZERO : 0); - if (first_extent) { - extent->flags = flags; - extent->length = num; - first_extent = false; - } else if (flags == extent->flags) { - /* extend current extent */ - extent->length += num; - } else { - if (extent + 1 == extents_end) { - break; - } - - /* start new extent */ - extent++; - extent->flags = flags; - extent->length = num; + if (nbd_extent_array_add(ea, num, flags) < 0) { + return 0; } + offset += num; - remaining_bytes -= num; + bytes -= num; } - extents_end = extent + 1; - - for (extent = extents; extent < extents_end; extent++) { - extent->flags = cpu_to_be32(extent->flags); - extent->length = cpu_to_be32(extent->length); - } - - *bytes -= remaining_bytes; - *nb_extents = extents_end - extents; - return 0; } -/* nbd_co_send_extents +/* + * nbd_co_send_extents * - * @length is only for tracing purposes (and may be smaller or larger - * than the client's original request). @last controls whether - * NBD_REPLY_FLAG_DONE is sent. @extents should already be in - * big-endian format. + * @ea is converted to BE by the function + * @last controls whether NBD_REPLY_FLAG_DONE is sent. */ static int nbd_co_send_extents(NBDClient *client, uint64_t handle, - NBDExtent *extents, unsigned int nb_extents, - uint64_t length, bool last, - uint32_t context_id, Error **errp) + NBDExtentArray *ea, + bool last, uint32_t context_id, Error **errp) { NBDStructuredMeta chunk; - struct iovec iov[] = { {.iov_base = &chunk, .iov_len = sizeof(chunk)}, - {.iov_base = extents, .iov_len = nb_extents * sizeof(extents[0])} + {.iov_base = ea->extents, .iov_len = ea->count * sizeof(ea->extents[0])} }; - trace_nbd_co_send_extents(handle, nb_extents, context_id, length, last); + nbd_extent_array_convert_to_be(ea); + + trace_nbd_co_send_extents(handle, ea->count, context_id, ea->total_length, + last); set_be_chunk(&chunk.h, last ? NBD_REPLY_FLAG_DONE : 0, NBD_REPLY_TYPE_BLOCK_STATUS, handle, sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len); @@ -2009,39 +2057,27 @@ static int nbd_co_send_block_status(NBDClient *client, uint64_t handle, { int ret; unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS; - NBDExtent *extents = g_new(NBDExtent, nb_extents); - uint64_t final_length = length; + g_autoptr(NBDExtentArray) ea = nbd_extent_array_new(nb_extents); - ret = blockstatus_to_extents(bs, offset, &final_length, extents, - &nb_extents); + ret = blockstatus_to_extents(bs, offset, length, ea); if (ret < 0) { - g_free(extents); return nbd_co_send_structured_error( client, handle, -ret, "can't get block status", errp); } - ret = nbd_co_send_extents(client, handle, extents, nb_extents, - final_length, last, context_id, errp); - - g_free(extents); - - return ret; + return nbd_co_send_extents(client, handle, ea, last, context_id, errp); } /* - * Populate @extents from a dirty bitmap. Unless @dont_fragment, the - * final extent may exceed the original @length. Store in @length the - * byte length encoded (which may be smaller or larger than the - * original), and return the number of extents used. + * Populate @ea from a dirty bitmap. Unless @dont_fragment, the + * final extent may exceed the original @length. */ -static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset, - uint64_t *length, NBDExtent *extents, - unsigned int nb_extents, - bool dont_fragment) +static void bitmap_to_extents(BdrvDirtyBitmap *bitmap, + uint64_t offset, uint64_t length, + NBDExtentArray *ea, bool dont_fragment) { uint64_t begin = offset, end = offset; - uint64_t overall_end = offset + *length; - unsigned int i = 0; + uint64_t overall_end = offset + length; BdrvDirtyBitmapIter *it; bool dirty; @@ -2050,8 +2086,7 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset, it = bdrv_dirty_iter_new(bitmap); dirty = bdrv_dirty_bitmap_get_locked(bitmap, offset); - assert(begin < overall_end && nb_extents); - while (begin < overall_end && i < nb_extents) { + while (begin < overall_end) { bool next_dirty = !dirty; if (dirty) { @@ -2071,9 +2106,10 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset, end = overall_end; } - extents[i].length = cpu_to_be32(end - begin); - extents[i].flags = cpu_to_be32(dirty ? NBD_STATE_DIRTY : 0); - i++; + if (nbd_extent_array_add(ea, end - begin, + dirty ? NBD_STATE_DIRTY : 0) < 0) { + break; + } begin = end; dirty = next_dirty; } @@ -2083,8 +2119,6 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset, bdrv_dirty_bitmap_unlock(bitmap); assert(offset < end); - *length = end - offset; - return i; } static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle, @@ -2092,20 +2126,12 @@ static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle, uint32_t length, bool dont_fragment, bool last, uint32_t context_id, Error **errp) { - int ret; unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS; - NBDExtent *extents = g_new(NBDExtent, nb_extents); - uint64_t final_length = length; + g_autoptr(NBDExtentArray) ea = nbd_extent_array_new(nb_extents); - nb_extents = bitmap_to_extents(bitmap, offset, &final_length, extents, - nb_extents, dont_fragment); + bitmap_to_extents(bitmap, offset, length, ea, dont_fragment); - ret = nbd_co_send_extents(client, handle, extents, nb_extents, - final_length, last, context_id, errp); - - g_free(extents); - - return ret; + return nbd_co_send_extents(client, handle, ea, last, context_id, errp); } /* nbd_co_receive_request