All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate()
@ 2017-03-28 20:51 Max Reitz
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 1/4] block/vhdx: Make vhdx_create() always set errp Max Reitz
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Max Reitz @ 2017-03-28 20:51 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Stefan Hajnoczi, Eric Blake

Having an Error parameter for these functions makes sense because we
sometimes want a bit more information than just "Something failed". Some
drivers already use error_report() and the like to emit this additional
information, so it's rather obvious that we do want a real error object
here.


v3:
- Patch 2: Keep "Could not resize image" message in qcow2_create2() by
           using error_prepend() [Kevin]
- Patch 3: Dropped archipelago
- Patch 4:
  - Keep errno information where available [Kevin]
  - Make all drivers generate error messages [Stefan/Eric]
  - Drop generic error message from bdrv_truncate() [Stefan/Eric]


Max Reitz (4):
  block/vhdx: Make vhdx_create() always set errp
  block: Add errp to b{lk,drv}_truncate()
  block: Add errp to BD.bdrv_truncate()
  block: Add .bdrv_truncate() error messages

 include/block/block.h          |  2 +-
 include/block/block_int.h      |  2 +-
 include/sysemu/block-backend.h |  2 +-
 block.c                        | 16 +++++++++++-----
 block/blkdebug.c               |  4 ++--
 block/block-backend.c          |  5 +++--
 block/commit.c                 |  5 +++--
 block/crypto.c                 |  5 +++--
 block/file-posix.c             | 19 +++++++++++++------
 block/file-win32.c             |  6 +++---
 block/gluster.c                |  7 +++++--
 block/iscsi.c                  |  6 ++++--
 block/mirror.c                 |  2 +-
 block/nfs.c                    | 12 ++++++++++--
 block/parallels.c              | 13 ++++++++-----
 block/qcow.c                   |  6 +++---
 block/qcow2-refcount.c         |  5 ++++-
 block/qcow2.c                  | 24 +++++++++++++++---------
 block/qed.c                    |  8 +++++---
 block/raw-format.c             |  6 ++++--
 block/rbd.c                    |  3 ++-
 block/sheepdog.c               | 14 ++++++--------
 block/vdi.c                    |  4 ++--
 block/vhdx-log.c               |  2 +-
 block/vhdx.c                   | 25 ++++++++++++++++++-------
 block/vmdk.c                   | 13 +++----------
 block/vpc.c                    | 13 +++++++------
 blockdev.c                     | 21 +--------------------
 qemu-img.c                     | 17 ++++-------------
 qemu-io-cmds.c                 |  5 +++--
 30 files changed, 147 insertions(+), 125 deletions(-)

-- 
2.12.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 for-2.10 1/4] block/vhdx: Make vhdx_create() always set errp
  2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
@ 2017-03-28 20:51 ` Max Reitz
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 2/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Max Reitz @ 2017-03-28 20:51 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Stefan Hajnoczi, Eric Blake

This patch makes vhdx_create() always set errp in case of an error. It
also adds errp parameters to vhdx_create_bat() and
vhdx_create_new_region_table() so we can pass on the error object
generated by blk_truncate() as of a future commit.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vhdx.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 052a753159..d25bcd91de 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1586,7 +1586,7 @@ exit:
 static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s,
                            uint64_t image_size, VHDXImageType type,
                            bool use_zero_blocks, uint64_t file_offset,
-                           uint32_t length)
+                           uint32_t length, Error **errp)
 {
     int ret = 0;
     uint64_t data_file_offset;
@@ -1609,14 +1609,19 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s,
          * is the furthest thing we have written yet */
         ret = blk_truncate(blk, data_file_offset);
         if (ret < 0) {
+            error_setg_errno(errp, -ret,
+                            "Failed to resize the underlying file");
             goto exit;
         }
     } else if (type == VHDX_TYPE_FIXED) {
         ret = blk_truncate(blk, data_file_offset + image_size);
         if (ret < 0) {
+            error_setg_errno(errp, -ret,
+                            "Failed to resize the underlying file");
             goto exit;
         }
     } else {
+        error_setg(errp, "Unsupported image type");
         ret = -ENOTSUP;
         goto exit;
     }
@@ -1627,6 +1632,7 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s,
         /* for a fixed file, the default BAT entry is not zero */
         s->bat = g_try_malloc0(length);
         if (length && s->bat == NULL) {
+            error_setg(errp, "Failed to allocate memory for the BAT");
             ret = -ENOMEM;
             goto exit;
         }
@@ -1646,6 +1652,7 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s,
         }
         ret = blk_pwrite(blk, file_offset, s->bat, length, 0);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Failed to write the BAT");
             goto exit;
         }
     }
@@ -1671,7 +1678,8 @@ static int vhdx_create_new_region_table(BlockBackend *blk,
                                         uint32_t log_size,
                                         bool use_zero_blocks,
                                         VHDXImageType type,
-                                        uint64_t *metadata_offset)
+                                        uint64_t *metadata_offset,
+                                        Error **errp)
 {
     int ret = 0;
     uint32_t offset = 0;
@@ -1740,7 +1748,7 @@ static int vhdx_create_new_region_table(BlockBackend *blk,
     /* The region table gives us the data we need to create the BAT,
      * so do that now */
     ret = vhdx_create_bat(blk, s, image_size, type, use_zero_blocks,
-                          bat_file_offset, bat_length);
+                          bat_file_offset, bat_length, errp);
     if (ret < 0) {
         goto exit;
     }
@@ -1749,12 +1757,14 @@ static int vhdx_create_new_region_table(BlockBackend *blk,
     ret = blk_pwrite(blk, VHDX_REGION_TABLE_OFFSET, buffer,
                      VHDX_HEADER_BLOCK_SIZE, 0);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to write first region table");
         goto exit;
     }
 
     ret = blk_pwrite(blk, VHDX_REGION_TABLE2_OFFSET, buffer,
                      VHDX_HEADER_BLOCK_SIZE, 0);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to write second region table");
         goto exit;
     }
 
@@ -1825,6 +1835,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
         ret = -ENOTSUP;
         goto exit;
     } else {
+        error_setg(errp, "Invalid subformat '%s'", type);
         ret = -EINVAL;
         goto exit;
     }
@@ -1879,12 +1890,14 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET, &signature, sizeof(signature),
                      0);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to write file signature");
         goto delete_and_exit;
     }
     if (creator) {
         ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET + sizeof(signature),
                          creator, creator_items * sizeof(gunichar2), 0);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Failed to write creator field");
             goto delete_and_exit;
         }
     }
@@ -1893,13 +1906,14 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     /* Creates (B),(C) */
     ret = vhdx_create_new_headers(blk, image_size, log_size);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to write image headers");
         goto delete_and_exit;
     }
 
     /* Creates (D),(E),(G) explicitly. (F) created as by-product */
     ret = vhdx_create_new_region_table(blk, image_size, block_size, 512,
                                        log_size, use_zero_blocks, image_type,
-                                       &metadata_offset);
+                                       &metadata_offset, errp);
     if (ret < 0) {
         goto delete_and_exit;
     }
@@ -1908,6 +1922,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     ret = vhdx_create_new_metadata(blk, image_size, block_size, 512,
                                    metadata_offset, image_type);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to initialize metadata");
         goto delete_and_exit;
     }
 
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 for-2.10 2/4] block: Add errp to b{lk, drv}_truncate()
  2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 1/4] block/vhdx: Make vhdx_create() always set errp Max Reitz
@ 2017-03-28 20:51 ` Max Reitz
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 3/4] block: Add errp to BD.bdrv_truncate() Max Reitz
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Max Reitz @ 2017-03-28 20:51 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Stefan Hajnoczi, Eric Blake

