Re: [Qemu-devel] [PATCH V12 07/18] block: change VM snapshot checking logic

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]

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 <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qapi.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)

Reviewed-by: Eric Blake <eblake@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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]