[Qemu-devel] [PATCH] raw-posix: Handle errors in raw_create

[Qemu-devel] [PATCH] raw-posix: Handle errors in raw_create

[Stefan Weil]

In qemu-iotests, some large images are created using qemu-img.

Without checks for errors, qemu-img will just create an
empty image, and later read / write tests will fail.

With the patch, failures during image creation are detected
and reported.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 block/raw-posix.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index fa4f83e..389903e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -852,6 +852,7 @@ again:
 static int raw_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd;
+    int result = 0;
     int64_t total_size = 0;
 
     /* Read out options */
@@ -864,11 +865,17 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
 
     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
               0644);
-    if (fd < 0)
-        return -EIO;
-    ftruncate(fd, total_size * 512);
-    close(fd);
-    return 0;
+    if (fd < 0) {
+        result = -errno;
+    } else {
+        if (ftruncate(fd, total_size * 512) != 0) {
+            result = -errno;
+        }
+        if (close(fd) != 0) {
+            result = -errno;
+        }
+    }
+    return result;
 }
 
 static void raw_flush(BlockDriverState *bs)
-- 
1.5.6.5

Re: [Qemu-devel] [PATCH] raw-posix: Handle errors in raw_create

[Christoph Hellwig]

On Sat, Jul 11, 2009 at 04:43:37PM +0200, Stefan Weil wrote:
> In qemu-iotests, some large images are created using qemu-img.
> 
> Without checks for errors, qemu-img will just create an
> empty image, and later read / write tests will fail.
> 
> With the patch, failures during image creation are detected
> and reported.

Yeah, we should handle the failures and your patch looks correct in
that respect.  But returning close error codes is in general
not very useful.  There's not much we can do about them, in they
might override the more useful ftruncate error code.

So I'd rather do something like the following:

    if (fd < 0) {
        return -errno;
    }
 
    if (ftruncate(fd, total_size * 512) != 0) {
        result = -errno;
    }

    close(fd);
    return result;

Re: [Qemu-devel] [PATCH] raw-posix: Handle errors in raw_create

[Stefan Weil]

Christoph Hellwig schrieb:
> On Sat, Jul 11, 2009 at 04:43:37PM +0200, Stefan Weil wrote:
>> In qemu-iotests, some large images are created using qemu-img.
>>
>> Without checks for errors, qemu-img will just create an
>> empty image, and later read / write tests will fail.
>>
>> With the patch, failures during image creation are detected
>> and reported.
>
> Yeah, we should handle the failures and your patch looks correct in
> that respect. But returning close error codes is in general
> not very useful. There's not much we can do about them, in they
> might override the more useful ftruncate error code.
>
> So I'd rather do something like the following:
>
> if (fd < 0) {
> return -errno;
> }
>
> if (ftruncate(fd, total_size * 512) != 0) {
> result = -errno;
> }
>
> close(fd);
> return result;

A useful ftruncate error code won't be overwritten by a close error
in most cases because it's not very likely to get both errors together.
For the rare case where ftruncate and close both fail and set different
errno values, I'd accept to get the error code from close.

Here is an extract from the man page of close:

Not checking the return value of close() is a common  but  nevertheless
serious  programming error. ...
Not checking the return value when closing the file may lead to silent
loss of data. ...

The patch works: on disks with less than 128 MB free space
qemu-iotests will give better diagnostics, because qemu-img
won't silently create an image of length 0.

Regards,
Stefan