For one thing, this allows us to drop the error message generation from
qemu-img.c and blockdev.c and instead have it unified in
bdrv_truncate().

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block.h          |  2 +-
 include/sysemu/block-backend.h |  2 +-
 block.c                        | 16 ++++++++++++----
 block/blkdebug.c               |  2 +-
 block/block-backend.c          |  5 +++--
 block/commit.c                 |  5 +++--
 block/crypto.c                 |  2 +-
 block/mirror.c                 |  2 +-
 block/parallels.c              | 13 ++++++++-----
 block/qcow.c                   |  6 +++---
 block/qcow2-refcount.c         |  5 ++++-
 block/qcow2.c                  | 14 +++++++++-----
 block/qed.c                    |  2 +-
 block/raw-format.c             |  2 +-
 block/vdi.c                    |  4 ++--
 block/vhdx-log.c               |  2 +-
 block/vhdx.c                   | 10 +++-------
 block/vmdk.c                   | 13 +++----------
 block/vpc.c                    | 13 +++++++------
 blockdev.c                     | 21 +--------------------
 qemu-img.c                     | 17 ++++-------------
 qemu-io-cmds.c                 |  5 +++--
 22 files changed, 73 insertions(+), 90 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 5149260827..4c9ed0e43c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -294,7 +294,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
     const char *backing_file);
 int bdrv_get_backing_file_depth(BlockDriverState *bs);
 void bdrv_refresh_filename(BlockDriverState *bs);
-int bdrv_truncate(BdrvChild *child, int64_t offset);
+int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp);
 int64_t bdrv_nb_sectors(BlockDriverState *bs);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 7462228ac1..0ba4e277b9 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -225,7 +225,7 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                                       int count, BdrvRequestFlags flags);
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
                           int count);
-int blk_truncate(BlockBackend *blk, int64_t offset);
+int blk_truncate(BlockBackend *blk, int64_t offset, Error **errp);
 int blk_pdiscard(BlockBackend *blk, int64_t offset, int count);
 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
                      int64_t pos, int size);
diff --git a/block.c b/block.c
index 6e906ec53c..9ed526e01d 100644
--- a/block.c
+++ b/block.c
@@ -3222,7 +3222,7 @@ exit:
 /**
  * Truncate file to 'offset' bytes (needed only for file protocols)
  */
-int bdrv_truncate(BdrvChild *child, int64_t offset)
+int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
 {
     BlockDriverState *bs = child->bs;
     BlockDriver *drv = bs->drv;
@@ -3230,12 +3230,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset)
 
     assert(child->perm & BLK_PERM_RESIZE);
 
-    if (!drv)
+    if (!drv) {
+        error_setg(errp, "No medium inserted");
         return -ENOMEDIUM;
-    if (!drv->bdrv_truncate)
+    }
+    if (!drv->bdrv_truncate) {
+        error_setg(errp, "Image format driver does not support resize");
         return -ENOTSUP;
-    if (bs->read_only)
+    }
+    if (bs->read_only) {
+        error_setg(errp, "Image is read-only");
         return -EACCES;
+    }
 
     ret = drv->bdrv_truncate(bs, offset);
     if (ret == 0) {
@@ -3243,6 +3249,8 @@ int bdrv_truncate(BdrvChild *child, int64_t offset)
         bdrv_dirty_bitmap_truncate(bs);
         bdrv_parent_cb_resize(bs);
         ++bs->write_gen;
+    } else {
+        error_setg_errno(errp, -ret, "Failed to resize image");
     }
     return ret;
 }
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 67e8024e36..15a9966096 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -663,7 +663,7 @@ static int64_t blkdebug_getlength(BlockDriverState *bs)
 
 static int blkdebug_truncate(BlockDriverState *bs, int64_t offset)
 {
-    return bdrv_truncate(bs->file, offset);
+    return bdrv_truncate(bs->file, offset, NULL);
 }
 
 static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
diff --git a/block/block-backend.c b/block/block-backend.c
index 0b6377332c..3abd9005b9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1705,13 +1705,14 @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
                    BDRV_REQ_WRITE_COMPRESSED);
 }
 
-int blk_truncate(BlockBackend *blk, int64_t offset)
+int blk_truncate(BlockBackend *blk, int64_t offset, Error **errp)
 {
     if (!blk_is_available(blk)) {
+        error_setg(errp, "No medium inserted");
         return -ENOMEDIUM;
     }
 
-    return bdrv_truncate(blk->root, offset);
+    return bdrv_truncate(blk->root, offset, errp);
 }
 
 static void blk_pdiscard_entry(void *opaque)
diff --git a/block/commit.c b/block/commit.c
index 28324820a4..bfdd1b4142 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -151,7 +151,7 @@ static void coroutine_fn commit_run(void *opaque)
     }
 
     if (base_len < s->common.len) {
-        ret = blk_truncate(s->base, s->common.len);
+        ret = blk_truncate(s->base, s->common.len, NULL);
         if (ret) {
             goto out;
         }
@@ -508,8 +508,9 @@ int bdrv_commit(BlockDriverState *bs)
      * grow the backing file image if possible.  If not possible,
      * we must return an error */
     if (length > backing_length) {
-        ret = blk_truncate(backing, length);
+        ret = blk_truncate(backing, length, &local_err);
         if (ret < 0) {
+            error_report_err(local_err);
             goto ro_cleanup;
         }
     }
diff --git a/block/crypto.c b/block/crypto.c
index 4a2038888d..52e4f2b20f 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -389,7 +389,7 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset)
 
     offset += payload_offset;
 
