qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PULL 00/20] Block patches


From: Stefan Hajnoczi
Subject: Re: [PULL 00/20] Block patches
Date: Mon, 24 Apr 2023 13:52:51 -0400

On Fri, Apr 21, 2023 at 08:01:56PM +0100, Richard Henderson wrote:
> On 4/20/23 13:09, Stefan Hajnoczi wrote:
> > The following changes since commit c1eb2ddf0f8075faddc5f7c3d39feae3e8e9d6b4:
> > 
> >    Update version for v8.0.0 release (2023-04-19 17:27:13 +0100)
> > 
> > are available in the Git repository at:
> > 
> >    https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> > 
> > for you to fetch changes up to 36e5e9b22abe56aa00ca067851555ad8127a7966:
> > 
> >    tracing: install trace events file only if necessary (2023-04-20 
> > 07:39:43 -0400)
> > 
> > ----------------------------------------------------------------
> > Pull request
> > 
> > Sam Li's zoned storage work and fixes I collected during the 8.0 freeze.
> > 
> > ----------------------------------------------------------------
> > 
> > Carlos Santos (1):
> >    tracing: install trace events file only if necessary
> > 
> > Philippe Mathieu-Daudé (1):
> >    block/dmg: Declare a type definition for DMG uncompress function
> > 
> > Sam Li (17):
> >    block/block-common: add zoned device structs
> >    block/file-posix: introduce helper functions for sysfs attributes
> >    block/block-backend: add block layer APIs resembling Linux
> >      ZonedBlockDevice ioctls
> >    block/raw-format: add zone operations to pass through requests
> >    block: add zoned BlockDriver check to block layer
> >    iotests: test new zone operations
> >    block: add some trace events for new block layer APIs
> >    docs/zoned-storage: add zoned device documentation
> >    file-posix: add tracking of the zone write pointers
> >    block: introduce zone append write for zoned devices
> >    qemu-iotests: test zone append operation
> >    block: add some trace events for zone append
> >    include: update virtio_blk headers to v6.3-rc1
> >    virtio-blk: add zoned storage emulation for zoned devices
> >    block: add accounting for zone append operation
> >    virtio-blk: add some trace events for zoned emulation
> >    docs/zoned-storage:add zoned emulation use case
> > 
> > Thomas De Schampheleire (1):
> >    tracetool: use relative paths for '#line' preprocessor directives
> 
> 32 failed CI jobs:
> https://gitlab.com/qemu-project/qemu/-/pipelines/844927626/failures

Hi Sam,
I have dropped the zoned storage patches from the block pull request.
Please take a look at the diff below and squash the fixes into your
original commits.

Once you have reworked your patch series, please retest them and then
resend so we can merge them in the next block pull request.

Thank you!

Stefan

---
From 08dd0db534d2dbc3aa41d6147ae99bf589bbed57 Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Mon, 24 Apr 2023 13:46:46 -0400
Subject: [PATCH 1/4] Fix mingw32 32-bit compilation

Add uintptr_t casts where necessary so 32-bit mingw32 builds succeed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/block-backend.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6b2b92b7ff..b9274661fc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1839,7 +1839,8 @@ static void coroutine_fn blk_aio_zone_report_entry(void 
*opaque)
     BlkRwCo *rwco = &acb->rwco;
 
     rwco->ret = blk_co_zone_report(rwco->blk, rwco->offset,
-                                   (unsigned int*)acb->bytes,rwco->iobuf);
+                                   (unsigned int *)(uintptr_t)acb->bytes,
+                                   rwco->iobuf);
     blk_aio_complete(acb);
 }
 
@@ -1860,7 +1861,7 @@ BlockAIOCB *blk_aio_zone_report(BlockBackend *blk, 
int64_t offset,
         .iobuf  = zones,
         .ret    = NOT_DONE,
     };
-    acb->bytes = (int64_t)nr_zones,
+    acb->bytes = (int64_t)(uintptr_t)nr_zones,
     acb->has_returned = false;
 
     co = qemu_coroutine_create(blk_aio_zone_report_entry, acb);
@@ -1880,7 +1881,8 @@ static void coroutine_fn blk_aio_zone_mgmt_entry(void 
*opaque)
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
 
