[Qemu-devel] [PATCH v2 00/10] FORTIFY_SOURCE followup

[Qemu-devel] [PATCH v2 00/10] FORTIFY_SOURCE followup

[Juan Quintela]

Hi

Series updated with yesterday comments

v2:
   - drop un-needed slirp exit() patch (kevin)
   - add qemu_write_full() documentation (danp)
   - use strerror(-errno) (kevin, pbonzini)

Please review and apply.

Later, Juan.


v1:
This series make:
- all block *_create() functions return -errno instead of -1
- this makes that we can end writting errno/error at bdrv_create()
   callers (qemu-img)
- once there found a double free problem in the error handling of
  vmdk, fixed it.
- slirp: also check that system() was able to fork (amit noticed it)
- daemonize: if we are unable to write into the pipe, print a message and exit.
  We can't really recover from that error (amit noticed it).

Juan Quintela (10):
  cow: return errno instead of -1
  slirp: check system() success
  qcow2: return errno instead of -1
  qcow: return errno instead of -1
  vmdk: return errno instead of -1
  vmdk: make vmdk_snapshot_create return -errno
  vmdk: fix double free
  vmdk: share cleanup code
  block: print errno on error
  documentation: qemu_write_full don't work with non-blocking fd's

 block/cow.c   |    5 +--
 block/qcow.c  |    8 ++--
 block/qcow2.c |   18 +++++-----
 block/vmdk.c  |  106 +++++++++++++++++++++++++++++++++++++--------------------
 net/slirp.c   |    2 +-
 osdep.c       |    5 +++
 qemu-img.c    |    4 +-
 7 files changed, 92 insertions(+), 56 deletions(-)

[Qemu-devel] [PATCH 01/10] cow: return errno instead of -1

[Juan Quintela]

Remove not needed ret = 0 assignment.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/cow.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 3733385..97e9745 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -224,7 +224,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
               0644);
     if (cow_fd < 0)
-        return -1;
+        return -errno;
     memset(&cow_header, 0, sizeof(cow_header));
     cow_header.magic = cpu_to_be32(COW_MAGIC);
     cow_header.version = cpu_to_be32(COW_VERSION);
@@ -251,7 +251,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     cow_header.size = cpu_to_be64(image_sectors * 512);
     ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header));
     if (ret != sizeof(cow_header)) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

@@ -262,7 +262,6 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
         goto exit;
     }

-    ret = 0;
 exit:
     close(cow_fd);
     return ret;
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH 01/10] cow: return errno instead of -1

[Anthony Liguori]

On 03/04/2010 03:00 AM, Juan Quintela wrote:
> Remove not needed ret = 0 assignment.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
>    

Applied all.  Thanks.

Regards,

Anthony Liguori

> ---
>   block/cow.c |    5 ++---
>   1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/block/cow.c b/block/cow.c
> index 3733385..97e9745 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -224,7 +224,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
>       cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
>                 0644);
>       if (cow_fd<  0)
> -        return -1;
> +        return -errno;
>       memset(&cow_header, 0, sizeof(cow_header));
>       cow_header.magic = cpu_to_be32(COW_MAGIC);
>       cow_header.version = cpu_to_be32(COW_VERSION);
> @@ -251,7 +251,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
>       cow_header.size = cpu_to_be64(image_sectors * 512);
>       ret = qemu_write_full(cow_fd,&cow_header, sizeof(cow_header));
>       if (ret != sizeof(cow_header)) {
> -        ret = -1;
> +        ret = -errno;
>           goto exit;
>       }
>
> @@ -262,7 +262,6 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
>           goto exit;
>       }
>
> -    ret = 0;
>   exit:
>       close(cow_fd);
>       return ret;
>    

[Qemu-devel] [PATCH 02/10] slirp: check system() success

[Juan Quintela]

