From nobody Wed Nov 5 14:39:37 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=gmail.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1534885433900140.13901295023447; Tue, 21 Aug 2018 14:03:53 -0700 (PDT) Received: from localhost ([::1]:55872 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fsDoe-0004x4-VV for importer@patchew.org; Tue, 21 Aug 2018 17:03:52 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45613) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fsDli-0003Ce-4V for qemu-devel@nongnu.org; Tue, 21 Aug 2018 17:00:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fsDlf-0005jZ-1N for qemu-devel@nongnu.org; Tue, 21 Aug 2018 17:00:50 -0400 Received: from mail-qt0-x241.google.com ([2607:f8b0:400d:c0d::241]:36575) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fsDle-0005j3-QD for qemu-devel@nongnu.org; Tue, 21 Aug 2018 17:00:46 -0400 Received: by mail-qt0-x241.google.com with SMTP id t5-v6so21949456qtn.3 for ; Tue, 21 Aug 2018 14:00:46 -0700 (PDT) Received: from salty.br.ibm.com ([32.104.18.202]) by smtp.gmail.com with ESMTPSA id e29-v6sm10139383qte.47.2018.08.21.14.00.44 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 21 Aug 2018 14:00:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=bT2r8V74033xk+DGVr172Gu8HP1ZinVaO+w5cMZosb0=; b=iSIZfOjAq4+aSI57/9dmomd24ltIGfidlOEJmKUJlDmgh8rUpsOwrO8jLJvyM4jpW/ nIh52xKsrvocSPg5ujztQ+KvIBKU3g8faBc9i37oIV7HROhTZhk1vi58Y+/XZ9OZE1q+ SO2WAtbrqHoXPSS5B7bdES1Nzzbz9YhJWna/pmMElOGVh8P069lRm1YWEv5oFsSTy0iT t4l+bXwBp2ZaSXrCg5bC0a7UWPYLHaf9HLRq1u0EI4gCztWTqYDODH3dutwK5ltGnoQb vRr7nTfUH5YTBACYGMx/iF/Z9L2TyufP72KkSfS8ixa+FGB0IuJE5De4Dz60TlQQuib8 F8GQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=bT2r8V74033xk+DGVr172Gu8HP1ZinVaO+w5cMZosb0=; b=Q1Bs8Z1kebUyVl2LNwGfrtfkaA/598XARhcUrXK+f5C/DxuFuIZRGsq57b7eprpTdi jjbzCToRf3oAHAjfKMFZQk7aAqhq+QBOOVwACuCa1q3KulSj/9BuWvEtiSsG6bnMNkHR CVBJY3P6/HkvhoOlwMg3Ue67A6JVd0p3Psd65WO79TZU+epM67ML1u5MBP7M9FjtY4HY JG96/IB28DA7XU8wBRyS4HsT5NSqYbncQskEEc9yeXBRCJHELPu4MySkLnJZVOnW4fxl M65bRYNCaCMRX48NaRbfVc/fubm4hHg1+tD1Q+zOD+KXEBm/h0lhkeU4mrZNhKGa3ztK /vLg== X-Gm-Message-State: AOUpUlFs8Gn8xB2+sbcvXTSTdtgcpPMqnC+Qb4vIRVP3X4hv07acKwBQ QSxjoJQofHMg2SHPNrZKKn5K5Pi5tt4= X-Google-Smtp-Source: AA+uWPxtZRj4m94PuhfBWjVQ/um3lYXVgD9OzyE9c55Ac84IqMYj4L/ps4E4O6+arg7qDS4uiNFI7w== X-Received: by 2002:a0c:fa10:: with SMTP id q16-v6mr48258070qvn.46.1534885246113; Tue, 21 Aug 2018 14:00:46 -0700 (PDT) From: Daniel Henrique Barboza To: qemu-devel@nongnu.org Date: Tue, 21 Aug 2018 18:00:24 -0300 Message-Id: <20180821210024.3587-3-danielhb413@gmail.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180821210024.3587-1-danielhb413@gmail.com> References: <20180821210024.3587-1-danielhb413@gmail.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:400d:c0d::241 Subject: [Qemu-devel] [PATCH v1 2/2] 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, armbru@redhat.com, Daniel Henrique Barboza , dgilbert@redhat.com, mreitz@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDMRC_1 RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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) >=3D 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 =3D 0; i < s->nb_snapshots; i++) { sn =3D s->snapshots + i; id =3D strtoul(sn->id_str, NULL, 10); if (id > id_max) id_max =3D 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 =3D sn_info->id_str and name =3D NULL. This will cause the function to execute the following: } else if (id) { for (i =3D 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 --- 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, QEMUSn= apshotInfo *sn_info) /* Generate an ID */ find_new_snapshot_id(bs, sn_info->id_str, sizeof(sn_info->id_str)); =20 - /* Check that the ID is unique */ - if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >=3D 0) { - return -EEXIST; - } - /* Populate sn with passed data */ sn->id_str =3D g_strdup(sn_info->id_str); sn->name =3D g_strdup(sn_info->name); --=20 2.17.1