-    return bdrv_truncate(bs->file, offset);
+    return bdrv_truncate(bs->file, offset, NULL);
 }
 
 static void block_crypto_close(BlockDriverState *bs)
diff --git a/block/mirror.c b/block/mirror.c
index 9e2fecc15e..d40e199761 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -724,7 +724,7 @@ static void coroutine_fn mirror_run(void *opaque)
         }
 
         if (s->bdev_length > base_length) {
-            ret = blk_truncate(s->target, s->bdev_length);
+            ret = blk_truncate(s->target, s->bdev_length, NULL);
             if (ret < 0) {
                 goto immediate_exit;
             }
diff --git a/block/parallels.c b/block/parallels.c
index 4173b3fb9d..5fe97a126b 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -216,7 +216,8 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
                                      space << BDRV_SECTOR_BITS, 0);
         } else {
             ret = bdrv_truncate(bs->file,
-                                (s->data_end + space) << BDRV_SECTOR_BITS);
+                                (s->data_end + space) << BDRV_SECTOR_BITS,
+                                NULL);
         }
         if (ret < 0) {
             return ret;
@@ -449,8 +450,10 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
                 size - res->image_end_offset);
         res->leaks += count;
         if (fix & BDRV_FIX_LEAKS) {
-            ret = bdrv_truncate(bs->file, res->image_end_offset);
+            Error *local_err = NULL;
+            ret = bdrv_truncate(bs->file, res->image_end_offset, &local_err);
             if (ret < 0) {
+                error_report_err(local_err);
                 res->check_errors++;
                 return ret;
             }
@@ -497,7 +500,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
 
     blk_set_allow_write_beyond_eof(file, true);
 
-    ret = blk_truncate(file, 0);
+    ret = blk_truncate(file, 0, errp);
     if (ret < 0) {
         goto exit;
     }
@@ -689,7 +692,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     if (!(flags & BDRV_O_RESIZE) || !bdrv_has_zero_init(bs->file->bs) ||
-            bdrv_truncate(bs->file, bdrv_getlength(bs->file->bs)) != 0) {
+            bdrv_truncate(bs->file, bdrv_getlength(bs->file->bs), NULL) != 0) {
         s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
     }
 
@@ -732,7 +735,7 @@ static void parallels_close(BlockDriverState *bs)
     }
 
     if (bs->open_flags & BDRV_O_RDWR) {
-        bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS);
+        bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, NULL);
     }
 
     g_free(s->bat_dirty_bmap);
diff --git a/block/qcow.c b/block/qcow.c
index 9d6ac83959..5d147b962e 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -473,7 +473,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
                 /* round to cluster size */
                 cluster_offset = (cluster_offset + s->cluster_size - 1) &
                     ~(s->cluster_size - 1);
-                bdrv_truncate(bs->file, cluster_offset + s->cluster_size);
+                bdrv_truncate(bs->file, cluster_offset + s->cluster_size, NULL);
                 /* if encrypted, we must initialize the cluster
                    content which won't be written */
                 if (bs->encrypted &&
@@ -833,7 +833,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
 
     blk_set_allow_write_beyond_eof(qcow_blk, true);
 
-    ret = blk_truncate(qcow_blk, 0);
+    ret = blk_truncate(qcow_blk, 0, errp);
     if (ret < 0) {
         goto exit;
     }
@@ -916,7 +916,7 @@ static int qcow_make_empty(BlockDriverState *bs)
     if (bdrv_pwrite_sync(bs->file, s->l1_table_offset, s->l1_table,
             l1_length) < 0)
         return -1;
-    ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length);
+    ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length, NULL);
     if (ret < 0)
         return ret;
 
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9e96f64c8b..4efca7ebdb 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1728,14 +1728,17 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
 
             if (fix & BDRV_FIX_ERRORS) {
                 int64_t new_nb_clusters;
+                Error *local_err = NULL;
 
                 if (offset > INT64_MAX - s->cluster_size) {
                     ret = -EINVAL;
                     goto resize_fail;
                 }
 
-                ret = bdrv_truncate(bs->file, offset + s->cluster_size);
+                ret = bdrv_truncate(bs->file, offset + s->cluster_size,
+                                    &local_err);
                 if (ret < 0) {
+                    error_report_err(local_err);
                     goto resize_fail;
                 }
                 size = bdrv_getlength(bs->file->bs);
diff --git a/block/qcow2.c b/block/qcow2.c
index 6a92d2ef3f..845eee4bd9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2294,9 +2294,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     }
 
     /* Okay, now that we have a valid image, let's give it the right size */
-    ret = blk_truncate(blk, total_size);
+    ret = blk_truncate(blk, total_size, errp);
     if (ret < 0) {
-        error_setg_errno(errp, -ret, "Could not resize image");
+        error_prepend(errp, "Could not resize image: ");
         goto out;
     }
 
@@ -2584,7 +2584,7 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
         /* align end of file to a sector boundary to ease reading with
            sector based I/Os */
         cluster_offset = bdrv_getlength(bs->file->bs);
-        return bdrv_truncate(bs->file, cluster_offset);
+        return bdrv_truncate(bs->file, cluster_offset, NULL);
     }
 
     buf = qemu_blockalign(bs, s->cluster_size);
@@ -2674,6 +2674,7 @@ fail:
 static int make_completely_empty(BlockDriverState *bs)
 {
     BDRVQcow2State *s = bs->opaque;
+    Error *local_err = NULL;
     int ret, l1_clusters;
     int64_t offset;
     uint64_t *new_reftable = NULL;
@@ -2798,8 +2799,10 @@ static int make_completely_empty(BlockDriverState *bs)
         goto fail;
     }
 
-    ret = bdrv_truncate(bs->file, (3 + l1_clusters) * s->cluster_size);
+    ret = bdrv_truncate(bs->file, (3 + l1_clusters) * s->cluster_size,
+                        &local_err);
     if (ret < 0) {
+        error_report_err(local_err);
         goto fail;
     }
 
@@ -3273,9 +3276,10 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
             return ret;
         }
 
-        ret = blk_truncate(blk, new_size);
+        ret = blk_truncate(blk, new_size, &local_err);
         blk_unref(blk);
         if (ret < 0) {
+            error_report_err(local_err);
             return ret;
         }
     }
diff --git a/block/qed.c b/block/qed.c
index 5ec7fd83f2..53199ac8d7 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -635,7 +635,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     blk_set_allow_write_beyond_eof(blk, true);
 
     /* File must start empty and grow, check truncate is supported */
