Re: [Qemu-devel] [PATCH V3 11/11] hmp: show snapshot on single block device

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-1-15 19:05, Luiz Capitulino 写道:
> On Tue, 15 Jan 2013 10:36:22 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>>   > On Mon, 2013-01-14 at 15:09 +0800, Wenchao Xia wrote:
>>>>     This patch use block layer API to qmp snapshot info on a block
>>>> device, then use the same code dumping vm snapshot info, to print
>>>> in monitor.
>>>>
>>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>>> ---
>>>> Note:
>>>>     This patch need previous hmp extention patch which enable
>>>> info sub command take qdict * as paramter.
>>>
>>>> diff --git a/savevm.c b/savevm.c
>>>> index cabdcb6..cd474e9 100644
>>>> --- a/savevm.c
>>>> +++ b/savevm.c
>>>> @@ -2354,9 +2354,50 @@ static void do_info_snapshots_vm(Monitor *mon)
>>>>        return;
>>>>    }
>>>>
>>>> +static void do_info_snapshots_blk(Monitor *mon, const char *device)
>>>> +{
>>>> +    Error *err = NULL;
>>>> +    SnapshotInfoList *list;
>>>> +    BlockDriverState *bs;
>>>> +
>>>> +    /* 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 ;
>>>> +    }
>>>> +
>>>> +    list = bdrv_query_snapshot_infolist(bs, NULL, NULL, &err);
>>>> +    if (error_is_set(&err)) {
>>>> +        hmp_handle_error(mon, &err);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (list == NULL) {
>>>> +        monitor_printf(mon, "There is no snapshot available.\n");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    monitor_printf(mon, "Device '%s':\n", device);
>>>> +    monitor_dump_snapshotinfolist(mon, list);
>>>> +    qapi_free_SnapshotInfoList(list);
>>>> +    return;
>>>> +}
>>>> +
>>>>    void do_info_snapshots(Monitor *mon, const QDict *qdict)
>>>>    {
>>>> -    do_info_snapshots_vm(mon);
>>>> +    const char *device = qdict_get_try_str(qdict, "device");
>>>> +    if (!device) {
>>>> +        do_info_snapshots_vm(mon);
>>>> +    } else {
>>>> +        do_info_snapshots_blk(mon, device);
>>>> +    }
>>>> +    return;
>>>>    }
>>>>
>>>
>>> I think that you should move these functions into hmp.c file. This also
>>> applies to previous patch and according to this changes you don't have
>>> to export hmp_handle_error() function which should be used only in hmp.c
>>>
>>> Pavel
>>>
>>     It seems there are other "do_info_**" function in other .c files,
>> so I suggest a different serial later which move those functions, if
>> necessary.
>
> The other functions are probably old hmp code. That is, code that is not
> converted to make QMP calls. Generally, new hmp code goes into hmp.c unless
> there's a good reason to put them in a different file.
>
> So, please move it to hmp.c.
>

OK, new hmp function will goto hmp.c

-- 
Best Regards

Wenchao Xia