Re: [Qemu-devel] [Qemu-block] [PATCH v4] qemu-io: add pattern file for write command

[Denis Plotnikov]

On 30.05.2019 11:26, Stefano Garzarella wrote:
> On Thu, May 30, 2019 at 11:10:55AM +0300, Denis Plotnikov wrote:
>> The patch allows to provide a pattern file for write
>> command. There was no similar ability before.
>>
>> Signed-off-by: Denis Plotnikov <address@hidden>
>> ---
> 
> Hi Denis,
> for next versions I suggest to describe here, after the ---, the changes
> with previous versions. In this way the review should be easier.
---
Sure
> 
>>   qemu-io-cmds.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 75 insertions(+), 6 deletions(-)
>>
>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>> index 09750a23ce..1c6a5e4faf 100644
>> --- a/qemu-io-cmds.c
>> +++ b/qemu-io-cmds.c
>> @@ -21,6 +21,7 @@
>>   #include "qemu/option.h"
>>   #include "qemu/timer.h"
>>   #include "qemu/cutils.h"
>> +#include "string.h"
>>   
>>   #define CMD_NOFILE_OK   0x01
>>   
>> @@ -343,6 +344,61 @@ static void *qemu_io_alloc(BlockBackend *blk, size_t 
>> len, int pattern)
>>       return buf;
>>   }
>>   
>> +static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,
>> +                                     char *file_name)
>> +{
>> +    char *buf, *buf_pos;
>> +    FILE *f = fopen(file_name, "r");
>> +    int l;
>> +
>> +    if (!f) {
>> +        printf("'%s': %s\n", file_name, strerror(errno));
>> +        return NULL;
>> +    }
>> +
>> +    if (qemuio_misalign) {
>> +        len += MISALIGN_OFFSET;
>> +    }
>> +    buf = blk_blockalign(blk, len);
>> +    memset(buf, 0, len);
>> +
>> +    buf_pos = buf;
>> +
>> +    while (len > 0) {
>> +        l = fread(buf_pos, sizeof(char), len, f);
>> +
>> +        if (feof(f)) {
>> +            rewind(f);
>> +        }
>> +
>> +        if (ferror(f)) {
>> +            printf("'%s': %s\n", file_name, strerror(errno));
>> +            goto error;
>> +        }
>> +
>> +        if (l == 0) {
>> +            printf("'%s' is empty\n", file_name);
>> +            goto error;
>> +        }
> 
> Could it happen that we read some bytes, than we reached the EOF, so we made
> the rewind() and fread() returns 0?
> If that can happen, maybe we should change it in this way:
> 
>          if (l == 0 && buf_pos == buf) {
>              printf("'%s' is empty\n", file_name);
>              goto error;
>          }
This shouldn't happen. To get that situation we need to read exactly the 
file length (from the current position to the end) on the first step, 
then rewind, read again and in that case get l == 0 and feof == true. 
But reading the exact length means that we asked for that size, so the 
buffer is fully populated, thus len == 0 and we leave the loop.

So, l == 0 is only when we read an empty file on the first iteration.


> 
> Thanks,
> Stefano
> 

-- 
Best,
Denis