-    ret = blk_truncate(blk, 0);
+    ret = blk_truncate(blk, 0, errp);
     if (ret < 0) {
         goto out;
     }
diff --git a/block/raw-format.c b/block/raw-format.c
index 86fbc657eb..a80073369a 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -341,7 +341,7 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset)
 
     s->size = offset;
     offset += s->offset;
-    return bdrv_truncate(bs->file, offset);
+    return bdrv_truncate(bs->file, offset, NULL);
 }
 
 static int raw_media_changed(BlockDriverState *bs)
diff --git a/block/vdi.c b/block/vdi.c
index 9b4f70e977..d12d9cdc79 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -832,9 +832,9 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
     }
 
     if (image_type == VDI_TYPE_STATIC) {
-        ret = blk_truncate(blk, offset + blocks * block_size);
+        ret = blk_truncate(blk, offset + blocks * block_size, errp);
         if (ret < 0) {
-            error_setg(errp, "Failed to statically allocate %s", filename);
+            error_prepend(errp, "Failed to statically allocate %s", filename);
             goto exit;
         }
     }
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index 67a91c0de5..3f4c2aa095 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -548,7 +548,7 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s,
             if (new_file_size % (1024*1024)) {
                 /* round up to nearest 1MB boundary */
                 new_file_size = ((new_file_size >> 20) + 1) << 20;
-                bdrv_truncate(bs->file, new_file_size);
+                bdrv_truncate(bs->file, new_file_size, NULL);
             }
         }
         qemu_vfree(desc_entries);
diff --git a/block/vhdx.c b/block/vhdx.c
index d25bcd91de..e8fe3fb5e9 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1171,7 +1171,7 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
     /* per the spec, the address for a block is in units of 1MB */
     *new_offset = ROUND_UP(*new_offset, 1024 * 1024);
 
-    return bdrv_truncate(bs->file, *new_offset + s->block_size);
+    return bdrv_truncate(bs->file, *new_offset + s->block_size, NULL);
 }
 
 /*
@@ -1607,17 +1607,13 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s,
     if (type == VHDX_TYPE_DYNAMIC) {
         /* All zeroes, so we can just extend the file - the end of the BAT
          * is the furthest thing we have written yet */
-        ret = blk_truncate(blk, data_file_offset);
+        ret = blk_truncate(blk, data_file_offset, errp);
         if (ret < 0) {
-            error_setg_errno(errp, -ret,
-                            "Failed to resize the underlying file");
             goto exit;
         }
     } else if (type == VHDX_TYPE_FIXED) {
-        ret = blk_truncate(blk, data_file_offset + image_size);
+        ret = blk_truncate(blk, data_file_offset + image_size, errp);
         if (ret < 0) {
-            error_setg_errno(errp, -ret,
-                            "Failed to resize the underlying file");
             goto exit;
         }
     } else {
diff --git a/block/vmdk.c b/block/vmdk.c
index a9bd22bf93..c61b9cc8e0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1714,10 +1714,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
     blk_set_allow_write_beyond_eof(blk, true);
 
     if (flat) {
-        ret = blk_truncate(blk, filesize);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not truncate file");
-        }
+        ret = blk_truncate(blk, filesize, errp);
         goto exit;
     }
     magic = cpu_to_be32(VMDK4_MAGIC);
@@ -1780,9 +1777,8 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
         goto exit;
     }
 
-    ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9);
+    ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9, errp);
     if (ret < 0) {
-        error_setg_errno(errp, -ret, "Could not truncate file");
         goto exit;
     }
 
@@ -2090,10 +2086,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     /* bdrv_pwrite write padding zeros to align to sector, we don't need that
      * for description file */
     if (desc_offset == 0) {
-        ret = blk_truncate(new_blk, desc_len);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not truncate file");
-        }
+        ret = blk_truncate(new_blk, desc_len, errp);
     }
 exit:
     if (new_blk) {
diff --git a/block/vpc.c b/block/vpc.c
index f591d4be38..ecfee77149 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -851,20 +851,21 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t *buf,
 }
 
 static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
-                             int64_t total_size)
+                             int64_t total_size, Error **errp)
 {
     int ret;
 
     /* Add footer to total size */
     total_size += HEADER_SIZE;
 
-    ret = blk_truncate(blk, total_size);
+    ret = blk_truncate(blk, total_size, errp);
     if (ret < 0) {
         return ret;
     }
 
     ret = blk_pwrite(blk, total_size - HEADER_SIZE, buf, HEADER_SIZE, 0);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Unable to write VHD header");
         return ret;
     }
 
@@ -996,11 +997,11 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
 
     if (disk_type == VHD_DYNAMIC) {
         ret = create_dynamic_disk(blk, buf, total_sectors);
+        if (ret < 0) {
+            error_setg(errp, "Unable to create or write VHD header");
+        }
     } else {
-        ret = create_fixed_disk(blk, buf, total_size);
-    }
-    if (ret < 0) {
-        error_setg(errp, "Unable to create or write VHD header");
+        ret = create_fixed_disk(blk, buf, total_size, errp);
     }
 
 out:
diff --git a/blockdev.c b/blockdev.c
index 040c152512..9afd1e97c3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2930,26 +2930,7 @@ void qmp_block_resize(bool has_device, const char *device,
     /* complete all in-flight operations before resizing the device */
     bdrv_drain_all();
 
-    ret = blk_truncate(blk, size);
-    switch (ret) {
-    case 0:
-        break;
-    case -ENOMEDIUM:
-        error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-        break;
-    case -ENOTSUP:
-        error_setg(errp, QERR_UNSUPPORTED);
-        break;
-    case -EACCES:
-        error_setg(errp, "Device '%s' is read only", device);
-        break;
-    case -EBUSY:
-        error_setg(errp, QERR_DEVICE_IN_USE, device);
-        break;
-    default:
-        error_setg_errno(errp, -ret, "Could not resize");
-        break;
-    }
+    ret = blk_truncate(blk, size, errp);
 
 out:
     blk_unref(blk);
diff --git a/qemu-img.c b/qemu-img.c
index b220cf71d7..37c2894678 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3500,20 +3500,11 @@ static int img_resize(int argc, char **argv)
         goto out;
     }
 
