Re: [Qemu-devel] [PATCH] qemu-img: simplify img_convert

[Eric Blake]

On 02/28/2017 07:35 AM, Peter Lieven wrote:
> img_convert has been around before there was an ImgConvertState or
> a block backend, but it has never been modified to directly use
> these structs. Change this by parsing parameters directly into
> the ImgConvertState and directly use BlockBackend where possible.
> Futhermore variable initalization has been reworked and sorted.

s/Futhermore/Furthermore/
s/initalization/initialization/

I almost wonder if this should be split into multiple patches for ease
of review.  But since you've already submitted it, I'll review it as-is.

> 
> Signed-off-by: Peter Lieven <address@hidden>
> ---
>  qemu-img.c | 197 
> +++++++++++++++++++++++++------------------------------------
>  1 file changed, 81 insertions(+), 116 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index caa76a7..f271167 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1488,7 +1488,7 @@ typedef struct ImgConvertState {
>      int min_sparse;
>      size_t cluster_sectors;
>      size_t buf_sectors;
> -    int num_coroutines;
> +    long num_coroutines;

Why is this change needed? A hint in the commit message may be
appropriate (I'm especially wary of 'long', as it differs between 32-
and 64-bit platforms).

> @@ -2012,19 +2002,18 @@ static int img_convert(int argc, char **argv)
>              quiet = true;
>              break;
>          case 'n':
> -            skip_create = 1;
> +            skip_create = true;
>              break;
>          case 'm':
> -            if (qemu_strtol(optarg, NULL, 0, &num_coroutines) ||
> -                num_coroutines < 1 || num_coroutines > MAX_COROUTINES) {
> +            if (qemu_strtol(optarg, NULL, 0, &s.num_coroutines) ||
> +                s.num_coroutines < 1 || s.num_coroutines > MAX_COROUTINES) {

At any rate, it looks like you clamp it, so the change in type doesn't
matter too much.

I didn't spot any problems in the conversion, so:
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature