[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qemu_file: use fwrite() correctly
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2] qemu_file: use fwrite() correctly |
Date: |
Thu, 27 Feb 2014 08:58:08 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Juan Quintela <address@hidden> writes:
> fwrite() returns the number of items written. But when there is one
> error, it can return a short write.
>
> In the particular bug that I was tracking, I did a migration to a
> read-only filesystem. And it was able to finish the migration
> correctly. fwrite() never returned a negative error code, the 1st
> time it returns 0, after that it returns 4096. (migration writes
> chunks of about 14000 bytes). And it was able to "complete" the
> migration with success (yes, reading the file was a bit more
> difficult). On the 1st fwrite() for the read-only filesystem,
> it returns an errno of -EPIPE, that is exactly what has failed.
>
> To add insult to injury, if your amount of memory was big enough (12GB
> on my case), it overwrote some important structure, and from them,
> malloc failed. This check makes the problem go away.
>
> Signed-off-by: Juan Quintela <address@hidden>
> ---
>
> v2: a.k.a Paolo was right
>
> On the first call to fwrite() it returns 0, and errno is setup to
> EPIPE, exactly what we wanted.
>
> Once here, improve the commit message.
>
> qemu-file.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-file.c b/qemu-file.c
> index 9473b67..e5ec798 100644
> --- a/qemu-file.c
> +++ b/qemu-file.c
> @@ -100,7 +100,14 @@ static int stdio_put_buffer(void *opaque, const uint8_t
> *buf, int64_t pos,
> int size)
> {
> QEMUFileStdio *s = opaque;
> - return fwrite(buf, 1, size, s->stdio_file);
> + int res;
> +
> + res = fwrite(buf, 1, size, s->stdio_file);
> +
> + if (res != size) {
> + return -errno;
> + }
> + return res;
> }
>
> static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int
> size)
This is what got an R-by from Eric and me. And this is what Peter
pulled from Juan:
diff --git a/qemu-file.c b/qemu-file.c
index 9473b67..f074af1 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -100,7 +100,14 @@ static int stdio_put_buffer(void *opaque, const uint8_t
*buf, int64_t pos,
int size)
{
QEMUFileStdio *s = opaque;
- return fwrite(buf, 1, size, s->stdio_file);
+ int res;
+
+ res = fwrite(buf, 1, size, s->stdio_file);
+
+ if (res != size) {
+ return -EIO; /* fake errno value */
+ }
+ return res;
}
Follow-up fix sent.