-    ret = blk_truncate(blk, total_size);
-    switch (ret) {
-    case 0:
+    ret = blk_truncate(blk, total_size, &err);
+    if (!ret) {
         qprintf(quiet, "Image resized.\n");
-        break;
-    case -ENOTSUP:
-        error_report("This image does not support resize");
-        break;
-    case -EACCES:
-        error_report("Image is read-only");
-        break;
-    default:
-        error_report("Error resizing image: %s", strerror(-ret));
-        break;
+    } else {
+        error_report_err(err);
     }
 out:
     blk_unref(blk);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 2c48f9ce1a..05bb0b34ec 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1560,6 +1560,7 @@ static const cmdinfo_t flush_cmd = {
 
 static int truncate_f(BlockBackend *blk, int argc, char **argv)
 {
+    Error *local_err = NULL;
     int64_t offset;
     int ret;
 
@@ -1569,9 +1570,9 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv)
         return 0;
     }
 
-    ret = blk_truncate(blk, offset);
+    ret = blk_truncate(blk, offset, &local_err);
     if (ret < 0) {
-        printf("truncate: %s\n", strerror(-ret));
+        error_report_err(local_err);
         return 0;
     }
 
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 for-2.10 3/4] block: Add errp to BD.bdrv_truncate()
  2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 1/4] block/vhdx: Make vhdx_create() always set errp Max Reitz
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 2/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
@ 2017-03-28 20:51 ` Max Reitz
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 4/4] block: Add .bdrv_truncate() error messages Max Reitz
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Max Reitz @ 2017-03-28 20:51 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Stefan Hajnoczi, Eric Blake

Add an Error parameter to the block drivers' bdrv_truncate() interface.
If a block driver does not set this in case of an error, the generic
bdrv_truncate() implementation will do so.

Where it is obvious, this patch also makes some block drivers set this
value.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block_int.h |  2 +-
 block.c                   |  4 ++--
 block/blkdebug.c          |  4 ++--
 block/crypto.c            |  5 +++--
 block/file-posix.c        |  2 +-
 block/file-win32.c        |  6 +++---
 block/gluster.c           |  3 ++-
 block/iscsi.c             |  4 ++--
 block/nfs.c               |  2 +-
 block/qcow2.c             |  8 ++++----
 block/qed.c               |  2 +-
 block/raw-format.c        |  4 ++--
 block/rbd.c               |  2 +-
 block/sheepdog.c          | 14 ++++++--------
 14 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 59400bd848..08063c10c8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -196,7 +196,7 @@ struct BlockDriver {
     int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
 
     const char *protocol_name;
-    int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset);
+    int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset, Error **errp);
 
     int64_t (*bdrv_getlength)(BlockDriverState *bs);
     bool has_variable_length;
diff --git a/block.c b/block.c
index 9ed526e01d..7b9841f99a 100644
--- a/block.c
+++ b/block.c
@@ -3243,13 +3243,13 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
         return -EACCES;
     }
 
-    ret = drv->bdrv_truncate(bs, offset);
+    ret = drv->bdrv_truncate(bs, offset, errp);
     if (ret == 0) {
         ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
         bdrv_dirty_bitmap_truncate(bs);
         bdrv_parent_cb_resize(bs);
         ++bs->write_gen;
-    } else {
+    } else if (errp && !*errp) {
         error_setg_errno(errp, -ret, "Failed to resize image");
     }
     return ret;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 15a9966096..c795ae9e72 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -661,9 +661,9 @@ static int64_t blkdebug_getlength(BlockDriverState *bs)
     return bdrv_getlength(bs->file->bs);
 }
 
-static int blkdebug_truncate(BlockDriverState *bs, int64_t offset)
+static int blkdebug_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
-    return bdrv_truncate(bs->file, offset, NULL);
+    return bdrv_truncate(bs->file, offset, errp);
 }
 
 static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
diff --git a/block/crypto.c b/block/crypto.c
index 52e4f2b20f..17b3140998 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -381,7 +381,8 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,
     return ret;
 }
 
-static int block_crypto_truncate(BlockDriverState *bs, int64_t offset)
+static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,
+                                 Error **errp)
 {
     BlockCrypto *crypto = bs->opaque;
     size_t payload_offset =
@@ -389,7 +390,7 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset)
 
     offset += payload_offset;
 
-    return bdrv_truncate(bs->file, offset, NULL);
+    return bdrv_truncate(bs->file, offset, errp);
 }
 
 static void block_crypto_close(BlockDriverState *bs)
diff --git a/block/file-posix.c b/block/file-posix.c
index 0841a08785..d23464013f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1409,7 +1409,7 @@ static void raw_close(BlockDriverState *bs)
     }
 }
 
-static int raw_truncate(BlockDriverState *bs, int64_t offset)
+static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     struct stat st;
diff --git a/block/file-win32.c b/block/file-win32.c
index 800fabdd72..3f3925623f 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -461,7 +461,7 @@ static void raw_close(BlockDriverState *bs)
     }
 }
 
-static int raw_truncate(BlockDriverState *bs, int64_t offset)
+static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     LONG low, high;
@@ -476,11 +476,11 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset)
      */
     dwPtrLow = SetFilePointer(s->hfile, low, &high, FILE_BEGIN);
     if (dwPtrLow == INVALID_SET_FILE_POINTER && GetLastError() != NO_ERROR) {
-        fprintf(stderr, "SetFilePointer error: %lu\n", GetLastError());
+        error_setg_win32(errp, GetLastError(), "SetFilePointer error");
         return -EIO;
     }
     if (SetEndOfFile(s->hfile) == 0) {
-        fprintf(stderr, "SetEndOfFile error: %lu\n", GetLastError());
+        error_setg_win32(errp, GetLastError(), "SetEndOfFile error");
         return -EIO;
     }
     return 0;
diff --git a/block/gluster.c b/block/gluster.c
index a577daef10..00b8240562 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1084,7 +1084,8 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
     return acb.ret;
 }
 
-static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset)
+static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset,
+                                 Error **errp)
 {
     int ret;
     BDRVGlusterState *s = bs->opaque;
diff --git a/block/iscsi.c b/block/iscsi.c
index 75d890538e..ab559a6f71 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2060,7 +2060,7 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
     }
 }
 
-static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
+static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     IscsiLun *iscsilun = bs->opaque;
     Error *local_err = NULL;
@@ -2071,7 +2071,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
 
     iscsi_readcapacity_sync(iscsilun, &local_err);
     if (local_err != NULL) {
-        error_free(local_err);
+        error_propagate(errp, local_err);
         return -EIO;
     }
 
diff --git a/block/nfs.c b/block/nfs.c
index 3f43f6e26a..57d12efc51 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -757,7 +757,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
 }
 
-static int nfs_file_truncate(BlockDriverState *bs, int64_t offset)
+static int nfs_file_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     NFSClient *client = bs->opaque;
     return nfs_ftruncate(client->context, client->fh, offset);
diff --git a/block/qcow2.c b/block/qcow2.c
index 845eee4bd9..6c347989e3 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2525,26 +2525,26 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
     return ret;
 }
 
-static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
+static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     BDRVQcow2State *s = bs->opaque;
     int64_t new_l1_size;
     int ret;
 
     if (offset & 511) {
-        error_report("The new size must be a multiple of 512");
+        error_setg(errp, "The new size must be a multiple of 512");
         return -EINVAL;
     }
 
     /* cannot proceed if image has snapshots */
     if (s->nb_snapshots) {
-        error_report("Can't resize an image which has snapshots");
+        error_setg(errp, "Can't resize an image which has snapshots");
         return -ENOTSUP;
     }
 
     /* shrinking is currently not supported */
     if (offset < bs->total_sectors * 512) {
-        error_report("qcow2 doesn't support shrinking images yet");
+        error_setg(errp, "qcow2 doesn't support shrinking images yet");
         return -ENOTSUP;
     }
 
diff --git a/block/qed.c b/block/qed.c
index 53199ac8d7..fa2aeee471 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1518,7 +1518,7 @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
     return cb.ret;
 }
 
-static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset)
+static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     BDRVQEDState *s = bs->opaque;
     uint64_t old_image_size;
diff --git a/block/raw-format.c b/block/raw-format.c
index a80073369a..9761bdaff8 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -327,7 +327,7 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
     }
 }
 
-static int raw_truncate(BlockDriverState *bs, int64_t offset)
+static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
 
@@ -341,7 +341,7 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset)
 
     s->size = offset;
     offset += s->offset;
-    return bdrv_truncate(bs->file, offset, NULL);
+    return bdrv_truncate(bs->file, offset, errp);
 }
 
 static int raw_media_changed(BlockDriverState *bs)
diff --git a/block/rbd.c b/block/rbd.c
index 498322b30b..30bb0b7798 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -879,7 +879,7 @@ static int64_t qemu_rbd_getlength(BlockDriverState *bs)
     return info.size;
 }
 
-static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset)
+static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     BDRVRBDState *s = bs->opaque;
     int r;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 89e98edab6..27126ea474 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2133,9 +2133,8 @@ static int64_t sd_getlength(BlockDriverState *bs)
     return s->inode.vdi_size;
 }
 
-static int sd_truncate(BlockDriverState *bs, int64_t offset)
+static int sd_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
-    Error *local_err = NULL;
     BDRVSheepdogState *s = bs->opaque;
     int ret, fd;
     unsigned int datalen;
@@ -2143,16 +2142,15 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
 
     max_vdi_size = (UINT64_C(1) << s->inode.block_size_shift) * MAX_DATA_OBJS;
     if (offset < s->inode.vdi_size) {
-        error_report("shrinking is not supported");
+        error_setg(errp, "shrinking is not supported");
         return -EINVAL;
     } else if (offset > max_vdi_size) {
-        error_report("too big image size");
+        error_setg(errp, "too big image size");
         return -EINVAL;
     }
 
-    fd = connect_to_sdog(s, &local_err);
+    fd = connect_to_sdog(s, errp);
     if (fd < 0) {
-        error_report_err(local_err);
         return fd;
     }
 
@@ -2165,7 +2163,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
     close(fd);
 
     if (ret < 0) {
-        error_report("failed to update an inode.");
+        error_setg_errno(errp, -ret, "failed to update an inode");
     }
 
     return ret;
@@ -2430,7 +2428,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
     BDRVSheepdogState *s = bs->opaque;
 
     if (offset > s->inode.vdi_size) {
-        ret = sd_truncate(bs, offset);
+        ret = sd_truncate(bs, offset, NULL);
         if (ret < 0) {
             return ret;
         }
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 for-2.10 4/4] block: Add .bdrv_truncate() error messages
  2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
                   ` (2 preceding siblings ...)
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 3/4] block: Add errp to BD.bdrv_truncate() Max Reitz
@ 2017-03-28 20:51 ` Max Reitz
  2017-03-28 21:01 ` [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Max Reitz @ 2017-03-28 20:51 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Stefan Hajnoczi, Eric Blake

Add missing error messages for the block driver implementations of
.bdrv_truncate(); drop the generic one from block.c's bdrv_truncate().

Since one of these changes touches a mis-indented block in
block/file-posix.c, this patch fixes that coding style issue along the
way.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c            |  2 --
 block/file-posix.c | 17 ++++++++++++-----
 block/gluster.c    |  4 +++-
 block/iscsi.c      |  2 ++
 block/nfs.c        | 10 +++++++++-
 block/qcow2.c      |  2 ++
 block/qed.c        |  4 +++-
 block/raw-format.c |  2 ++
 block/rbd.c        |  1 +
 9 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 7b9841f99a..80e16e33d3 100644
--- a/block.c
+++ b/block.c
@@ -3249,8 +3249,6 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
         bdrv_dirty_bitmap_truncate(bs);
         bdrv_parent_cb_resize(bs);
         ++bs->write_gen;
-    } else if (errp && !*errp) {
-        error_setg_errno(errp, -ret, "Failed to resize image");
     }
     return ret;
 }
