qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH RFC 1/1] qemu-img: add the 'dd' subcommand


From: Fam Zheng
Subject: Re: [Qemu-block] [PATCH RFC 1/1] qemu-img: add the 'dd' subcommand
Date: Thu, 23 Jun 2016 11:32:50 +0800
User-agent: Mutt/1.6.1 (2016-04-27)

On Wed, 06/22 19:21, Reda Sallahi wrote:
> This patch adds a basic dd subcommand analogous to dd(1) to qemu-img.
> 
> For the start, this implements the bs, if, of and count options and requires
> both if and of to be specified (no stdin/stdout if not specified) and doesn't
> support tty, pipes, etc.
> 
> The image format must be specified with -O for the output if the raw format
> is not the intended one.
> 
> get_size() and get_offset() were needed for the size syntax dd(1) supports
> which is different from qemu_strtosz_suffix.

Looks good in general! Comments below.

> 
> Signed-off-by: Reda Sallahi <address@hidden>
> ---
>  qemu-img-cmds.hx |   6 +
>  qemu-img.c       | 833 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 838 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 7e95b2d..68f81b0 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -45,6 +45,12 @@ STEXI
>  @item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] 
> [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] 
> [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] 
> [-S @var{sparse_size}] @var{filename} address@hidden [...]] 
> @var{output_filename}
>  ETEXI
>  
> +DEF("dd", img_dd,
> +    "dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] 
> [ibs=in_block_size] [count=blocks] if=input of=output")
> +STEXI
> address@hidden dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] 
> address@hidden address@hidden address@hidden address@hidden address@hidden
> +ETEXI
> +
>  DEF("info", img_info,
>      "info [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] 
> [--backing-chain] filename")
>  STEXI
> diff --git a/qemu-img.c b/qemu-img.c
> index 14e2661..dace76b 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -159,7 +159,25 @@ static void QEMU_NORETURN help(void)
>             "Parameters to compare subcommand:\n"
>             "  '-f' first image format\n"
>             "  '-F' second image format\n"
> -           "  '-s' run in Strict mode - fail on different image size or 
> sector allocation\n";
> +           "  '-s' run in Strict mode - fail on different image size or 
> sector allocation\n"
> +           "\n"
> +           "Parameters to dd subcommand:\n"
> +           "  'bs=BYTES' read and write up to BYTES bytes at a time\n"
> +/*         "  'cbs=BYTES' convert BYTES bytes at a time\n"
> +           "  'conv=CONVS' convert the file as per the comma separated "
> +           "symbol list\n" */
> +           "  'count=N' copy only N input blocks\n"
> +           "  'ibs=BYTES' read up to BYTES bytes at a time (default: 512)\n"
> +           "  'if=FILE' read from FILE instead of stdin\n"
> +           "  'obs=BYTES' write BYTES bytes at a time (default: 512)\n"
> +           "  'of=FILE' write to FILE instead of stdout\n";
> +/*         "  'seek=N' skip N obs-sized blocks at start of output\n"
> +           "  'skip=N' skip N ibs-sized blocks at start of input\n"
> +           "  'status=LEVEL' The LEVEL of information to print to stderr; "
> +           "'none' suppresses everything but error messages, 'noxfer' "
> +           "suppresses the final transfer statistics, 'progress' shows "
> +           "periodic transfer statistics\n" */

In next revisions please clean up unused code/document in this patch, and add
them in later patches when they are needed - so the initial change for "if, of
and bs" can be merged.

> +
>  
>      printf("%s\nSupported formats:", help_msg);
>      bdrv_iterate_format(format_print, NULL);
> @@ -3788,6 +3806,819 @@ out:
>      return 0;
>  }
>  
> +#define C_BS      01
> +#define C_CBS     02
> +#define C_CONV    04
> +#define C_COUNT   010
> +#define C_IBS     020
> +#define C_IF      040
> +#define C_IFLAG   0100
> +#define C_OBS     0200
> +#define C_OF      0400
> +#define C_OFLAG   01000
> +#define C_SEEK    02000
> +#define C_SKIP    04000
> +#define C_STATUS  010000

Set but not used, maybe remove?