we shouldn't call W*() macros until we check that fork worked.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 net/slirp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 317cca7..7f846ec 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -472,7 +472,7 @@ static void slirp_smb_cleanup(SlirpState *s)
     if (s->smb_dir[0] != '\0') {
         snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
         ret = system(cmd);
-        if (!WIFEXITED(ret)) {
+        if (ret == -1 || !WIFEXITED(ret)) {
             qemu_error("'%s' failed.\n", cmd);
         } else if (WEXITSTATUS(ret)) {
             qemu_error("'%s' failed. Error code: %d\n",
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 02/10] slirp: check system() success

[Michael S. Tsirkin]

On Thu, Mar 04, 2010 at 10:00:31AM +0100, Juan Quintela wrote:
> we shouldn't call W*() macros until we check that fork worked.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  net/slirp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 317cca7..7f846ec 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -472,7 +472,7 @@ static void slirp_smb_cleanup(SlirpState *s)
>      if (s->smb_dir[0] != '\0') {
>          snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
>          ret = system(cmd);
> -        if (!WIFEXITED(ret)) {
> +        if (ret == -1 || !WIFEXITED(ret)) {
>              qemu_error("'%s' failed.\n", cmd);
>          } else if (WEXITSTATUS(ret)) {
>              qemu_error("'%s' failed. Error code: %d\n",
> -- 
> 1.6.6.1
> 
> 

[Qemu-devel] [PATCH 03/10] qcow2: return errno instead of -1

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/qcow2.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index bf8170e..5b6dad9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -851,7 +851,7 @@ static int qcow_create2(const char *filename, int64_t total_size,

     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
     if (fd < 0)
-        return -1;
+        return -errno;
     memset(&header, 0, sizeof(header));
     header.magic = cpu_to_be32(QCOW_MAGIC);
     header.version = cpu_to_be32(QCOW_VERSION);
@@ -930,7 +930,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
     /* write all the data */
     ret = qemu_write_full(fd, &header, sizeof(header));
     if (ret != sizeof(header)) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }
     if (backing_file) {
@@ -943,25 +943,25 @@ static int qcow_create2(const char *filename, int64_t total_size,
             cpu_to_be32s(&ext_bf.len);
             ret = qemu_write_full(fd, &ext_bf, sizeof(ext_bf));
             if (ret != sizeof(ext_bf)) {
-                ret = -1;
+                ret = -errno;
                 goto exit;
             }
             ret = qemu_write_full(fd, backing_format, backing_format_len);
             if (ret != backing_format_len) {
-                ret = -1;
+                ret = -errno;
                 goto exit;
             }
             if (padding > 0) {
                 ret = qemu_write_full(fd, zero, padding);
                 if (ret != padding) {
-                    ret = -1;
+                    ret = -errno;
                     goto exit;
                 }
             }
         }
         ret = qemu_write_full(fd, backing_file, backing_filename_len);
         if (ret != backing_filename_len) {
-            ret = -1;
+            ret = -errno;
             goto exit;
         }
     }
@@ -970,14 +970,14 @@ static int qcow_create2(const char *filename, int64_t total_size,
     for(i = 0;i < l1_size; i++) {
         ret = qemu_write_full(fd, &tmp, sizeof(tmp));
         if (ret != sizeof(tmp)) {
-            ret = -1;
+            ret = -errno;
             goto exit;
         }
     }
     lseek(fd, s->refcount_table_offset, SEEK_SET);
     ret = qemu_write_full(fd, s->refcount_table, s->cluster_size);
     if (ret != s->cluster_size) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

@@ -985,7 +985,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
     ret = qemu_write_full(fd, s->refcount_block,
 		    ref_clusters * s->cluster_size);
     if (ret != ref_clusters * s->cluster_size) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

-- 
1.6.6.1

[Qemu-devel] [PATCH 04/10] qcow: return errno instead of -1

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/qcow.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 003db1e..c619984 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -766,7 +766,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)

     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
     if (fd < 0)
-        return -1;
+        return -errno;
     memset(&header, 0, sizeof(header));
     header.magic = cpu_to_be32(QCOW_MAGIC);
     header.version = cpu_to_be32(QCOW_VERSION);
@@ -804,14 +804,14 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
     /* write all the data */
     ret = qemu_write_full(fd, &header, sizeof(header));
     if (ret != sizeof(header)) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

     if (backing_file) {
         ret = qemu_write_full(fd, backing_file, backing_filename_len);
         if (ret != backing_filename_len) {
-            ret = -1;
+            ret = -errno;
             goto exit;
         }

@@ -821,7 +821,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
     for(i = 0;i < l1_size; i++) {
         ret = qemu_write_full(fd, &tmp, sizeof(tmp));
         if (ret != sizeof(tmp)) {
-            ret = -1;
+            ret = -errno;
             goto exit;
         }
     }
-- 
1.6.6.1

[Qemu-devel] [PATCH 05/10] vmdk: return errno instead of -1

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/vmdk.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 56c28a0..5b1d197 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -740,7 +740,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
               0644);
     if (fd < 0)
-        return -1;
+        return -errno;
     magic = cpu_to_be32(VMDK4_MAGIC);
     memset(&header, 0, sizeof(header));
     header.version = cpu_to_le32(1);
@@ -777,18 +777,18 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     /* write all the data */
     ret = qemu_write_full(fd, &magic, sizeof(magic));
     if (ret != sizeof(magic)) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }
     ret = qemu_write_full(fd, &header, sizeof(header));
     if (ret != sizeof(header)) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

     ret = ftruncate(fd, header.grain_offset << 9);
     if (ret < 0) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

@@ -798,7 +798,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
          i < gt_count; i++, tmp += gt_size) {
         ret = qemu_write_full(fd, &tmp, sizeof(tmp));
         if (ret != sizeof(tmp)) {
-            ret = -1;
+            ret = -errno;
             goto exit;
         }
     }
@@ -809,7 +809,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
          i < gt_count; i++, tmp += gt_size) {
         ret = qemu_write_full(fd, &tmp, sizeof(tmp));
         if (ret != sizeof(tmp)) {
-            ret = -1;
+            ret = -errno;
             goto exit;
         }
     }
@@ -831,7 +831,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET);
     ret = qemu_write_full(fd, desc, strlen(desc));
     if (ret != strlen(desc)) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

-- 
1.6.6.1

[Qemu-devel] [PATCH 06/10] vmdk: make vmdk_snapshot_create return -errno

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/vmdk.c |   79 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 5b1d197..67a690e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -187,6 +187,7 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
 static int vmdk_snapshot_create(const char *filename, const char *backing_file)
 {
     int snp_fd, p_fd;
+    int ret;
     uint32_t p_cid;
     char *p_name, *gd_buf, *rgd_buf;
     const char *real_filename, *temp_str;
@@ -211,35 +212,49 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)

     snp_fd = open(filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644);
     if (snp_fd < 0)
-        return -1;
+        return -errno;
     p_fd = open(backing_file, O_RDONLY | O_BINARY | O_LARGEFILE);
     if (p_fd < 0) {
         close(snp_fd);
-        return -1;
+        return -errno;
     }

     /* read the header */
-    if (lseek(p_fd, 0x0, SEEK_SET) == -1)
+    if (lseek(p_fd, 0x0, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail;
-    if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE)
+    }
+    if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE) {
+        ret = -errno;
         goto fail;
+    }

     /* write the header */
-    if (lseek(snp_fd, 0x0, SEEK_SET) == -1)
+    if (lseek(snp_fd, 0x0, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail;
-    if (write(snp_fd, hdr, HEADER_SIZE) == -1)
+    }
+    if (write(snp_fd, hdr, HEADER_SIZE) == -1) {
+        ret = -errno;
         goto fail;
+    }

     memset(&header, 0, sizeof(header));
     memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC

-    if (ftruncate(snp_fd, header.grain_offset << 9))
+    if (ftruncate(snp_fd, header.grain_offset << 9)) {
+        ret = -errno;
         goto fail;
+    }
     /* the descriptor offset = 0x200 */
-    if (lseek(p_fd, 0x200, SEEK_SET) == -1)
+    if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail;
-    if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE)
+    }
+    if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE) {
+        ret = -errno;
         goto fail;
+    }

     if ((p_name = strstr(p_desc,"CID")) != NULL) {
         p_name += sizeof("CID");
@@ -258,10 +273,14 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
              (uint32_t)header.capacity, real_filename);

     /* write the descriptor */