diff --git a/block/file-posix.c b/block/file-posix.c
index d23464013f..c08d031c78 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1413,20 +1413,27 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     struct stat st;
+    int ret;
 
     if (fstat(s->fd, &st)) {
-        return -errno;
+        ret = -errno;
+        error_setg_errno(errp, -ret, "Failed to fstat() the file");
+        return ret;
     }
 
     if (S_ISREG(st.st_mode)) {
         if (ftruncate(s->fd, offset) < 0) {
-            return -errno;
+            ret = -errno;
+            error_setg_errno(errp, -ret, "Failed to resize the file");
+            return ret;
         }
     } else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-       if (offset > raw_getlength(bs)) {
-           return -EINVAL;
-       }
+        if (offset > raw_getlength(bs)) {
+            error_setg(errp, "Cannot grow device files");
+            return -EINVAL;
+        }
     } else {
+        error_setg(errp, "Resizing this file is not supported");
         return -ENOTSUP;
     }
 
diff --git a/block/gluster.c b/block/gluster.c
index 00b8240562..65350b575b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1092,7 +1092,9 @@ static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset,
 
     ret = glfs_ftruncate(s->fd, offset);
     if (ret < 0) {
-        return -errno;
+        ret = -errno;
+        error_setg_errno(errp, -ret, "Failed to truncate file");
+        return ret;
     }
 
     return 0;
