[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PULL 05/71] qcow2-snapshot: remove redundant find_snapshot
From: |
Kevin Wolf |
Subject: |
[Qemu-block] [PULL 05/71] qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call |
Date: |
Mon, 25 Feb 2019 16:19:47 +0100 |
From: Daniel Henrique Barboza <address@hidden>
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 <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
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);
--
2.20.1
- [Qemu-block] [PULL 00/71] Block layer patches, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 01/71] MAINTAINERS: Replace myself with John Snow for block jobs, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 02/71] MAINTAINERS: Remove myself as block maintainer, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 03/71] block/snapshot.c: eliminate use of ID input in snapshot operations, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 04/71] block/snapshot: remove bdrv_snapshot_delete_by_id_or_name, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 05/71] qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call,
Kevin Wolf <=
- [Qemu-block] [PULL 07/71] commit: Replace commit_top_bs on failure after deleting the block job, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 06/71] block: don't set the same context, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 08/71] qemu-img: fix error reporting for -object, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 09/71] block-backend: Make blk_inc/dec_in_flight public, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 10/71] virtio-blk: Increase in_flight for request restart BH, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 11/71] nbd: Restrict connection_co reentrance, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 12/71] io: Make qio_channel_yield() interruptible, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 13/71] io: Remove redundant read/write_coroutine assignments, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 14/71] nbd: Move nbd_read_eof() to nbd/client.c, Kevin Wolf, 2019/02/25
- [Qemu-block] [PULL 18/71] block: Fix AioContext switch for drained node, Kevin Wolf, 2019/02/25