Eric Blake <ebl...@redhat.com> writes: > On 01/14/2013 12:09 AM, Wenchao Xia wrote: >> This patch move it from savevm.c to block.c and export it. >> >> Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com> >> --- >> block.c | 23 +++++++++++++++++++++++ >> include/block/block.h | 2 ++ >> savevm.c | 22 ---------------------- >> 3 files changed, 25 insertions(+), 22 deletions(-) >> >> diff --git a/block.c b/block.c >> index 8192d8e..b7d2f03 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3351,6 +3351,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs, >> return -ENOTSUP; >> } >> >> +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, >> + const char *name) >> +{ >> + QEMUSnapshotInfo *sn_tab, *sn; >> + int nb_sns, i, ret; >> + >> + ret = -ENOENT; >> + nb_sns = bdrv_snapshot_list(bs, &sn_tab); >> + if (nb_sns < 0) { >> + return ret; >> + } >> + for (i = 0; i < nb_sns; i++) { >> + sn = &sn_tab[i]; >> + if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) { > > It is possible (albeit probably stupid) to create a qcow2 file where > snapshot names are merely numeric strings. In fact, just to see what > would happen, I once[1] created a file where: > > snapshot id '1' was named '2' > snapshot id '2' was named 'foo' > > This code comparison favors ids over names; so if I request to delvm 2, > I end up removing the second snapshot, not the first. This is okay, but > probably worth documenting, and probably worth making sure that all code > that looks up a snapshot by name or id goes through this function so > that we get the same behavior everywhere. My experiment was done > several months ago, but my recollection was that at the time, there was > an inconsistency where 'qemu-img snapshot' picked a different snapshot > for the request of '2' than the online 'delvm' monitor command of qemu; > making it unsafe to rely on either behavior in that version of qemu > source code. > > [1]https://bugzilla.redhat.com/show_bug.cgi?id=733143
In QemuOpts, we restricted IDs to [[:alpha:]][[:alnum:]-._]*, see id_wellformed(). I'd recommend the same for new interfaces. But this is an old one, and we shouldn't make existing snapshots inaccessible just because their names were chosen unwisely.