diff --git a/block/iscsi.c b/block/iscsi.c
index ab559a6f71..036f5b6930 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2066,6 +2066,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     Error *local_err = NULL;
 
     if (iscsilun->type != TYPE_DISK) {
+        error_setg(errp, "Cannot resize non-disk iSCSI devices");
         return -ENOTSUP;
     }
 
@@ -2076,6 +2077,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     }
 
     if (offset > iscsi_getlength(bs)) {
+        error_setg(errp, "Cannot grow iSCSI devices");
         return -EINVAL;
     }
 
diff --git a/block/nfs.c b/block/nfs.c
index 57d12efc51..6eccf18d75 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -760,7 +760,15 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
 static int nfs_file_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 {
     NFSClient *client = bs->opaque;
-    return nfs_ftruncate(client->context, client->fh, offset);
+    int ret;
+
+    ret = nfs_ftruncate(client->context, client->fh, offset);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to truncate file");
+        return ret;
+    }
+
+    return 0;
 }
 
 /* Note that this will not re-establish a connection with the NFS server
diff --git a/block/qcow2.c b/block/qcow2.c
index 6c347989e3..4ca4cf04b0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2551,6 +2551,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     new_l1_size = size_to_l1(s, offset);
     ret = qcow2_grow_l1_table(bs, new_l1_size, true);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to grow the L1 table");
         return ret;
     }
 
@@ -2559,6 +2560,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size),
                            &offset, sizeof(uint64_t));
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Failed to update the image size");
         return ret;
     }
 
diff --git a/block/qed.c b/block/qed.c
index fa2aeee471..fd76817cbb 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1526,11 +1526,12 @@ static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 
     if (!qed_is_image_size_valid(offset, s->header.cluster_size,
                                  s->header.table_size)) {
+        error_setg(errp, "Invalid image size specified");
         return -EINVAL;
     }
 
-    /* Shrinking is currently not supported */
     if ((uint64_t)offset < s->header.image_size) {
+        error_setg(errp, "Shrinking images is currently not supported");
         return -ENOTSUP;
     }
 
@@ -1539,6 +1540,7 @@ static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     ret = qed_write_header_sync(s);
     if (ret < 0) {
         s->header.image_size = old_image_size;
+        error_setg_errno(errp, -ret, "Failed to update the image size");
     }
     return ret;
 }
diff --git a/block/raw-format.c b/block/raw-format.c
index 9761bdaff8..36e65036f0 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -332,10 +332,12 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     BDRVRawState *s = bs->opaque;
 
     if (s->has_size) {
+        error_setg(errp, "Cannot resize fixed-size raw disks");
         return -ENOTSUP;
     }
 
     if (INT64_MAX - offset < s->offset) {
+        error_setg(errp, "Disk size too large for the chosen offset");
         return -EINVAL;
     }
 
diff --git a/block/rbd.c b/block/rbd.c
index 30bb0b7798..1ea29f7a97 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -886,6 +886,7 @@ static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
 
     r = rbd_resize(s->image, offset);
     if (r < 0) {
+        error_setg_errno(errp, -r, "Failed to resize file");
         return r;
     }
 
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate()
  2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
                   ` (3 preceding siblings ...)
  2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 4/4] block: Add .bdrv_truncate() error messages Max Reitz
@ 2017-03-28 21:01 ` Max Reitz
  2017-03-30 12:41 ` Stefan Hajnoczi
  2017-03-31 13:38 ` Max Reitz
  6 siblings, 0 replies; 8+ messages in thread
From: Max Reitz @ 2017-03-28 21:01 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Kevin Wolf, Stefan Hajnoczi, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 3048 bytes --]

On 28.03.2017 22:51, Max Reitz wrote:
> Having an Error parameter for these functions makes sense because we
> sometimes want a bit more information than just "Something failed". Some
> drivers already use error_report() and the like to emit this additional
> information, so it's rather obvious that we do want a real error object
> here.
> 
> 
> v3:
> - Patch 2: Keep "Could not resize image" message in qcow2_create2() by
>            using error_prepend() [Kevin]
> - Patch 3: Dropped archipelago
> - Patch 4:
>   - Keep errno information where available [Kevin]
>   - Make all drivers generate error messages [Stefan/Eric]
>   - Drop generic error message from bdrv_truncate() [Stefan/Eric]

Oops, forgot the backport-diff against v2, here you go:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences,
respectively

001/4:[----] [--] 'block/vhdx: Make vhdx_create() always set errp'
002/4:[0001] [FC] 'block: Add errp to b{lk,drv}_truncate()'
003/4:[0003] [FC] 'block: Add errp to BD.bdrv_truncate()'
004/4:[0025] [FC] 'block: Add .bdrv_truncate() error messages'


Max

> Max Reitz (4):
>   block/vhdx: Make vhdx_create() always set errp
>   block: Add errp to b{lk,drv}_truncate()
>   block: Add errp to BD.bdrv_truncate()
>   block: Add .bdrv_truncate() error messages
> 
>  include/block/block.h          |  2 +-
>  include/block/block_int.h      |  2 +-
>  include/sysemu/block-backend.h |  2 +-
>  block.c                        | 16 +++++++++++-----
>  block/blkdebug.c               |  4 ++--
>  block/block-backend.c          |  5 +++--
>  block/commit.c                 |  5 +++--
>  block/crypto.c                 |  5 +++--
>  block/file-posix.c             | 19 +++++++++++++------
>  block/file-win32.c             |  6 +++---
>  block/gluster.c                |  7 +++++--
>  block/iscsi.c                  |  6 ++++--
>  block/mirror.c                 |  2 +-
>  block/nfs.c                    | 12 ++++++++++--
>  block/parallels.c              | 13 ++++++++-----
>  block/qcow.c                   |  6 +++---
>  block/qcow2-refcount.c         |  5 ++++-
>  block/qcow2.c                  | 24 +++++++++++++++---------
>  block/qed.c                    |  8 +++++---
>  block/raw-format.c             |  6 ++++--
>  block/rbd.c                    |  3 ++-
>  block/sheepdog.c               | 14 ++++++--------
>  block/vdi.c                    |  4 ++--
>  block/vhdx-log.c               |  2 +-
>  block/vhdx.c                   | 25 ++++++++++++++++++-------
>  block/vmdk.c                   | 13 +++----------
>  block/vpc.c                    | 13 +++++++------
>  blockdev.c                     | 21 +--------------------
>  qemu-img.c                     | 17 ++++-------------
>  qemu-io-cmds.c                 |  5 +++--
>  30 files changed, 147 insertions(+), 125 deletions(-)
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate()
  2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
                   ` (4 preceding siblings ...)
  2017-03-28 21:01 ` [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
@ 2017-03-30 12:41 ` Stefan Hajnoczi
  2017-03-31 13:38 ` Max Reitz
  6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2017-03-30 12:41 UTC (permalink / raw)
  To: Max Reitz; +Cc: qemu-block, qemu-devel, Kevin Wolf, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 2590 bytes --]

