On Sat, 06/08 14:58, Wenchao Xia wrote:
>
> -static int find_snapshot_by_id(BlockDriverState *bs, const char *id_str)
> +static int find_snapshot_by_id_and_name(BlockDriverState *bs,
> + const char *id,
> + const char *name)
> {
> BDRVQcowState *s = bs->opaque;
> int i;
>
> - for(i = 0; i < s->nb_snapshots; i++) {
> - if (!strcmp(s->snapshots[i].id_str, id_str))
> - return i;
> + if (id && name) {
> + for (i = 0; i < s->nb_snapshots; i++) {
> + if (!strcmp(s->snapshots[i].id_str, id) &&
> + !strcmp(s->snapshots[i].name, name)) {
> + return i;
> + }
> + }
> + } else if (id) {
> + for (i = 0; i < s->nb_snapshots; i++) {
> + if (!strcmp(s->snapshots[i].id_str, id)) {
> + return i;
> + }
> + }
> + } else if (name) {
> + for (i = 0; i < s->nb_snapshots; i++) {
> + if (!strcmp(s->snapshots[i].name, name)) {
> + return i;
> + }
> + }
> }
Can be simplified the same way with patch 4.
>
> -int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
> +int qcow2_snapshot_delete(BlockDriverState *bs,
> + const char *snapshot_id,
> + const char *name,
> + Error **errp)
> {
> BDRVQcowState *s = bs->opaque;
> QCowSnapshot sn;
> int snapshot_index, ret;
>
> /* Search the snapshot */
> - snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
> + snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
> if (snapshot_index < 0) {
> + error_setg(errp,
> + "Can't find a snapshot with ID %s and name %s",
> + snapshot_id, name);
> return -ENOENT;
> }
> sn = s->snapshots[snapshot_index];
> @@ -550,6 +570,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const
> char *snapshot_id)
> s->nb_snapshots--;
> ret = qcow2_write_snapshots(bs);
> if (ret < 0) {
> + error_setg(errp, "Failed to remove it from snapshot list");
Maybe put name and id in error message, as above?