Re: [PATCH for 5.2 1/1] qemu-io: add -V flag for read sub-command

[Denis V. Lunev]

On 8/10/20 5:40 PM, Kevin Wolf wrote:
> Am 10.08.2020 um 16:02 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 10.08.2020 15:35, Denis V. Lunev wrote:
>>> The problem this patch is trying to address is libguestfs behavior on the
>>> appliance startup. It starts supporting to use root=UUID definition in
>>> the kernel command line of its root filesystem using
>>>      file --  /usr/lib64/guestfs/appliance/root
>>> This works fine with RAW image, but we are using QCOW2 as a storage to
>>> save a bit of file space and in this case we get
>>>      QEMU QCOW Image (v3), 1610612736 bytes
>>> instead of UUID of the root filesystem.
>>>
>>> The solution is very simple - we should dump first 256k of the image file
>>> like the follows
>>>      qemu-io -c "read -V 0 256k" appliance | file -
>>> which will provide correct result for all possible types of the appliance
>>> storage.
>>>
>>> Unfortunately, additional option for qemu-io is the only and the simplest
>>> solution as '-v' creates very specific output, which requires to be
>>> parsed. 'qemu-img dd of=/dev/stdout' does not work and the fix would be
>>> much more intrusive.
>>>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> CC: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> CC: Kevin Wolf <kwolf@redhat.com>
>>> CC: Max Reitz <mreitz@redhat.com>
>>> CC: Richard W.M. Jones <rjones@redhat.com>
>>> ---
>>> P.S. Patch to libguestfs will follow.
>>>
>>>   qemu-io-cmds.c | 17 +++++++++++++----
>>>   1 file changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>>> index baeae86d8c..7aae9726cd 100644
>>> --- a/qemu-io-cmds.c
>>> +++ b/qemu-io-cmds.c
>>> @@ -718,7 +718,7 @@ static const cmdinfo_t read_cmd = {
>>>       .cfunc      = read_f,
>>>       .argmin     = 2,
>>>       .argmax     = -1,
>>> -    .args       = "[-abCqv] [-P pattern [-s off] [-l len]] off len",
>>> +    .args       = "[-abCqvV] [-P pattern [-s off] [-l len]] off len",
>>>       .oneline    = "reads a number of bytes at a specified offset",
>>>       .help       = read_help,
>>>   };
>>> @@ -728,6 +728,7 @@ static int read_f(BlockBackend *blk, int argc, char 
>>> **argv)
>>>       struct timespec t1, t2;
>>>       bool Cflag = false, qflag = false, vflag = false;
>>>       bool Pflag = false, sflag = false, lflag = false, bflag = false;
>>> +    bool vrawflag = true;
>>>       int c, cnt, ret;
>>>       char *buf;
>>>       int64_t offset;
>>> @@ -737,7 +738,7 @@ static int read_f(BlockBackend *blk, int argc, char 
>>> **argv)
>>>       int pattern = 0;
>>>       int64_t pattern_offset = 0, pattern_count = 0;
>>> -    while ((c = getopt(argc, argv, "bCl:pP:qs:v")) != -1) {
>>> +    while ((c = getopt(argc, argv, "bCl:pP:qs:vV")) != -1) {
>>>           switch (c) {
>>>           case 'b':
>>>               bflag = true;
>>> @@ -777,6 +778,9 @@ static int read_f(BlockBackend *blk, int argc, char 
>>> **argv)
>>>           case 'v':
>>>               vflag = true;
>>>               break;
>>> +        case 'V':
>>> +            vrawflag = true;
>>> +            break;
>>>           default:
>>>               qemuio_command_usage(&read_cmd);
>>>               return -EINVAL;
>>> @@ -869,10 +873,15 @@ static int read_f(BlockBackend *blk, int argc, char 
>>> **argv)
>>>       if (vflag) {
>>>           dump_buffer(buf, offset, count);
>>>       }
>>> +    if (vrawflag) {
>>> +        write(STDOUT_FILENO, buf, count);
>>> +    }
>>>       /* Finally, report back -- -C gives a parsable format */
>>> -    t2 = tsub(t2, t1);
>>> -    print_report("read", &t2, offset, count, total, cnt, Cflag);
>>> +    if (!vrawflag) {
>>> +        t2 = tsub(t2, t1);
>>> +        print_report("read", &t2, offset, count, total, cnt, Cflag);
>>> +    }
>>>   out:
>>>       qemu_io_free(buf);
>>>
>> I think -v and -V should be mutually exclusive, as combined output doesn't 
>> make real sense.
>> Still, I'm OK with it as is (as well as with -V renamed to -r). I can 
>> suggest also -d (aka dump).
> I like -d (maybe with an alias --dump), though in the end the naming is
> secondary. I think having some flag like this is very useful.
>
> How about adding the same thing to write, i.e. get the buffer to write
> from stdin?

no prob, will do :)

Den