-    if (lseek(snp_fd, 0x200, SEEK_SET) == -1)
+    if (lseek(snp_fd, 0x200, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail;
-    if (write(snp_fd, s_desc, strlen(s_desc)) == -1)
+    }
+    if (write(snp_fd, s_desc, strlen(s_desc)) == -1) {
+        ret = -errno;
         goto fail;
+    }

     gd_offset = header.gd_offset * SECTOR_SIZE;     // offset of GD table
     rgd_offset = header.rgd_offset * SECTOR_SIZE;   // offset of RGD table
@@ -271,33 +290,51 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
      * 512 GTE per GT, each GTE points to grain
      */
     gt_size = (int64_t)header.num_gtes_per_gte * header.granularity * SECTOR_SIZE;
-    if (!gt_size)
+    if (!gt_size) {
+        ret = -EINVAL;
         goto fail;
+    }
     gde_entries = (uint32_t)(capacity / gt_size);  // number of gde/rgde
     gd_size = gde_entries * sizeof(uint32_t);

     /* write RGD */
     rgd_buf = qemu_malloc(gd_size);
-    if (lseek(p_fd, rgd_offset, SEEK_SET) == -1)
+    if (lseek(p_fd, rgd_offset, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail_rgd;
-    if (read(p_fd, rgd_buf, gd_size) != gd_size)
+    }
+    if (read(p_fd, rgd_buf, gd_size) != gd_size) {
+        ret = -errno;
         goto fail_rgd;
-    if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1)
+    }
+    if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail_rgd;
-    if (write(snp_fd, rgd_buf, gd_size) == -1)
+    }
+    if (write(snp_fd, rgd_buf, gd_size) == -1) {
+        ret = -errno;
         goto fail_rgd;
+    }
     qemu_free(rgd_buf);

     /* write GD */
     gd_buf = qemu_malloc(gd_size);
