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

[Stefano Garzarella]

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.

>  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;
        }

Thanks,
Stefano