-    rwco->ret = blk_co_zone_mgmt(rwco->blk, (BlockZoneOp)rwco->iobuf,
+    rwco->ret = blk_co_zone_mgmt(rwco->blk,
+                                 (BlockZoneOp)(uintptr_t)rwco->iobuf,
                                  rwco->offset, acb->bytes);
     blk_aio_complete(acb);
 }
@@ -1897,7 +1899,7 @@ BlockAIOCB *blk_aio_zone_mgmt(BlockBackend *blk, 
BlockZoneOp op,
     acb->rwco = (BlkRwCo) {
         .blk    = blk,
         .offset = offset,
-        .iobuf  = (void *)op,
+        .iobuf  = (void *)(uintptr_t)op,
         .ret    = NOT_DONE,
     };
     acb->bytes = len;
@@ -1920,7 +1922,7 @@ static void coroutine_fn blk_aio_zone_append_entry(void 
*opaque)
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
 
-    rwco->ret = blk_co_zone_append(rwco->blk, (int64_t *)acb->bytes,
+    rwco->ret = blk_co_zone_append(rwco->blk, (int64_t *)(uintptr_t)acb->bytes,
                                    rwco->iobuf, rwco->flags);
     blk_aio_complete(acb);
 }
@@ -1940,7 +1942,7 @@ BlockAIOCB *blk_aio_zone_append(BlockBackend *blk, 
int64_t *offset,
         .flags  = flags,
         .iobuf  = qiov,
     };
-    acb->bytes = (int64_t)offset;
+    acb->bytes = (int64_t)(uintptr_t)offset;
     acb->has_returned = false;
 
     co = qemu_coroutine_create(blk_aio_zone_append_entry, acb);
-- 
2.39.2


From 517aea71e044885946864ade870716f38c6f067a Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Mon, 24 Apr 2023 13:48:05 -0400
Subject: [PATCH 2/4] Add GRAPH_RDLOCK_GUARD() to zoned blk_co_*() functions

GRAPH_RDLOCK_GUARD() was introduced recently and your
patches were probably written before that happened. When QEMU is
compiled with the LLVM clang compiler, there are errors because the
compiler detects that the graph read lock is not taken. Fixed by
adding GRAPH_RDLOCK_GUARD().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/block-backend.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index b9274661fc..360cde1645 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1971,6 +1971,7 @@ int coroutine_fn blk_co_zone_report(BlockBackend *blk, 
int64_t offset,
 
     blk_inc_in_flight(blk); /* increase before waiting */
     blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
     if (!blk_is_available(blk)) {
         blk_dec_in_flight(blk);
         return -ENOMEDIUM;
@@ -1995,6 +1996,7 @@ int coroutine_fn blk_co_zone_mgmt(BlockBackend *blk, 
BlockZoneOp op,
 
     blk_inc_in_flight(blk);
     blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
 
     ret = blk_check_byte_request(blk, offset, len);
     if (ret < 0) {
@@ -2018,6 +2020,7 @@ int coroutine_fn blk_co_zone_append(BlockBackend *blk, 
int64_t *offset,
 
     blk_inc_in_flight(blk);
     blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
     if (!blk_is_available(blk)) {
         blk_dec_in_flight(blk);
         return -ENOMEDIUM;
-- 
2.39.2


From 6801fe4981e37fba38f452c837ee733b3fce13c4 Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Mon, 24 Apr 2023 13:49:11 -0400
Subject: [PATCH 3/4] Fix #ifdef CONFIG_BLKZONED in block/file-posix

Ensure the code compiles without CONFIG_BLKZONED by placing the #ifdef
around functions only called when zoned storage support is enabled.
Rework the refresh limits function to compile cleanly.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/file-posix.c | 156 +++++++++++++++++++++++++--------------------
 1 file changed, 86 insertions(+), 70 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 972d641538..f2632af1a5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1259,6 +1259,7 @@ static int get_sysfs_str_val(struct stat *st, const char 
*attribute,
 #endif
 }
 
+#if defined(CONFIG_BLKZONED)
 static int get_sysfs_zoned_model(struct stat *st, BlockZoneModel *zoned)
 {
     g_autofree char *val = NULL;
@@ -1280,6 +1281,7 @@ static int get_sysfs_zoned_model(struct stat *st, 
BlockZoneModel *zoned)
     }
     return 0;
 }
+#endif /* defined(CONFIG_BLKZONED) */
 
 /*
  * Get a sysfs attribute value as a long integer.
@@ -1407,14 +1409,93 @@ static void update_zones_wp(BlockDriverState *bs, int 
fd, int64_t offset,
         error_report("update zone wp failed");
     }
 }
-#endif
 
-static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
+static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+                                     Error **errp)
 {
     BDRVRawState *s = bs->opaque;
-    struct stat st;
-    int ret;
     BlockZoneModel zoned;
+    int ret;
+
+    bs->bl.zoned = BLK_Z_NONE;
+
+    ret = get_sysfs_zoned_model(st, &zoned);
+    if (ret < 0 || ret == BLK_Z_NONE) {
+        return;
+    }
+
+    /*
+     * The zoned device must at least have zone size and nr_zones fields.
+     */
+    ret = get_sysfs_long_val(st, "chunk_sectors");
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Unable to read chunk_sectors "
+                                     "sysfs attribute");
+        return;
+    } else if (!ret) {
+        error_setg(errp, "Read 0 from chunk_sectors sysfs attribute");
+        return;
+    }
+    bs->bl.zone_size = ret << BDRV_SECTOR_BITS;
+
+    ret = get_sysfs_long_val(st, "nr_zones");
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Unable to read nr_zones "
+                                     "sysfs attribute");
+        return;
+    } else if (!ret) {
+        error_setg(errp, "Read 0 from nr_zones sysfs attribute");
+        return;
+    }
+    bs->bl.nr_zones = ret;
+
+    ret = get_sysfs_long_val(st, "zone_append_max_bytes");
+    if (ret > 0) {
+        bs->bl.max_append_sectors = ret >> BDRV_SECTOR_BITS;
+    }
+
+    ret = get_sysfs_long_val(st, "max_open_zones");
+    if (ret >= 0) {
+        bs->bl.max_open_zones = ret;
+    }
+
+    ret = get_sysfs_long_val(st, "max_active_zones");
+    if (ret >= 0) {
+        bs->bl.max_active_zones = ret;
+    }
+
+    ret = get_sysfs_long_val(st, "physical_block_size");
+    if (ret >= 0) {
+        bs->bl.write_granularity = ret;
+    }
+
+    bs->bl.zoned = zoned;
+
+    /* The refresh_limits() function can be called multiple times. */
+    g_free(bs->wps);
+    bs->wps = g_malloc(sizeof(BlockZoneWps) +
+            sizeof(int64_t) * bs->bl.nr_zones);
+    ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "report wps failed");
+        bs->bl.zoned = BLK_Z_NONE;
+        bs->wps = NULL;
+        return;
+    }
+    qemu_co_mutex_init(&bs->wps->colock);
+}
+#else /* !defined(CONFIG_BLKZONED) */
+static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+                                     Error **errp)
+{
+    bs->bl.zoned = BLK_Z_NONE;
+}
+#endif /* !defined(CONFIG_BLKZONED) */
+
+static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    BDRVRawState *s = bs->opaque;
+    struct stat st;
 
     s->needs_alignment = raw_needs_alignment(bs);
     raw_probe_alignment(bs, s->fd, errp);
