On 11/16/2015 12:31 PM, Stefan Hajnoczi wrote:
On Tue, Nov 10, 2015 at 05:25:30PM +0300, Denis V. Lunev wrote:+int bdrv_all_find_snapshot(const char *name, bool read_only, + BlockDriverState **first_bad_bs) +{ + QEMUSnapshotInfo sn; + int err = 0; + BlockDriverState *bs = NULL; + + while (err == 0 && (bs = bdrv_next(bs))) { + AioContext *ctx = bdrv_get_aio_context(bs); + + aio_context_acquire(ctx); + if (read_only || (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs))) { + err = bdrv_snapshot_find(bs, &sn, name); + } + aio_context_release(ctx); + } + + *first_bad_bs = bs; + return err; +}It's difficult to see how bdrv_all_find_snapshot(read_only=true) is equivalent to what you replaced below:@@ -1500,21 +1489,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict) available_snapshots = g_new0(int, nb_sns); total = 0; for (i = 0; i < nb_sns; i++) { - sn = &sn_tab[i]; - available = 1; - bs1 = NULL; - - while ((bs1 = bdrv_next(bs1))) { - if (bdrv_can_snapshot(bs1) && bs1 != bs) { - ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str); - if (ret < 0) { - available = 0; - break; - } - } - } - - if (available) { + if (bdrv_all_find_snapshot(sn_tab[i].id_str, true, &bs1) == 0) {The original loop skips drives that cannot snapshot and the vmstate drive. The new code tries to find a snapshot all devices. To me it seems the semantics are changed. Can you explain why this change is safe?
yep. you are once again right... OK.