> +
> +struct DdEss {
> +    unsigned int flags;
> +    unsigned int status;
> +    unsigned int conv;
> +    size_t count;
> +    size_t cbsz; /* Conversion block size */
> +};
> +
> +struct DdIo {
> +    size_t bsz;    /* Block size */
> +    off_t offset;
> +    const char *filename;
> +    unsigned int flags;
> +    uint8_t *buf;
> +};
> +
> +struct DdOpts {
> +    const char *name;
> +    int (*f)(const char *, struct DdIo *, struct DdIo *, struct DdEss *);
> +    unsigned int flag;
> +};
> +
> +static size_t get_size(const char *str)
> +{
> +    /* XXX: handle {k,m,g}B notations */
> +    unsigned long num;
> +    size_t res = 0;
> +    const char *buf;
> +
> +    errno = 0;
> +    qemu_strtoul(str, &buf, 0, &num);
> +
> +    if (num == ULONG_MAX && errno == ERANGE) {

qemu_strtol returns error code in return value, instead of errno. And in the
case of error, no need to test num == ULONG_MAX.

> +        error_report("invalid number: %s", str);
> +        return res;
> +    }
> +
> +    switch (*buf) {
> +    case '\0':
> +    case 'c':
> +        res = num;
> +        break;
> +    case 'w':
> +        res = num * 2;
> +        break;
> +    case 'b':
> +        res = num * 512;
> +        break;
> +    case 'K':
> +        res = num * 1024;
> +        break;
> +    case 'M':
> +        res = num * 1024 * 1024;
> +        break;
> +    case 'G':
> +        res = num * 1024 * 1024 * 1024;
> +        break;
> +    case 'T':
> +        res = num * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'P':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'E':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'Z':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'Y':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    default:
> +        error_report("invalid number: '%s'", str);
> +        errno = EINVAL;
> +    }
> +
> +    return res;
> +

Superfluous blank line.

> +}
> +
> +static off_t get_offset(const char *str)

Can this be merged with get_size?