On Tue, Mar 28, 2017 at 10:51:25PM +0200, Max Reitz wrote:
> Having an Error parameter for these functions makes sense because we
> sometimes want a bit more information than just "Something failed". Some
> drivers already use error_report() and the like to emit this additional
> information, so it's rather obvious that we do want a real error object
> here.
> 
> 
> v3:
> - Patch 2: Keep "Could not resize image" message in qcow2_create2() by
>            using error_prepend() [Kevin]
> - Patch 3: Dropped archipelago
> - Patch 4:
>   - Keep errno information where available [Kevin]
>   - Make all drivers generate error messages [Stefan/Eric]
>   - Drop generic error message from bdrv_truncate() [Stefan/Eric]
> 
> 
> Max Reitz (4):
>   block/vhdx: Make vhdx_create() always set errp
>   block: Add errp to b{lk,drv}_truncate()
>   block: Add errp to BD.bdrv_truncate()
>   block: Add .bdrv_truncate() error messages
> 
>  include/block/block.h          |  2 +-
>  include/block/block_int.h      |  2 +-
>  include/sysemu/block-backend.h |  2 +-
>  block.c                        | 16 +++++++++++-----
>  block/blkdebug.c               |  4 ++--
>  block/block-backend.c          |  5 +++--
>  block/commit.c                 |  5 +++--
>  block/crypto.c                 |  5 +++--
>  block/file-posix.c             | 19 +++++++++++++------
>  block/file-win32.c             |  6 +++---
>  block/gluster.c                |  7 +++++--
>  block/iscsi.c                  |  6 ++++--
>  block/mirror.c                 |  2 +-
>  block/nfs.c                    | 12 ++++++++++--
>  block/parallels.c              | 13 ++++++++-----
>  block/qcow.c                   |  6 +++---
>  block/qcow2-refcount.c         |  5 ++++-
>  block/qcow2.c                  | 24 +++++++++++++++---------
>  block/qed.c                    |  8 +++++---
>  block/raw-format.c             |  6 ++++--
>  block/rbd.c                    |  3 ++-
>  block/sheepdog.c               | 14 ++++++--------
>  block/vdi.c                    |  4 ++--
>  block/vhdx-log.c               |  2 +-
>  block/vhdx.c                   | 25 ++++++++++++++++++-------
>  block/vmdk.c                   | 13 +++----------
>  block/vpc.c                    | 13 +++++++------
>  blockdev.c                     | 21 +--------------------
>  qemu-img.c                     | 17 ++++-------------
>  qemu-io-cmds.c                 |  5 +++--
>  30 files changed, 147 insertions(+), 125 deletions(-)
> 
> -- 
> 2.12.1
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate()
  2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
                   ` (5 preceding siblings ...)
  2017-03-30 12:41 ` Stefan Hajnoczi
@ 2017-03-31 13:38 ` Max Reitz
  6 siblings, 0 replies; 8+ messages in thread
From: Max Reitz @ 2017-03-31 13:38 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Kevin Wolf, Stefan Hajnoczi, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]

On 28.03.2017 22:51, Max Reitz wrote:
> Having an Error parameter for these functions makes sense because we
> sometimes want a bit more information than just "Something failed". Some
> drivers already use error_report() and the like to emit this additional
> information, so it's rather obvious that we do want a real error object
> here.
> 
> 
> v3:
> - Patch 2: Keep "Could not resize image" message in qcow2_create2() by
>            using error_prepend() [Kevin]
> - Patch 3: Dropped archipelago
> - Patch 4:
>   - Keep errno information where available [Kevin]
>   - Make all drivers generate error messages [Stefan/Eric]
>   - Drop generic error message from bdrv_truncate() [Stefan/Eric]
> 
> 
> Max Reitz (4):
>   block/vhdx: Make vhdx_create() always set errp
>   block: Add errp to b{lk,drv}_truncate()
>   block: Add errp to BD.bdrv_truncate()
>   block: Add .bdrv_truncate() error messages
> 
>  include/block/block.h          |  2 +-
>  include/block/block_int.h      |  2 +-
>  include/sysemu/block-backend.h |  2 +-
>  block.c                        | 16 +++++++++++-----
>  block/blkdebug.c               |  4 ++--
>  block/block-backend.c          |  5 +++--
>  block/commit.c                 |  5 +++--
>  block/crypto.c                 |  5 +++--
>  block/file-posix.c             | 19 +++++++++++++------
>  block/file-win32.c             |  6 +++---
>  block/gluster.c                |  7 +++++--
>  block/iscsi.c                  |  6 ++++--
>  block/mirror.c                 |  2 +-
>  block/nfs.c                    | 12 ++++++++++--
>  block/parallels.c              | 13 ++++++++-----
>  block/qcow.c                   |  6 +++---
>  block/qcow2-refcount.c         |  5 ++++-
>  block/qcow2.c                  | 24 +++++++++++++++---------
>  block/qed.c                    |  8 +++++---
>  block/raw-format.c             |  6 ++++--
>  block/rbd.c                    |  3 ++-
>  block/sheepdog.c               | 14 ++++++--------
>  block/vdi.c                    |  4 ++--
>  block/vhdx-log.c               |  2 +-
>  block/vhdx.c                   | 25 ++++++++++++++++++-------
>  block/vmdk.c                   | 13 +++----------
>  block/vpc.c                    | 13 +++++++------
>  blockdev.c                     | 21 +--------------------
>  qemu-img.c                     | 17 ++++-------------
>  qemu-io-cmds.c                 |  5 +++--
>  30 files changed, 147 insertions(+), 125 deletions(-)

Applied to my block-next branch:

https://github.com/XanClic/qemu/commits/block-next

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-03-31 13:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 20:51 [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 1/4] block/vhdx: Make vhdx_create() always set errp Max Reitz
2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 2/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 3/4] block: Add errp to BD.bdrv_truncate() Max Reitz
2017-03-28 20:51 ` [Qemu-devel] [PATCH v3 for-2.10 4/4] block: Add .bdrv_truncate() error messages Max Reitz
2017-03-28 21:01 ` [Qemu-devel] [PATCH v3 for-2.10 0/4] block: Add errp to b{lk, drv}_truncate() Max Reitz
2017-03-30 12:41 ` Stefan Hajnoczi
2017-03-31 13:38 ` Max Reitz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.