[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option |
Date: |
Thu, 16 Aug 2018 04:20:40 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-08-15 04:56, Eric Blake wrote:
> For feature parity with dd, we want to be able to specify
> the offset within the output file, just as we can specify
> the offset for the input (in particular, this makes copying
> a subset range of guest-visible bytes from one file to
> another much easier).
In my opinion, we do not want feature parity with dd. What we do want
is feature parity with convert.
> The code style for 'qemu-img dd' was pretty hard to read;
> unfortunately this patch focuses only on adding the new
> feature in the existing style rather than trying to improve
> the overall flow, other than switching octal constants to
> hex. Oh well.
No, the real issue is that dd is still not implemented just as a
frontend to convert. Which it should be. I'm not sure dd was a very
good idea from the start, and now it should ideally be a frontend to
convert.
(My full opinion on the matter: dd has a horrible interface. I don't
quite see why we replicated that inside qemu-img. Also, if you want to
use dd, why not use qemu-nbd + Linux nbd device + real dd?)
((That gave me a good idea. Actually, it's probably not such a good
idea, but I guess I'll do it in my spare time anyway. A qemu-img fuse
might be nice which represents an image as a raw image at some mount
point. Benefits over qemu-nbd: (1) You don't need root, (2) you don't
need to type modprobe nbd.))
> Also, switch the test to use an offset of 0 instead of 1,
> to test skip= and seek= on their own; as it is, this is
> effectively quadrupling the test runtime, which starts
> to make this test borderline on whether it should still
> belong to './check -g quick'. And I didn't bother to
> reindent the test shell code for the new nested loop.
In my opinion, it should no longer belong to quick. It takes 8 s on my
tmpfs. My border is somewhere around 2 or 3; and I haven't yet decided
whether that's on tmpfs or SSD.
> Signed-off-by: Eric Blake <address@hidden>
> ---
> qemu-img.c | 41 ++++--
> tests/qemu-iotests/160 | 12 +-
> tests/qemu-iotests/160.out | 304
> +++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 336 insertions(+), 21 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index d72f0f0ec94..ee01a18f331 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
[...]
> @@ -4574,7 +4592,14 @@ static int img_dd(int argc, char **argv)
> size = dd.count * in.bsz;
> }
>
> - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);
> + if (dd.flags & C_SEEK && out.offset * out.bsz > INT64_MAX - size) {
What about overflows in out.offset * out.bsz?
> + error_report("Seek too large for '%s'", out.filename);
> + ret = -1;
> + goto out;
Real dd doesn't seem to error out (it just reports an error). I don't
know whether that makes any difference, though.
The test looks good to me.
Max
> + }
> + seek = out.offset * out.bsz;
> +
> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size + seek, &error_abort);
> end = size + in_pos;
>
> ret = bdrv_create(drv, out.filename, opts, &local_err);
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 0/2] Improve qemu-img dd, Eric Blake, 2018/08/14
- [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Eric Blake, 2018/08/14
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option,
Max Reitz <=
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Eric Blake, 2018/08/15
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Eric Blake, 2018/08/15
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Max Reitz, 2018/08/15
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Eric Blake, 2018/08/15
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Max Reitz, 2018/08/15
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Kevin Wolf, 2018/08/16
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Max Reitz, 2018/08/17
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Fam Zheng, 2018/08/19
- Re: [Qemu-devel] [PATCH 2/2] qemu-img: Add dd seek= option, Max Reitz, 2018/08/20
Re: [Qemu-devel] [PATCH 0/2] Improve qemu-img dd, Eric Blake, 2018/08/15