-    if (lseek(p_fd, gd_offset, SEEK_SET) == -1)
+    if (lseek(p_fd, gd_offset, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail_gd;
-    if (read(p_fd, gd_buf, gd_size) != gd_size)
+    }
+    if (read(p_fd, gd_buf, gd_size) != gd_size) {
+        ret = -errno;
         goto fail_gd;
-    if (lseek(snp_fd, gd_offset, SEEK_SET) == -1)
+    }
+    if (lseek(snp_fd, gd_offset, SEEK_SET) == -1) {
+        ret = -errno;
         goto fail_gd;
-    if (write(snp_fd, gd_buf, gd_size) == -1)
+    }
+    if (write(snp_fd, gd_buf, gd_size) == -1) {
+        ret = -errno;
         goto fail_gd;
+    }
     qemu_free(gd_buf);

     close(p_fd);
@@ -311,7 +348,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
     fail:
     close(p_fd);
     close(snp_fd);
-    return -1;
+    return ret;
 }

 static void vmdk_parent_close(BlockDriverState *bs)
-- 
1.6.6.1

[Qemu-devel] [PATCH 07/10] vmdk: fix double free

[Juan Quintela]

fail_gd error case would also free rgd_buf that was already freed

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/vmdk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 67a690e..819c1c9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -315,7 +315,6 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
         ret = -errno;
         goto fail_rgd;
     }
-    qemu_free(rgd_buf);

     /* write GD */
     gd_buf = qemu_malloc(gd_size);
@@ -336,6 +335,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
         goto fail_gd;
     }
     qemu_free(gd_buf);
+    qemu_free(rgd_buf);

     close(p_fd);
     close(snp_fd);
-- 
1.6.6.1

[Qemu-devel] [PATCH 08/10] vmdk: share cleanup code

[Juan Quintela]

cleanup code is identical for error/success cases.  Only difference
are goto labels.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/vmdk.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 819c1c9..007fca4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -334,18 +334,13 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
         ret = -errno;
         goto fail_gd;
     }
-    qemu_free(gd_buf);
-    qemu_free(rgd_buf);
-
-    close(p_fd);
-    close(snp_fd);
-    return 0;
+    ret = 0;

