[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv5] add qemu-img convert -n option (skip target v
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCHv5] add qemu-img convert -n option (skip target volume creation) |
Date: |
Fri, 30 Aug 2013 11:31:52 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8 |
On 08/26/2013 03:12 AM, Alex Bligh wrote:
> From: Alexandre Derumier <address@hidden>
>
> Add a -n option to skip volume creation on qemu-img convert.
> This is useful for targets such as rbd / ceph, where the
> target volume may already exist; we cannot always rely on
> qemu-img convert to create the image, as dependent on the
> output format, there may be parameters which are not possible
> to specify through the qemu-img convert command line.
>
> Signed-off-by: Alexandre Derumier <address@hidden>
> Signed-off-by: Alex Bligh <address@hidden>
> ---
> static int img_convert(int argc, char **argv)
> {
> - int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
> cluster_sectors;
> + int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
> + cluster_sectors, skip_create;
Pre-existing usage, but this new variable is used only as a bool, so it
might be better to type it (and others like it) correctly...
> int progress = 0, flags;
> const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
> BlockDriver *drv, *proto_drv;
> @@ -1139,8 +1142,9 @@ static int img_convert(int argc, char **argv)
> cache = "unsafe";
> out_baseimg = NULL;
> compress = 0;
> + skip_create = 0;
...which would mean using 'false' instead of '0' here, and so forth. As
at least 'compress' is also impacted, I'm okay deferring the type
cleanup to a separate patch.
> for(;;) {
> - c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
> + c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qn");
The order here...
> if (c == -1) {
> break;
> }
> @@ -1161,6 +1165,9 @@ static int img_convert(int argc, char **argv)
> case 'c':
> compress = 1;
> break;
> + case 'n':
> + skip_create = 1;
> + break;
...and the order of the case statements, are unrelated. Which makes
maintenance a bit harder. Personally, I like to keep my optstring in
case-insensitive order, then my case statements in the same order. But
that's cosmetic, it doesn't affect the correctness of the patch.
> +++ b/tests/qemu-iotests/060
> @@ -0,0 +1,101 @@
> +#!/bin/bash
> +#
> +# test of qemu-img convert -n - convert without creation
> +#
> +# Copyright (C) 2009 Red Hat, Inc.
Where have you been the last 4 years? I could understand a range of
years, if this test borrows significantly from another file that old,
but I suspect that just 2013 is probably more accurate.
Fix that, and you can add:
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