diff mbox series

[01/12] file-posix: remove incorrect coroutine_fn calls

Message ID 20230601115145.196465-2-pbonzini@redhat.com
State New
Headers show
Series block: more fixes to coroutine_fn marking | expand

Commit Message

Paolo Bonzini June 1, 2023, 11:51 a.m. UTC 
raw_co_getlength is called by handle_aiocb_write_zeroes, which is not a coroutine
function.  This is harmless because raw_co_getlength does not actually suspend,
but in the interest of clarity make it a non-coroutine_fn that is just wrapped
by the coroutine_fn raw_co_getlength.  Likewise, check_cache_dropped was only
a coroutine_fn because it called raw_co_getlength, so it can be made non-coroutine
as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/file-posix.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

Comments

Eric Blake June 1, 2023, 1:50 p.m. UTC | #1 
On Thu, Jun 01, 2023 at 01:51:34PM +0200, Paolo Bonzini wrote:
> raw_co_getlength is called by handle_aiocb_write_zeroes, which is not a coroutine
> function.  This is harmless because raw_co_getlength does not actually suspend,
> but in the interest of clarity make it a non-coroutine_fn that is just wrapped
> by the coroutine_fn raw_co_getlength.  Likewise, check_cache_dropped was only
> a coroutine_fn because it called raw_co_getlength, so it can be made non-coroutine
> as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/file-posix.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 

> @@ -2696,7 +2696,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
>      }
>  
>      if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
> -        int64_t cur_length = raw_co_getlength(bs);
> +        int64_t cur_length = raw_getlength(bs);

Shouldn't this one still call the raw_co_getlength() wrapper?

> @@ -3245,7 +3250,7 @@ static int coroutine_fn raw_co_block_status(BlockDriverState *bs,
>           * round up if necessary.
>           */
>          if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
> -            int64_t file_length = raw_co_getlength(bs);
> +            int64_t file_length = raw_getlength(bs);

Likewise this one?

>  
>  static bool coroutine_fn cdrom_co_is_inserted(BlockDriverState *bs)
>  {
> -    return raw_co_getlength(bs) > 0;
> +    return raw_getlength(bs) > 0;
>  }

and this one?
Paolo Bonzini June 1, 2023, 2:22 p.m. UTC | #2 
Il gio 1 giu 2023, 15:50 Eric Blake <eblake@redhat.com> ha scritto:

> > @@ -2696,7 +2696,7 @@ static int coroutine_fn
> raw_co_truncate(BlockDriverState *bs, int64_t offset,
> >      }
> >
> >      if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
> > -        int64_t cur_length = raw_co_getlength(bs);
> > +        int64_t cur_length = raw_getlength(bs);
>
> Shouldn't this one still call the raw_co_getlength() wrapper?
>

It could, but instead I wanted to clarify that this will never suspend
(because it's calling the function directly rather than bdrv_co_getlength).

Paolo


> > @@ -3245,7 +3250,7 @@ static int coroutine_fn
> raw_co_block_status(BlockDriverState *bs,
> >           * round up if necessary.
> >           */
> >          if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
> > -            int64_t file_length = raw_co_getlength(bs);
> > +            int64_t file_length = raw_getlength(bs);
>
> Likewise this one?
>
> >
> >  static bool coroutine_fn cdrom_co_is_inserted(BlockDriverState *bs)
> >  {
> > -    return raw_co_getlength(bs) > 0;
> > +    return raw_getlength(bs) > 0;
> >  }
>
> and this one?
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>
>
diff mbox series

Patch

diff --git a/block/file-posix.c b/block/file-posix.c
index 942f529f6ffc..5e0afaba69a5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -193,7 +193,7 @@  static int fd_open(BlockDriverState *bs)
     return -EIO;
 }
 
-static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs);
+static int64_t raw_getlength(BlockDriverState *bs);
 
 typedef struct RawPosixAIOData {
     BlockDriverState *bs;
@@ -1974,7 +1974,7 @@  static int handle_aiocb_write_zeroes(void *opaque)
 #ifdef CONFIG_FALLOCATE
     /* Last resort: we are trying to extend the file with zeroed data. This
      * can be done via fallocate(fd, 0) */
-    len = raw_co_getlength(aiocb->bs);
+    len = raw_getlength(aiocb->bs);
     if (s->has_fallocate && len >= 0 && aiocb->aio_offset >= len) {
         int ret = do_fallocate(s->fd, 0, aiocb->aio_offset, aiocb->aio_nbytes);
         if (ret == 0 || ret != -ENOTSUP) {
@@ -2696,7 +2696,7 @@  static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
     }
 
     if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-        int64_t cur_length = raw_co_getlength(bs);
+        int64_t cur_length = raw_getlength(bs);
 
         if (offset != cur_length && exact) {
             error_setg(errp, "Cannot resize device files");
@@ -2714,7 +2714,7 @@  static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
 }
 
 #ifdef __OpenBSD__
-static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
+static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
     int fd = s->fd;
@@ -2733,7 +2733,7 @@  static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
         return st.st_size;
 }
 #elif defined(__NetBSD__)
-static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
+static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
     int fd = s->fd;
@@ -2758,7 +2758,7 @@  static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
         return st.st_size;
 }
 #elif defined(__sun__)
-static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
+static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
     struct dk_minfo minfo;
@@ -2789,7 +2789,7 @@  static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
     return size;
 }
 #elif defined(CONFIG_BSD)
-static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
+static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
     int fd = s->fd;
@@ -2861,7 +2861,7 @@  again:
     return size;
 }
 #else
-static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
+static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
     int ret;
@@ -2880,6 +2880,11 @@  static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
 }
 #endif
 
+static int64_t coroutine_fn raw_co_getlength(BlockDriverState *bs)
+{
+    return raw_getlength(bs);
+}
+
 static int64_t coroutine_fn raw_co_get_allocated_file_size(BlockDriverState *bs)
 {
     struct stat st;
@@ -3245,7 +3250,7 @@  static int coroutine_fn raw_co_block_status(BlockDriverState *bs,
          * round up if necessary.
          */
         if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
-            int64_t file_length = raw_co_getlength(bs);
+            int64_t file_length = raw_getlength(bs);
             if (file_length > 0) {
                 /* Ignore errors, this is just a safeguard */
                 assert(hole == file_length);
@@ -3267,7 +3272,7 @@  static int coroutine_fn raw_co_block_status(BlockDriverState *bs,
 
 #if defined(__linux__)
 /* Verify that the file is not in the page cache */
-static void coroutine_fn check_cache_dropped(BlockDriverState *bs, Error **errp)
+static void check_cache_dropped(BlockDriverState *bs, Error **errp)
 {
     const size_t window_size = 128 * 1024 * 1024;
     BDRVRawState *s = bs->opaque;
@@ -3282,7 +3287,7 @@  static void coroutine_fn check_cache_dropped(BlockDriverState *bs, Error **errp)
     page_size = sysconf(_SC_PAGESIZE);
     vec = g_malloc(DIV_ROUND_UP(window_size, page_size));
 
-    end = raw_co_getlength(bs);
+    end = raw_getlength(bs);
 
     for (offset = 0; offset < end; offset += window_size) {
         void *new_window;
@@ -4504,7 +4509,7 @@  static int cdrom_reopen(BlockDriverState *bs)
 
 static bool coroutine_fn cdrom_co_is_inserted(BlockDriverState *bs)
 {
-    return raw_co_getlength(bs) > 0;
+    return raw_getlength(bs) > 0;
 }
 
 static void coroutine_fn cdrom_co_eject(BlockDriverState *bs, bool eject_flag)