-    fail_gd:
+fail_gd:
     qemu_free(gd_buf);
-    fail_rgd:
+fail_rgd:
     qemu_free(rgd_buf);
-    fail:
+fail:
     close(p_fd);
     close(snp_fd);
     return ret;
-- 
1.6.6.1

[Qemu-devel] [PATCH 09/10] block: print errno on error

[Juan Quintela]

Now that we changed all create calls to return errno, just print it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 qemu-img.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0c9f2d4..e51b40c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -374,7 +374,7 @@ static int img_create(int argc, char **argv)
         } else if (ret == -EFBIG) {
             error("The image size is too large for file format '%s'", fmt);
         } else {
-            error("Error while formatting");
+            error("%s: error while creating %s: %s", filename, fmt, strerror(-ret));
         }
     }
     return 0;
@@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv)
         } else if (ret == -EFBIG) {
             error("The image size is too large for file format '%s'", out_fmt);
         } else {
-            error("Error while formatting '%s'", out_filename);
+            error("%s: error while converting %s: %s", out_filename, out_fmt, strerror(-ret));
         }
     }

-- 
1.6.6.1

[Qemu-devel] [PATCH 10/10] documentation: qemu_write_full don't work with non-blocking fd's

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 osdep.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/osdep.c b/osdep.c
index 5bf3c00..abbc8a2 100644
--- a/osdep.c
+++ b/osdep.c
@@ -261,6 +261,11 @@ int qemu_open(const char *name, int flags, ...)
  *
  * Return the number of bytes transferred.
  * Set errno if fewer than `count' bytes are written.
+ *
+ * This function don't work with non-blocking fd's.
+ * Any of the possibilities with non-bloking fd's is bad:
+ *   - return a short write (then name is wrong)
+ *   - busy wait adding (errno == EAGAIN) to the loop
  */
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 {
-- 
1.6.6.1

[Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup

[Juan Quintela]

Hi

This series make:
- all block *_create() functions return -errno instead of -1
- this makes that we can end writting errno/error at bdrv_create()
   callers (qemu-img)
- once there found a double free problem in the error handling of
  vmdk, fixed it.
- slirp: also check that system() was able to fork (amit noticed it)
- daemonize: if we are unable to write into the pipe, print a message and exit.
  We can't really recover from that error (amit noticed it).

Please review and apply.

Later, Juan.

Juan Quintela (10):
  cow: return errno instead of -1
  slirp: check system() success
  qcow2: return errno instead of -1
  qcow: return errno instead of -1
  vmdk: return errno instead of -1
  vl: exit if we are not able to write into the pipe
  vmdk: make vmdk_snapshot_create return -errno
  vmdk: fix double free
  vmdk: share cleanup code
  block: print errno on error

 block/cow.c   |    5 +--
 block/qcow.c  |    8 ++--
 block/qcow2.c |   18 +++++-----
 block/vmdk.c  |  106 +++++++++++++++++++++++++++++++++++++--------------------
 net/slirp.c   |    2 +-
 qemu-img.c    |    4 +-
 vl.c          |    1 +
 7 files changed, 88 insertions(+), 56 deletions(-)

[Qemu-devel] [PATCH 01/10] cow: return errno instead of -1

[Juan Quintela]

Remove not needed ret = 0 assignment.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/cow.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 3733385..97e9745 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -224,7 +224,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
               0644);
     if (cow_fd < 0)
-        return -1;
+        return -errno;
     memset(&cow_header, 0, sizeof(cow_header));
     cow_header.magic = cpu_to_be32(COW_MAGIC);
     cow_header.version = cpu_to_be32(COW_VERSION);
@@ -251,7 +251,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     cow_header.size = cpu_to_be64(image_sectors * 512);
     ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header));
     if (ret != sizeof(cow_header)) {
-        ret = -1;
+        ret = -errno;
         goto exit;
     }

@@ -262,7 +262,6 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
         goto exit;
     }

-    ret = 0;
 exit:
     close(cow_fd);
     return ret;
-- 
1.6.5.2