From patchwork Mon Feb 25 15:19:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 10828725 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9990F1515 for ; Mon, 25 Feb 2019 15:34:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8522127F4B for ; Mon, 25 Feb 2019 15:34:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 791BC280FC; Mon, 25 Feb 2019 15:34:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 1BBFF27F4B for ; Mon, 25 Feb 2019 15:34:38 +0000 (UTC) Received: from localhost ([127.0.0.1]:39199 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gyIH7-0002gu-4n for patchwork-qemu-devel@patchwork.kernel.org; Mon, 25 Feb 2019 10:34:37 -0500 Received: from eggs.gnu.org ([209.51.188.92]:36956) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gyI4C-00089r-5F for qemu-devel@nongnu.org; Mon, 25 Feb 2019 10:21:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gyI4B-0000ar-9M for qemu-devel@nongnu.org; Mon, 25 Feb 2019 10:21:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51811) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gyI48-0000Xq-Jk; Mon, 25 Feb 2019 10:21:12 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CC520C01D33A; Mon, 25 Feb 2019 15:21:11 +0000 (UTC) Received: from linux.fritz.box.com (ovpn-117-243.ams2.redhat.com [10.36.117.243]) by smtp.corp.redhat.com (Postfix) with ESMTP id B9E665C22B; Mon, 25 Feb 2019 15:21:10 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Mon, 25 Feb 2019 16:19:47 +0100 Message-Id: <20190225152053.15976-6-kwolf@redhat.com> In-Reply-To: <20190225152053.15976-1-kwolf@redhat.com> References: <20190225152053.15976-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 25 Feb 2019 15:21:11 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 05/71] qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call 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, peter.maydell@linaro.org, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Daniel Henrique Barboza In qcow2_snapshot_create there is the following code block: /* Generate an ID */ find_new_snapshot_id(bs, sn_info->id_str, sizeof(sn_info->id_str)); /* Check that the ID is unique */ if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) { return -EEXIST; } find_new_snapshot_id cycles through all snapshots, getting the id_str as an unsigned long int, calculating the max id_max value of all the existing id_strs and writing in the id_str pointer id_max + 1: for(i = 0; i < s->nb_snapshots; i++) { sn = s->snapshots + i; id = strtoul(sn->id_str, NULL, 10); if (id > id_max) id_max = id; } snprintf(id_str, id_str_size, "%lu", id_max + 1); Here, sn_info->id_str will have the unique value id_max + 1. Right after that, find_snapshot_by_id_and_name is called with id = sn_info->id_str and name = NULL. This will cause the function to execute the following: } else if (id) { for (i = 0; i < s->nb_snapshots; i++) { if (!strcmp(s->snapshots[i].id_str, id)) { return i; } } } In short, we're searching the existing snapshots to see if sn_info->id_str matches any existing id, right after we set in the previous line a sn_info->id_str value that is already unique. The first code block goes way back to commit 585f8587ad, a 2006 commit from Fabrice Bellard that simply says "new qcow2 disk image format". No more info is provided about this logic in any subsequent commits that moved this code block around. I can't say about the original design, but the current logic is redundant. bdrv_snapshot_create is called in aio_context lock, forbidding any concurrent call to accidentally create a new snapshot between the find_new_snapshot_id and find_snapshot_by_id_and_name calls. What we're ending up doing is to cycle through the snapshots two times for no viable reason. This patch eliminates the redundancy by removing the 'id is unique' check that calls find_snapshot_by_id_and_name. Signed-off-by: Daniel Henrique Barboza Signed-off-by: Kevin Wolf --- block/qcow2-snapshot.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index bb6a5b7516..20e8472191 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -358,11 +358,6 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) /* Generate an ID */ find_new_snapshot_id(bs, sn_info->id_str, sizeof(sn_info->id_str)); - /* Check that the ID is unique */ - if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) { - return -EEXIST; - } - /* Populate sn with passed data */ sn->id_str = g_strdup(sn_info->id_str); sn->name = g_strdup(sn_info->name);