@@ -1453,72 +1534,7 @@ static void raw_refresh_limits(BlockDriverState *bs, 
Error **errp)
         }
     }
 
-    ret = get_sysfs_zoned_model(&st, &zoned);
-    if (ret < 0) {
-        zoned = BLK_Z_NONE;
-    }
-    bs->bl.zoned = zoned;
-    if (zoned != BLK_Z_NONE) {
-        /*
-         * The zoned device must at least have zone size and nr_zones fields.
-         */
-        ret = get_sysfs_long_val(&st, "chunk_sectors");
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Unable to read chunk_sectors "
-                                         "sysfs attribute");
-            goto out;
-        } else if (!ret) {
-            error_setg(errp, "Read 0 from chunk_sectors sysfs attribute");
-            goto out;
-        }
-        bs->bl.zone_size = ret << BDRV_SECTOR_BITS;
-
-        ret = get_sysfs_long_val(&st, "nr_zones");
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Unable to read nr_zones "
-                                         "sysfs attribute");
-            goto out;
-        } else if (!ret) {
-            error_setg(errp, "Read 0 from nr_zones sysfs attribute");
-            goto out;
-        }
-        bs->bl.nr_zones = ret;
-
-        ret = get_sysfs_long_val(&st, "zone_append_max_bytes");
-        if (ret > 0) {
-            bs->bl.max_append_sectors = ret >> BDRV_SECTOR_BITS;
-        }
-
-        ret = get_sysfs_long_val(&st, "max_open_zones");
-        if (ret >= 0) {
-            bs->bl.max_open_zones = ret;
-        }
-
-        ret = get_sysfs_long_val(&st, "max_active_zones");
-        if (ret >= 0) {
-            bs->bl.max_active_zones = ret;
-        }
-
-        ret = get_sysfs_long_val(&st, "physical_block_size");
-        if (ret >= 0) {
-            bs->bl.write_granularity = ret;
-        }
-
-        /* The refresh_limits() function can be called multiple times. */
-        g_free(bs->wps);
-        bs->wps = g_malloc(sizeof(BlockZoneWps) +
-                sizeof(int64_t) * bs->bl.nr_zones);
-        ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "report wps failed");
-            bs->wps = NULL;
-            return;
-        }
-        qemu_co_mutex_init(&bs->wps->colock);
-        return;
-    }
-out:
-    bs->bl.zoned = BLK_Z_NONE;
+    raw_refresh_zoned_limits(bs, &st, errp);
 }
 
 static int check_for_dasd(int fd)