> +{
> +    /* XXX handle {k,m,g}B notations */
> +    off_t res = 0;
> +    long num;
> +    const char *buf;
> +
> +    errno = 0;
> +    qemu_strtol(str, &buf, 0, &num);

Same as above.

> +
> +    if (errno == ERANGE) {
> +        error_report("invalid number: '%s'", str);
> +        return res;
> +    }
> +
> +    switch (*buf) {
> +    case '\0':
> +    case 'c':
> +        res = num;
> +        break;
> +    case 'w':
> +        res = num * 2;
> +        break;
> +    case 'b':
> +        res = num * 512;
> +        break;
> +    case 'K':
> +        res = num * 1024;
> +        break;
> +    case 'M':
> +        res = num * 1024 * 1024;
> +        break;
> +    case 'G':
> +        res = num * 1024 * 1024 * 1024;
> +        break;
> +    case 'T':
> +        res = num * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'P':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'E':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'Z':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    case 'Y':
> +        res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
> +        break;
> +    default:
> +        error_report("invalid number: '%s'", str);
> +        errno = EINVAL;
> +    }
> +
> +    return res;
> +}
> +
> +static int img_dd_bs(const char *arg,
> +                     struct DdIo *in, struct DdIo *out,
> +                     struct DdEss *dd)
> +{
> +    if (strchr(arg, '-')) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +    in->bsz = out->bsz = get_size(arg);

I think all other checks in this function can go into get_size, so they are not
repeated in every img_foo argument handler below.

> +
> +    if (in->bsz == 0 && (errno == EINVAL || errno == ERANGE)) {
> +        return 1;
> +    }
> +    if (in->bsz == 0) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +

One empty line too much.

> +    return 0;
> +}
> +
> +static int img_dd_cbs(const char *arg,
> +                      struct DdIo *in, struct DdIo *out,
> +                      struct DdEss *dd)
> +{
> +    if (strchr(arg, '-')) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +    dd->cbsz = get_size(arg);
> +
> +    if (dd->cbsz == 0 && (errno == EINVAL || errno == ERANGE)) {
> +        return 1;
> +    }
> +    if (dd->cbsz == 0) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +
> +    return 0;
> +}
> +
> +struct DdSymbols {
> +    const char *name;
> +    unsigned int value;
> +};
> +
> +#define C_ASCII    01
> +#define C_EBCDIC   02
> +#define C_IBM      04
> +#define C_BLOCK    010
> +#define C_UNBLOCK  020
> +#define C_LCASE    040
> +#define C_UCASE    0100
> +#define C_SPARSE   0200
> +#define C_SWAB     0400
> +#define C_SYNC     01000
> +#define C_EXCL     02000
> +#define C_NOCREAT  04000
> +#define C_NOTRUNC  010000
> +#define C_NOERROR  020000
> +#define C_FDATASYNC 040000
> +#define C_FSYNC     0100000
> +
> +static int img_dd_conv(const char *arg,
> +                       struct DdIo *in, struct DdIo *out,
> +                       struct DdEss *dd)
> +{
> +    const char *tok;
> +    char *str, *tmp;
> +    int ret = 0;
> +    const struct DdSymbols conv[] = {
> +        { "ascii", C_ASCII },
> +        { "ebcdic", C_EBCDIC },
> +        { "ibm", C_IBM },
> +        { "block", C_BLOCK },
> +        { "unblock", C_UNBLOCK },
> +        { "lcase", C_LCASE },
> +        { "ucase", C_UCASE },
> +        { "sparse", C_SPARSE },
> +        { "sync", C_SYNC },
> +        { "excl", C_EXCL },
> +        { "nocreat", C_NOCREAT },
> +        { "notrunc", C_NOTRUNC },
> +        { "noerror", C_NOERROR },
> +        { "fdatasync", C_FDATASYNC },
> +        { "fsync", C_FSYNC },
> +        { NULL, 0 }
> +    };
> +
> +    tmp = str = g_strdup(arg);
> +
> +    while (tmp != NULL && !ret) {
> +        tok = qemu_strsep(&tmp, ",");
> +        int j;
> +        for (j = 0; conv[j].name != NULL; j++) {
> +            if (!strcmp(tok, conv[j].name)) {
> +                dd->conv |= conv[j].value;
> +                break;
> +            }
> +        }
> +        if (conv[j].name == NULL) {
> +            error_report("invalid conversion: '%s'", tok);
> +            ret = 1;
> +        }
> +    }
> +
> +    g_free(str);
> +    return ret;
> +}
> +
> +static int img_dd_count(const char *arg,
> +                        struct DdIo *in, struct DdIo *out,
> +                        struct DdEss *dd)
> +{
> +    if (strchr(arg, '-')) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +    dd->count = get_size(arg);
> +
> +    if (dd->count == 0 && (errno == EINVAL || errno == ERANGE)) {
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int img_dd_ibs(const char *arg,
> +                      struct DdIo *in, struct DdIo *out,
> +                      struct DdEss *dd)
> +{
> +    if (strchr(arg, '-')) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +    in->bsz = get_size(arg);
> +
> +    if (in->bsz == 0 && (errno == EINVAL || errno == ERANGE)) {
> +        return 1;
> +    }
> +    if (in->bsz == 0) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int img_dd_if(const char *arg,
> +                     struct DdIo *in, struct DdIo *out,
> +                     struct DdEss *dd)
> +{
> +    in->filename = arg;
> +
> +    return 0;
> +}
> +
> +#define C_APPEND      01
> +#define C_DIRECT      02
> +#define C_DIRECTORY   04
> +#define C_DSYNC       010
> +#define C_SYNC_FLAG   020
> +#define C_FULLBLOCK   040
> +#define C_NONBLOCK    0100
> +#define C_NOATIME     0200
> +#define C_NOCACHE     0400
> +#define C_NOCTTY      01000
> +#define C_NOFOLLOW    02000
> +#define C_COUNT_BYTES 04000
> +#define C_SKIP_BYTES  010000
> +#define C_SEEK_BYTES  020000
> +
> +static int img_dd_iflag(const char *arg,
> +                        struct DdIo *in, struct DdIo *out,
> +                        struct DdEss *dd)
> +{
> +    const struct DdSymbols flags[] = {
> +        { "direct", C_DIRECT },
> +        { "directory", C_DIRECTORY },
> +        { "dsync", C_DSYNC },
> +        { "sync", C_SYNC_FLAG },
> +        { "fullblock", C_FULLBLOCK },
> +        { "nonblock", C_NONBLOCK },
> +        { "noatime", C_NOATIME },
> +        { "nocache", C_NOCACHE },
> +        { "noctty", C_NOCTTY },
> +        { "nofollow", C_NOFOLLOW },
> +        { "count_bytes", C_COUNT_BYTES },
> +        { "skip_bytes", C_SKIP_BYTES },
> +        { NULL, 0}
> +    };
> +
> +    for (int j = 0; flags[j].name != NULL; j++) {
> +        if (!strcmp(arg, flags[j].name)) {
> +            in->flags = flags[j].value;
> +            return 0;
> +        }
> +    }
> +
> +    error_report("invalid input flag: '%s'", arg);
> +    return 1;
> +}
> +
> +static int img_dd_obs(const char *arg,
> +                      struct DdIo *in, struct DdIo *out,
> +                      struct DdEss *dd)
> +{
> +    if (strchr(arg, '-')) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +    out->bsz = get_size(arg);
> +
> +    if (out->bsz == 0 && (errno == EINVAL || errno == ERANGE)) {
> +        return 1;
> +    }
> +    if (out->bsz == 0) {
> +        error_report("invalid number: '%s'", arg);
> +        return 1;
> +    }
> +
> +
> +    return 0;
> +}
> +
> +static int img_dd_of(const char *arg,
> +                     struct DdIo *in, struct DdIo *out,
> +                     struct DdEss *dd)
> +{
> +    out->filename = arg;
> +
> +    return 0;
> +}
> +
> +static int img_dd_oflag(const char *arg,
> +                        struct DdIo *in, struct DdIo *out,
> +                        struct DdEss *dd)
> +{
> +    const struct DdSymbols flags[] = {
> +        { "append", C_APPEND },
> +        { "direct", C_DIRECT },
> +        { "directory", C_DIRECTORY },
> +        { "dsync", C_DSYNC },
> +        { "sync", C_SYNC_FLAG },
> +        { "nonblock", C_NONBLOCK },
> +        { "noatime", C_NOATIME },
> +        { "nocache", C_NOCACHE },
> +        { "noctty", C_NOCTTY },
> +        { "nofollow", C_NOFOLLOW },
> +        { "seek_bytes", C_SEEK_BYTES },
> +        { NULL, 0 }
> +    };
> +
> +    for (int j = 0; flags[j].name != NULL; j++) {
> +        if (!strcmp(arg, flags[j].name)) {
> +            out->flags = flags[j].value;
> +            return 0;
> +        }
> +    }
> +
> +    error_report("invalid output flag: '%s'", arg);
> +    return 1;
> +}
> +
> +static int img_dd_seek(const char *arg,
> +                       struct DdIo *in, struct DdIo *out,
> +                       struct DdEss *dd)
> +{
> +    out->offset = get_offset(arg);
> +
> +    if (out->offset == 0 && (errno == EINVAL || errno == ERANGE)) {
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int img_dd_skip(const char *arg,
> +                       struct DdIo *in, struct DdIo *out,
> +                       struct DdEss *dd)
> +{
> +    in->offset = get_offset(arg);
> +
> +    if (in->offset == 0 && (errno == EINVAL || errno == ERANGE)) {
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +#define C_STATUS_DEFAULT  00
> +#define C_STATUS_NONE     01
> +#define C_STATUS_NOXFER   02
> +#define C_STATUS_PROGRESS 04
> +
> +static int img_dd_status(const char *arg,
> +                         struct DdIo *in, struct DdIo *out,
> +                         struct DdEss *dd)
> +{
> +    const struct DdSymbols dd_status[] = {
> +        { "none", C_STATUS_NONE },
> +        { "noxfer", C_STATUS_NOXFER },
> +        { "progress", C_STATUS_PROGRESS },
> +        { "default", C_STATUS_DEFAULT },
> +        { NULL, 0 }
> +    };
> +
> +    for (int j = 0; dd_status[j].name != NULL; j++) {
> +        if (!strcmp(arg, dd_status[j].name)) {
> +            dd->status = dd_status[j].value;
> +            return 0;
> +        }
> +    }
> +
> +    error_report("invalid status level: '%s'", arg);
> +    return 1;
> +}
> +
> +/* Conversion table for later user for the conv option.
> +
> +static char const ascii_to_ebcdic[] =
> +{
> +  '\000', '\001', '\002', '\003', '\067', '\055', '\056', '\057',
> +  '\026', '\005', '\045', '\013', '\014', '\015', '\016', '\017',
> +  '\020', '\021', '\022', '\023', '\074', '\075', '\062', '\046',
> +  '\030', '\031', '\077', '\047', '\034', '\035', '\036', '\037',
> +  '\100', '\132', '\177', '\173', '\133', '\154', '\120', '\175',
> +  '\115', '\135', '\134', '\116', '\153', '\140', '\113', '\141',
> +  '\360', '\361', '\362', '\363', '\364', '\365', '\366', '\367',
> +  '\370', '\371', '\172', '\136', '\114', '\176', '\156', '\157',
> +  '\174', '\301', '\302', '\303', '\304', '\305', '\306', '\307',
> +  '\310', '\311', '\321', '\322', '\323', '\324', '\325', '\326',
> +  '\327', '\330', '\331', '\342', '\343', '\344', '\345', '\346',
> +  '\347', '\350', '\351', '\255', '\340', '\275', '\232', '\155',
> +  '\171', '\201', '\202', '\203', '\204', '\205', '\206', '\207',
> +  '\210', '\211', '\221', '\222', '\223', '\224', '\225', '\226',
> +  '\227', '\230', '\231', '\242', '\243', '\244', '\245', '\246',
> +  '\247', '\250', '\251', '\300', '\117', '\320', '\137', '\007',
> +  '\040', '\041', '\042', '\043', '\044', '\025', '\006', '\027',
> +  '\050', '\051', '\052', '\053', '\054', '\011', '\012', '\033',
> +  '\060', '\061', '\032', '\063', '\064', '\065', '\066', '\010',
> +  '\070', '\071', '\072', '\073', '\004', '\024', '\076', '\341',
> +  '\101', '\102', '\103', '\104', '\105', '\106', '\107', '\110',
> +  '\111', '\121', '\122', '\123', '\124', '\125', '\126', '\127',
> +  '\130', '\131', '\142', '\143', '\144', '\145', '\146', '\147',
> +  '\150', '\151', '\160', '\161', '\162', '\163', '\164', '\165',
> +  '\166', '\167', '\170', '\200', '\212', '\213', '\214', '\215',
> +  '\216', '\217', '\220', '\152', '\233', '\234', '\235', '\236',
> +  '\237', '\240', '\252', '\253', '\254', '\112', '\256', '\257',
> +  '\260', '\261', '\262', '\263', '\264', '\265', '\266', '\267',
> +  '\270', '\271', '\272', '\273', '\274', '\241', '\276', '\277',
> +  '\312', '\313', '\314', '\315', '\316', '\317', '\332', '\333',
> +  '\334', '\335', '\336', '\337', '\352', '\353', '\354', '\355',
> +  '\356', '\357', '\372', '\373', '\374', '\375', '\376', '\377'
> +};
> +
> +static char const ascii_to_ibm[] =
> +{
> +  '\000', '\001', '\002', '\003', '\067', '\055', '\056', '\057',
> +  '\026', '\005', '\045', '\013', '\014', '\015', '\016', '\017',
> +  '\020', '\021', '\022', '\023', '\074', '\075', '\062', '\046',
> +  '\030', '\031', '\077', '\047', '\034', '\035', '\036', '\037',
> +  '\100', '\132', '\177', '\173', '\133', '\154', '\120', '\175',
> +  '\115', '\135', '\134', '\116', '\153', '\140', '\113', '\141',
> +  '\360', '\361', '\362', '\363', '\364', '\365', '\366', '\367',
> +  '\370', '\371', '\172', '\136', '\114', '\176', '\156', '\157',
> +  '\174', '\301', '\302', '\303', '\304', '\305', '\306', '\307',
> +  '\310', '\311', '\321', '\322', '\323', '\324', '\325', '\326',
> +  '\327', '\330', '\331', '\342', '\343', '\344', '\345', '\346',
> +  '\347', '\350', '\351', '\255', '\340', '\275', '\137', '\155',
> +  '\171', '\201', '\202', '\203', '\204', '\205', '\206', '\207',
> +  '\210', '\211', '\221', '\222', '\223', '\224', '\225', '\226',
> +  '\227', '\230', '\231', '\242', '\243', '\244', '\245', '\246',
> +  '\247', '\250', '\251', '\300', '\117', '\320', '\241', '\007',
> +  '\040', '\041', '\042', '\043', '\044', '\025', '\006', '\027',
> +  '\050', '\051', '\052', '\053', '\054', '\011', '\012', '\033',
> +  '\060', '\061', '\032', '\063', '\064', '\065', '\066', '\010',
> +  '\070', '\071', '\072', '\073', '\004', '\024', '\076', '\341',
> +  '\101', '\102', '\103', '\104', '\105', '\106', '\107', '\110',
> +  '\111', '\121', '\122', '\123', '\124', '\125', '\126', '\127',
> +  '\130', '\131', '\142', '\143', '\144', '\145', '\146', '\147',
> +  '\150', '\151', '\160', '\161', '\162', '\163', '\164', '\165',
> +  '\166', '\167', '\170', '\200', '\212', '\213', '\214', '\215',
> +  '\216', '\217', '\220', '\232', '\233', '\234', '\235', '\236',
> +  '\237', '\240', '\252', '\253', '\254', '\255', '\256', '\257',
> +  '\260', '\261', '\262', '\263', '\264', '\265', '\266', '\267',
> +  '\270', '\271', '\272', '\273', '\274', '\275', '\276', '\277',
> +  '\312', '\313', '\314', '\315', '\316', '\317', '\332', '\333',
> +  '\334', '\335', '\336', '\337', '\352', '\353', '\354', '\355',
> +  '\356', '\357', '\372', '\373', '\374', '\375', '\376', '\377'
> +};
> +
> +static char const ebcdic_to_ascii[] =
> +{
> +  '\000', '\001', '\002', '\003', '\234', '\011', '\206', '\177',
> +  '\227', '\215', '\216', '\013', '\014', '\015', '\016', '\017',
> +  '\020', '\021', '\022', '\023', '\235', '\205', '\010', '\207',
> +  '\030', '\031', '\222', '\217', '\034', '\035', '\036', '\037',
> +  '\200', '\201', '\202', '\203', '\204', '\012', '\027', '\033',
> +  '\210', '\211', '\212', '\213', '\214', '\005', '\006', '\007',
> +  '\220', '\221', '\026', '\223', '\224', '\225', '\226', '\004',
> +  '\230', '\231', '\232', '\233', '\024', '\025', '\236', '\032',
> +  '\040', '\240', '\241', '\242', '\243', '\244', '\245', '\246',
> +  '\247', '\250', '\325', '\056', '\074', '\050', '\053', '\174',
> +  '\046', '\251', '\252', '\253', '\254', '\255', '\256', '\257',
> +  '\260', '\261', '\041', '\044', '\052', '\051', '\073', '\176',
> +  '\055', '\057', '\262', '\263', '\264', '\265', '\266', '\267',
> +  '\270', '\271', '\313', '\054', '\045', '\137', '\076', '\077',
> +  '\272', '\273', '\274', '\275', '\276', '\277', '\300', '\301',
> +  '\302', '\140', '\072', '\043', '\100', '\047', '\075', '\042',
> +  '\303', '\141', '\142', '\143', '\144', '\145', '\146', '\147',
> +  '\150', '\151', '\304', '\305', '\306', '\307', '\310', '\311',
> +  '\312', '\152', '\153', '\154', '\155', '\156', '\157', '\160',
> +  '\161', '\162', '\136', '\314', '\315', '\316', '\317', '\320',
> +  '\321', '\345', '\163', '\164', '\165', '\166', '\167', '\170',
> +  '\171', '\172', '\322', '\323', '\324', '\133', '\326', '\327',
> +  '\330', '\331', '\332', '\333', '\334', '\335', '\336', '\337',
> +  '\340', '\341', '\342', '\343', '\344', '\135', '\346', '\347',
> +  '\173', '\101', '\102', '\103', '\104', '\105', '\106', '\107',
> +  '\110', '\111', '\350', '\351', '\352', '\353', '\354', '\355',
> +  '\175', '\112', '\113', '\114', '\115', '\116', '\117', '\120',
> +  '\121', '\122', '\356', '\357', '\360', '\361', '\362', '\363',
> +  '\134', '\237', '\123', '\124', '\125', '\126', '\127', '\130',
> +  '\131', '\132', '\364', '\365', '\366', '\367', '\370', '\371',
> +  '\060', '\061', '\062', '\063', '\064', '\065', '\066', '\067',
> +  '\070', '\071', '\372', '\373', '\374', '\375', '\376', '\377'
> +};
> +*/
> +
> +
> +static int img_dd(int argc, char **argv)
> +{
> +    int ret = 0;
> +    char *arg = NULL;
> +    char *tmp;
> +    struct DdEss dd;
> +    struct DdIo in, out;

Maybe just use an initializer to zero out these structs?

> +    BlockDriver *drv = NULL, *proto_drv = NULL;
> +    BlockBackend *blk1 = NULL, *blk2 = NULL;
> +    QemuOpts *opts = NULL;
> +    QemuOptsList *create_opts = NULL;
> +    Error *local_err = NULL;
> +    bool image_opts = false;
> +    int c;
> +    const char *out_fmt = "raw";
> +    const char *fmt = NULL;
> +    int64_t size = 0;
> +
> +    dd.flags = 0;
> +    dd.status = C_STATUS_DEFAULT;
> +    dd.conv = 0;
> +    dd.count = 0;
> +    in.buf = out.buf = NULL;
> +    dd.cbsz = in.bsz = out.bsz = 512; /* Block size is by default 512 bytes 
> */
> +
> +    const struct DdOpts options[] = {
> +        { "bs", img_dd_bs, C_BS },
> +        { "cbs", img_dd_cbs, C_CBS },
> +        { "conv", img_dd_conv, C_CONV },
> +        { "count", img_dd_count, C_COUNT },
> +        { "ibs", img_dd_ibs, C_IBS },
> +        { "if", img_dd_if, C_IF },
> +        { "iflag", img_dd_iflag, C_IFLAG },
> +        { "obs", img_dd_obs, C_OBS },
> +        { "of", img_dd_of, C_OF },
> +        { "oflag", img_dd_oflag, C_OFLAG },
> +        { "seek", img_dd_seek, C_SEEK },
> +        { "skip", img_dd_skip, C_SKIP },
> +        { "status", img_dd_status, C_STATUS },
> +        { NULL, NULL, 0 }
> +    };
> +    const struct option long_options[] = {
> +        { "help", no_argument, 0, 'h'},
> +        { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> +        { 0, 0, 0, 0 }
> +    };
> +
> +    while ((c = getopt_long(argc, argv, "hf:O:", long_options, NULL))) {
> +        if (c == EOF) {
> +            break;
> +        }
> +        switch (c) {
> +        case 'O':
> +            out_fmt = optarg;
> +            break;
> +        case 'f':
> +            fmt = optarg;
> +            break;
> +        case '?':
> +            error_report("Try 'qemu-img --help' for more information.");
> +            ret = -1;
> +            goto out;
> +            break;
> +        case 'h':
> +            help();
> +            break;
> +        case OPTION_IMAGE_OPTS:
> +            image_opts = true;
> +            break;
> +        }
> +    }
> +
> +    for (int i = optind; i < argc; i++) {

Variable declarations must be in the beginning of block.

> +        arg = g_strdup(argv[i]);
> +
> +        tmp = strchr(arg, '=');
> +        if (tmp == NULL) {
> +            error_report("unrecognized operand %s", arg);
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        *tmp++ = '\0';
> +        int j;

Variable declarations must be in the beginning of block.

> +        for (j = 0; options[j].name != NULL; j++) {
> +            if (!strcmp(arg, options[j].name)) {
> +                break;
> +            }
> +        }
> +        if (options[j].name == NULL) {
> +            error_report("unrecognized operand %s", arg);
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        if (options[j].f(tmp, &in, &out, &dd) != 0) {
> +            ret = -1;
> +            goto out;
> +        }
> +        dd.flags |= options[j].flag;
> +    }
> +
> +    if (dd.flags & C_IF && dd.flags & C_OF) {
> +        blk1 = img_open(image_opts, in.filename, fmt, 0, false, true);
> +
> +        if (!blk1) {
> +            error_report("failed to open '%s'", in.filename);

img_open already reports the error, no need to report again.

> +            ret = -1;
> +            goto out;
> +        }
> +
> +        drv = bdrv_find_format(out_fmt);
> +        if (!drv) {
> +            error_report("Unknown file format");
> +            ret = -1;
> +            goto out;
> +        }
> +        proto_drv = bdrv_find_protocol(out.filename, true, &local_err);
> +
> +        if (!proto_drv) {
> +            error_report_err(local_err);
> +            ret = -1;
> +            goto out;
> +        }
> +        if (!drv->create_opts) {
> +            error_report("Format driver '%s' does not support image 
> creation",
> +                       drv->format_name);

Alignment off by one column.

> +            ret = -1;
> +            goto out;
> +        }
> +        if (!proto_drv->create_opts) {
> +            error_report("Protocol driver '%s' does not support image 
> creation",
> +                       proto_drv->format_name);
> +            ret = -1;
> +            goto out;
> +        }
> +        create_opts = qemu_opts_append(create_opts, drv->create_opts);
> +        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
> +
> +        opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> +
> +        size =  blk_getlength(blk1);
> +
> +        if (dd.flags & C_COUNT && dd.count * in.bsz < size) {
> +            qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> +                                dd.count * in.bsz, &error_abort);
> +        } else {
> +            qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);
> +        }
> +
> +        ret = bdrv_create(drv, out.filename, opts, &local_err);
> +        if (ret < 0) {
> +            error_reportf_err(local_err,
> +                              "%s: error while copying and converting raw: ",

not necessarily raw.

> +                              out.filename);
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
> +                        false, true);
> +
> +        if (!blk2) {
> +            error_report("failed to open '%s'", out.filename);

Superfluous error reporting.

> +            ret = -1;
> +            goto out;
> +        }
> +
> +        in.buf = g_new(uint8_t, in.bsz);
> +        out.buf = g_new(uint8_t, out.bsz);

out.buf unused?

> +
> +        int64_t count, block_count;
> +
> +        for (count = 0, block_count = 0; count < size;
> +             count += ret, block_count++) {
> +            /* If the count option is specified. */
> +            if (dd.flags & C_COUNT && block_count >= dd.count) {
> +                break;
> +            }
> +
> +            if (in.bsz + count > size) {
> +                ret = blk_pread(blk1, count, in.buf, size - count);
> +            } else {
> +                ret = blk_pread(blk1, count, in.buf, in.bsz);
> +            }
> +
> +            if (ret < 0) {
> +                error_report("error while reading from input image file: %s",
> +                             strerror(-ret));
> +                ret = -1;
> +                goto out;
> +            }
> +            int out_ret = blk_pwrite(blk2, count, in.buf, ret, 0);
> +
> +            if (out_ret < 0) {
> +                error_report("error while writing to output image file: %s",
> +                             strerror(-out_ret));
> +                ret = -1;
> +                goto out;
> +            }
> +        }
> +
> +    } else {
> +        error_report("Must specify both input and output files");
> +        ret = -1;
> +        goto out;
> +    }
> +out:
> +    g_free(arg);
> +    qemu_opts_del(opts);
> +    qemu_opts_free(create_opts);
> +    blk_unref(blk1);
> +    blk_unref(blk2);
> +    g_free(in.buf);
> +    g_free(out.buf);
> +
> +    if (ret) {
> +        return 1;
> +    }
> +    return 0;
> +}
> +
>  
>  static const img_cmd_t img_cmds[] = {
>  #define DEF(option, callback, arg_string)        \
> -- 
> 2.9.0
> 

Fam



reply via email to

[Prev in Thread] Current Thread [Next in Thread]