[Qemu-devel] [PATCH v2 4/4] raw-posix: Clean up around raw_co_get_block_status()

[Markus Armbruster <armbru@redhat.com>]

try_seek_hole() doesn't really seek to a hole, it tries to find out
whether its argument is in a hole or not, and where the hole or
non-hole ends.  Rename to find_allocation() and add a proper function
comment.

Using arguments passed by reference like local variables is a bad
habit.  Only assign to them right before return.

Avoid nesting of conditionals.

When find_allocation() fails, don't make up a range that'll get mapped
to nb_sectors, simply set *pnum = nb_sectors directly.

Don't repeat BDRV_BLOCK_OFFSET_VALID | start.

Drop a pointless assertion, add some meaningful ones.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/raw-posix.c | 62 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 2a12a50..ea5b3b8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1478,28 +1478,43 @@ out:
     return result;
 }
 
-static int try_seek_hole(BlockDriverState *bs, off_t start, off_t *data,
-                         off_t *hole)
+/*
+ * Find allocation range in @bs around offset @start.
+ * If @start is in a hole, store @start in @hole and the end of the
+ * hole in @data, and return 0.
+ * If @start is in a data, store @start to @data, and the end of the
+ * data to @hole, and return 0.
+ * If we can't find out, return -errno.
+ */
+static int find_allocation(BlockDriverState *bs, off_t start,
+                           off_t *data, off_t *hole)
 {
 #if defined SEEK_HOLE && defined SEEK_DATA
     BDRVRawState *s = bs->opaque;
+    off_t offs;
 
-    *hole = lseek(s->fd, start, SEEK_HOLE);
-    if (*hole == -1) {
+    offs = lseek(s->fd, start, SEEK_HOLE);
+    if (offs < 0) {
         return -errno;
     }
+    assert(offs >= start);
 
-    if (*hole > start) {
+    if (offs > start) {
+        /* in data, next hole at offs */
         *data = start;
-    } else {
-        /* On a hole.  We need another syscall to find its end.  */
-        *data = lseek(s->fd, start, SEEK_DATA);
-        if (*data < 0) {
-            /* no idea where the hole ends, give up (unlikely to happen) */
-            return -errno;
-        }
+        *hole = offs;
+        return 0;
     }
 
+    /* in hole, end not yet known */
+    offs = lseek(s->fd, start, SEEK_DATA);
+    if (offs < 0) {
+        /* no idea where the hole ends, give up (unlikely to happen) */
+        return -errno;
+    }
+    assert(offs > start);
+    *hole = start;
+    *data = offs;
     return 0;
 #else
     return -ENOTSUP;
@@ -1543,25 +1558,22 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
         nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
     }
 
-    ret = try_seek_hole(bs, start, &data, &hole);
-    if (ret < 0) {
-        /* Assume everything is allocated. */
-        data = 0;
-        hole = start + nb_sectors * BDRV_SECTOR_SIZE;
-        ret = 0;
-    }
-
-    assert(ret >= 0);
-
-    if (data <= start) {
+    ret = BDRV_BLOCK_OFFSET_VALID | start;
+    if (find_allocation(bs, start, &data, &hole) < 0) {
+        /* No info available, so pretend there are no holes */
+        *pnum = nb_sectors;
+        ret |= BDRV_BLOCK_DATA;
+    } else if (data == start) {
         /* On a data extent, compute sectors to the end of the extent.  */
         *pnum = MIN(nb_sectors, (hole - start) / BDRV_SECTOR_SIZE);
-        return ret | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
+        ret |= BDRV_BLOCK_DATA;
     } else {
         /* On a hole, compute sectors to the beginning of the next extent.  */
+        assert(hole == start);
         *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
-        return ret | BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID | start;
+        ret |= BDRV_BLOCK_ZERO;
     }
+    return ret;
 }
 
 static coroutine_fn BlockAIOCB *raw_aio_discard(BlockDriverState *bs,
-- 
1.9.3