-- 
2.39.2


From 2e15b856cba2caf9a7635aabf82fd6d8efa1618d Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Mon, 24 Apr 2023 13:50:32 -0400
Subject: [PATCH 4/4] Fix declaration after label warning

There is a compiler warning that a label must come right before a
statement. A variable declaration is not a statement (I didn't know
this), so compilation fails. Use a {} statement to fix this.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/file-posix.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index f2632af1a5..59c25436d6 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2512,31 +2512,33 @@ static int coroutine_fn raw_co_prw(BlockDriverState 
*bs, uint64_t offset,
 
 out:
 #if defined(CONFIG_BLKZONED)
-    BlockZoneWps *wps = bs->wps;
-    if (ret == 0) {
-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))
-            && wps && bs->bl.zone_size) {
-            uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
-            if (!BDRV_ZT_IS_CONV(*wp)) {
-                if (type & QEMU_AIO_ZONE_APPEND) {
-                    *s->offset = *wp;
-                    trace_zbd_zone_append_complete(bs, *s->offset
-                        >> BDRV_SECTOR_BITS);
-                }
-                /* Advance the wp if needed */
-                if (offset + bytes > *wp) {
-                    *wp = offset + bytes;
+    {
+        BlockZoneWps *wps = bs->wps;
+        if (ret == 0) {
+            if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))
+                && wps && bs->bl.zone_size) {
+                uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
+                if (!BDRV_ZT_IS_CONV(*wp)) {
+                    if (type & QEMU_AIO_ZONE_APPEND) {
+                        *s->offset = *wp;
+                        trace_zbd_zone_append_complete(bs, *s->offset
+                            >> BDRV_SECTOR_BITS);
+                    }
+                    /* Advance the wp if needed */
+                    if (offset + bytes > *wp) {
+                        *wp = offset + bytes;
+                    }
                 }
             }
+        } else {
+            if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
+                update_zones_wp(bs, s->fd, 0, 1);
+            }
         }
-    } else {
-        if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
-            update_zones_wp(bs, s->fd, 0, 1);
-        }
-    }
 
-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && wps) {
-        qemu_co_mutex_unlock(&wps->colock);
+        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && wps) {
+            qemu_co_mutex_unlock(&wps->colock);
+        }
     }
 #endif
     return ret;
-- 
2.39.2

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]