On 04/13/2013 02:56 AM, Wenchao Xia wrote: > Original logic is different with load_vmstate(), this patch change it
s/with/from/
s/change/changes/
> to be exactly the same with load_vmstate(), so any VM snapshot shown in
s/with/as/
> qmp/hmp should succeed in load_vmstate().
Looking through git logs, most people use blank lines and start at
column 1, rather than your idiom of 2-space indent and no blank line,
when starting a new paragraph.
> Note that, runtime snapshot info maybe different with what is got
s/that,/that consistent/
s/maybe different with/may be a subset of/
s/got/listed/
> in "qemu-img info" as static snapshot info, and this patch clearly
> tips it.
s/tips/identifies/
>
> Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com>
> ---
> block/qapi.c | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
Reviewed-by: Eric Blake <ebl...@redhat.com>
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 97c5cd4..49c0eb0 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -46,7 +46,18 @@ static bool snapshot_valid_for_vm(const QEMUSnapshotInfo
> *sn,
> take snapshot, for example, readonly ones, will be ignored in
> load_vmstate(). */
> while ((bs1 = bdrv_next(bs1))) {
> - if (bs1 != bs && bdrv_can_snapshot(bs1)) {
> +
A blank line after a while(){ is not common, but as it is only
whitespace, and checkpatch didn't complain, it doesn't hold up my review.
> + if (!bdrv_is_inserted(bs1) || bdrv_is_read_only(bs1)) {
> + continue;
> + }
> +
> + if (!bdrv_can_snapshot(bs1)) {
> + /* Device is writable but does not support snapshots, will be
> + rejected by load_vmstate(). */
> + return false;
> + }
> +
> + if (bs1 != bs) {
> ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
> if (ret < 0) {
> return false;
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
