>> >> +static void do_info_snapshots_blk(Monitor *mon, const char *device) >> +{ >> + BlockDriverState *bs; >> + QEMUSnapshotInfo *sn_tab, *sn; >> + int nb_sns, i; >> + char buf[256]; >> + >> + /* find the target bs */ >> + bs = bdrv_find(device); >> + if (!bs) { >> + monitor_printf(mon, "Device '%s' not found.\n", device); >> + return ; >> + } >> + >> + if (!bdrv_can_snapshot(bs)) { >> + monitor_printf(mon, "Device '%s' can't have snapshot.\n", device); >> + return ; >> + } > > Rest of the function is is essentially code copied from > do_info_snapshots_vm(). Could it be factored out? > Sure, I forgot that before. I guess a better way is adding a QMP interface first and using common code from it.
>> + >> + nb_sns = bdrv_snapshot_list(bs, &sn_tab); >> + if (nb_sns < 0) { >> + monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n", >> + device, nb_sns); >> + return; >> + } >> + >> + if (nb_sns == 0) { >> + monitor_printf(mon, "There is no snapshot available.\n"); >> + return; >> + } >> + >> + monitor_printf(mon, "Device %s:\n", device); >> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL)); >> + for (i = 0; i < nb_sns; i++) { >> + sn = &sn_tab[i]; >> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), >> sn)); >> + } >> + g_free(sn_tab); >> + return; >> +} >> + >> +void do_info_snapshots(Monitor *mon, const QDict *qdict) >> +{ >> + /* Todo, there should be a layer rebuild qdict before enter this func. >> */ > > I'm not sure I get this comment. > > We usually capitalize TODO for easy grepping. > My mistake, this comments should have been removed, it is used before where qdict was not rebuilt as a new one. >> + const char *device = qdict_get_try_str(qdict, "device"); >> + if (!device) { >> + do_info_snapshots_vm(mon); >> + } else { >> + do_info_snapshots_blk(mon, device); >> + } >> + return; >> +} >> + >> void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) >> { >> qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK, > -- Best Regards Wenchao Xia