[Qemu-devel] [PATCH v3 0/4] virtio/block: handle zoned backing devices

[Qemu-devel] [PATCH v3 0/4] virtio/block: handle zoned backing devices

[Dmitry Fomichev]

Currently, attaching zoned block devices (i.e., storage devices
compliant to ZAC/ZBC standards) using several virtio methods doesn't
work properly as zoned devices appear as regular block devices at the
guest. This may cause unexpected i/o errors and, potentially, some
data corruption.

To be more precise, attaching a zoned device via virtio-pci-blk,
virtio-scsi-pci/scsi-disk or virtio-scsi-pci/scsi-hd demonstrates the
above behavior. The virtio-scsi-pci/scsi-block method works with a
recent patch. The virtio-scsi-pci/scsi-generic method also appears to
handle zoned devices without problems.

This patch set adds code to check if the backing device that is being
opened is a zoned Host Managed device. If this is the case, the patch
prohibits attaching such device for all use cases lacking proper
zoned support.

Host Aware zoned block devices are designed to work as regular block
devices at a guest system that does not support ZBD. Therefore, this
patch set doesn't prohibit attachment of Host Aware devices.

Considering that there is still a couple of different working ways
to attach a ZBD, this patch set provides a reasonable short-term
solution for this problem. What about long term?

It appears to be beneficial to add proper ZBD support to virtio-blk.
In order to support this use case properly, some virtio-blk protocol
changes will be necessary. They are needed to allow the host code to
propagate some ZBD properties that are required for virtio guest
driver to configure the guest block device as ZBD, such as zoned
device model, zone size and the total number of zones. Further, some
support needs to be added for REPORT ZONES command as well as for zone
operations, such as OPEN ZONE, CLOSE ZONE, FINISH ZONE and RESET ZONE.

These additions to the protocol are relatively straightforward, but
they need to be approved by the virtio TC and the whole process may
take some time.

ZBD support for virtio-scsi-pci/scsi-disk and virtio-scsi-pci/scsi-hd
does not seem as necessary. Users will be expected to attach zoned
block devices via virtio-scsi-pci/scsi-block instead.

This patch set contains some Linux-specific code. This code is
necessary to obtain Zoned Block Device model value from Linux sysfs.

History:

v1 -> v2:
- rework the code to be permission-based
- always allow Host Aware devices to be attached
- add fix for Host Aware attachments aka RCAP output snoop

v2 -> v3:
- drop the patch for RCAP output snoop - merged separately


Dmitry Fomichev (4):
  block: Add zoned device model property
  raw: Recognize zoned backing devices
  block/ide/scsi: Set BLK_PERM_SUPPORT_ZONED
  raw: Don't open ZBDs if backend can't handle them

 block.c                   | 19 +++++++++
 block/file-posix.c        | 88 +++++++++++++++++++++++++++++++++------
 block/raw-format.c        |  8 ++++
 hw/block/block.c          |  8 +++-
 hw/block/fdc.c            |  4 +-
 hw/block/nvme.c           |  2 +-
 hw/block/virtio-blk.c     |  2 +-
 hw/block/xen-block.c      |  2 +-
 hw/ide/qdev.c             |  2 +-
 hw/scsi/scsi-disk.c       | 13 +++---
 hw/scsi/scsi-generic.c    |  2 +-
 hw/usb/dev-storage.c      |  2 +-
 include/block/block.h     | 21 +++++++++-
 include/block/block_int.h |  4 ++
 include/hw/block/block.h  |  3 +-
 15 files changed, 150 insertions(+), 30 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v3 1/4] block: Add zoned device model property

[Dmitry Fomichev]

This commit adds Zoned Device Model (as defined in T10 ZBC and
T13 ZAC standards) as a block driver property, along with some
useful access functions.

A new backend driver permission, BLK_PERM_SUPPORT_ZONED, is also
introduced. Only the drivers having this permission will be allowed
to open zoned block devices.

No code is added yet to initialize or check the value of this new
property, therefore this commit doesn't change any functionality.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 block.c                   | 19 +++++++++++++++++++
 include/block/block.h     | 21 ++++++++++++++++++++-
 include/block/block_int.h |  4 ++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index cbd8da5f3b..c717c4d6f5 100644
--- a/block.c
+++ b/block.c
@@ -4667,6 +4667,25 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
     *nb_sectors_ptr = nb_sectors < 0 ? 0 : nb_sectors;
 }
 
+uint8_t bdrv_get_zoned_model(BlockDriverState *bs)
+{
+    if (bs->drv->bdrv_get_zoned_info) {
+        bs->drv->bdrv_get_zoned_info(bs);
+    }
+
+    return bs->bl.zoned_model;
+}
+
+uint8_t bdrv_is_zoned(BlockDriverState *bs)
+{
+    /*
+     * Host Aware zone devices are supposed to be able to work
+     * just like regular block devices. Thus, we only consider
+     * Host Managed devices to be zoned here.
+     */
+    return bdrv_get_zoned_model(bs) == BLK_ZONED_MODEL_HM;
+}
+
 bool bdrv_is_sg(BlockDriverState *bs)
 {
     return bs->sg;
diff --git a/include/block/block.h b/include/block/block.h
index 50a07c1c33..bd98933f67 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -266,18 +266,35 @@ enum {
      */
     BLK_PERM_GRAPH_MOD          = 0x10,
 
+    /** This permission is required to open zoned block devices. */
+    BLK_PERM_SUPPORT_ZONED      = 0x20,
+
     BLK_PERM_ALL                = 0x1f,
 
     DEFAULT_PERM_PASSTHROUGH    = BLK_PERM_CONSISTENT_READ
                                  | BLK_PERM_WRITE
                                  | BLK_PERM_WRITE_UNCHANGED
-                                 | BLK_PERM_RESIZE,
+                                 | BLK_PERM_RESIZE
+                                 | BLK_PERM_SUPPORT_ZONED,
 
     DEFAULT_PERM_UNCHANGED      = BLK_PERM_ALL & ~DEFAULT_PERM_PASSTHROUGH,
 };
 
 char *bdrv_perm_names(uint64_t perm);
 
+/*
+ * Known zoned device models.
+ *
+ * TODO For a Linux host, it could be preferrable to include
+ * /usr/include/linux/blkzoned.h instead of defining ZBD-specific
+ * values here.
+ */
+enum blk_zoned_model {
+    BLK_ZONED_MODEL_NONE, /* Regular block device */
+    BLK_ZONED_MODEL_HA,   /* Host-aware zoned block device */
+    BLK_ZONED_MODEL_HM,   /* Host-managed zoned block device */
+};
+
 /* disk I/O throttling */
 void bdrv_init(void);
 void bdrv_init_with_whitelist(void);
@@ -354,6 +371,8 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
                                BlockDriverState *in_bs, Error **errp);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
+uint8_t bdrv_get_zoned_model(BlockDriverState *bs);
+uint8_t bdrv_is_zoned(BlockDriverState *bs);
 void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
 int bdrv_commit(BlockDriverState *bs);
 int bdrv_change_backing_file(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3aa1e832a8..52c5758a9d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -416,6 +416,7 @@ struct BlockDriver {
     bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *tag);
 
     void (*bdrv_refresh_limits)(BlockDriverState *bs, Error **errp);
+    void (*bdrv_get_zoned_info)(BlockDriverState *bs);
 
     /*
      * Returns 1 if newly created images are guaranteed to contain only
@@ -614,6 +615,9 @@ typedef struct BlockLimits {
 
     /* maximum number of iovec elements */
     int max_iov;
+
+    /* Zoned device model. Zero value indicates a regular block device */
+    uint8_t zoned_model;
 } BlockLimits;
 
 typedef struct BdrvOpBlocker BdrvOpBlocker;
-- 
2.21.0

[Qemu-devel] [PATCH v3 2/4] raw: Recognize zoned backing devices

[Dmitry Fomichev]

The purpose of this patch is to recognize a zoned block device (ZBD)
when it is opened as a raw file. The new code initializes the zoned
model propery introduced by the previous commit.

This commit is Linux-specific as it gets the Zoned Block Device Model
value (none/host-managed/host-aware) from sysfs on the host.

In order to avoid code duplication in file-posix.c, a common helper
function is added to read values of sysfs entries under
/sys/block/<dev>/queue. This way, the existing function that reads
the value of "max_segments" entry and the the new function that reads
"zoned" value both share the same helper code.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 block/file-posix.c | 74 ++++++++++++++++++++++++++++++++++++++--------
 block/raw-format.c |  8 +++++
 2 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 4479cc7ab4..e307cab7a4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1053,15 +1053,13 @@ static int sg_get_max_transfer_length(int fd)
 #endif
 }
 
-static int sg_get_max_segments(int fd)
+static int hdev_read_blk_queue_entry(int fd, const char *key,
+    char *buf, int buf_len)
 {
 #ifdef CONFIG_LINUX
-    char buf[32];
-    const char *end;
     char *sysfspath = NULL;
     int ret;
     int sysfd = -1;
-    long max_segments;
     struct stat st;
 
     if (fstat(fd, &st)) {
@@ -1069,23 +1067,45 @@ static int sg_get_max_segments(int fd)
         goto out;
     }
 
-    sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
-                                major(st.st_rdev), minor(st.st_rdev));
+    sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/%s",
+                                major(st.st_rdev), minor(st.st_rdev), key);
     sysfd = open(sysfspath, O_RDONLY);
     if (sysfd == -1) {
         ret = -errno;
         goto out;
     }
     do {
-        ret = read(sysfd, buf, sizeof(buf) - 1);
+        ret = read(sysfd, buf, buf_len - 1);
     } while (ret == -1 && errno == EINTR);
     if (ret < 0) {
         ret = -errno;
-        goto out;
     } else if (ret == 0) {
         ret = -EIO;
+    }
+out:
+    if (sysfd != -1) {
+        close(sysfd);
+    }
+    g_free(sysfspath);
+    return ret;
+#else
+    return -ENOTSUP;
+#endif
+}
+
+static int sg_get_max_segments(int fd)
+{
+#ifdef CONFIG_LINUX
+    char buf[32];
+    const char *end;
+    int ret;
+    long max_segments;
+
+    ret = hdev_read_blk_queue_entry(fd, "max_segments", buf, sizeof(buf));
+    if (ret < 0) {
         goto out;
     }
+
     buf[ret] = 0;
     /* The file is ended with '\n', pass 'end' to accept that. */
     ret = qemu_strtol(buf, &end, 10, &max_segments);
@@ -1094,10 +1114,33 @@ static int sg_get_max_segments(int fd)
     }
 
 out:
-    if (sysfd != -1) {
-        close(sysfd);
+    return ret;
+#else
+    return -ENOTSUP;
+#endif
+}
+
+static int hdev_get_zoned_model(int fd)
+{
+#ifdef CONFIG_LINUX
+    char buf[32];
+    int ret;
+
+    ret = hdev_read_blk_queue_entry(fd, "zoned", buf, sizeof(buf));
+    if (ret < 0) {
+        ret = BLK_ZONED_MODEL_NONE;
+        goto out;
     }
-    g_free(sysfspath);
+
+    buf[ret - 1] = 0;
+    ret = BLK_ZONED_MODEL_NONE;
+    if (strcmp(buf, "host-managed") == 0) {
+        ret = BLK_ZONED_MODEL_HM;
+    } else if (strcmp(buf, "host-aware") == 0) {
+        ret = BLK_ZONED_MODEL_HA;
+    }
+
+out:
     return ret;
 #else
     return -ENOTSUP;
@@ -1107,9 +1150,10 @@ out:
 static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
+    int ret;
 
     if (bs->sg) {
-        int ret = sg_get_max_transfer_length(s->fd);
+        ret = sg_get_max_transfer_length(s->fd);
 
         if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
             bs->bl.max_transfer = pow2floor(ret);
@@ -1119,6 +1163,12 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
         if (ret > 0) {
             bs->bl.max_transfer = MIN(bs->bl.max_transfer, ret * getpagesize());
         }
+
+    }
+
+    ret = hdev_get_zoned_model(s->fd);
+    if (ret >= 0) {
+        bs->bl.zoned_model = ret;
     }
 
     raw_probe_alignment(bs, s->fd, errp);
diff --git a/block/raw-format.c b/block/raw-format.c
index bffd424dd0..12c2a3f95d 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -369,6 +369,13 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
     }
 }
 
+static void raw_get_zoned_info(BlockDriverState *bs)
+{
+    if (!bs->probed) {
+        bs->bl.zoned_model = bs->file->bs->bl.zoned_model;
+    }
+}
+
 static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
                                         PreallocMode prealloc, Error **errp)
 {
@@ -572,6 +579,7 @@ BlockDriver bdrv_raw = {
     .bdrv_co_ioctl        = &raw_co_ioctl,
     .create_opts          = &raw_create_opts,
     .bdrv_has_zero_init   = &raw_has_zero_init,
+    .bdrv_get_zoned_info  = &raw_get_zoned_info,
     .strong_runtime_opts  = raw_strong_runtime_opts,
     .mutable_opts         = mutable_opts,
 };
-- 
2.21.0

[Qemu-devel] [PATCH v3 3/4] block/ide/scsi: Set BLK_PERM_SUPPORT_ZONED

[Dmitry Fomichev]

Added a new boolean argument to blkconf_apply_backend_options()
to let the common block code know whether the chosen block
backend can handle zoned block devices or not.

blkconf_apply_backend_options() then sets BLK_PERM_SUPPORT_ZONED
permission accordingly. The raw code can then use this permission
to allow or deny opening a zone device by a particular block driver.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Acked-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/block/block.c         |  8 ++++++--
 hw/block/fdc.c           |  4 ++--
 hw/block/nvme.c          |  2 +-
 hw/block/virtio-blk.c    |  2 +-
 hw/block/xen-block.c     |  2 +-
 hw/ide/qdev.c            |  2 +-
 hw/scsi/scsi-disk.c      | 13 +++++++------
 hw/scsi/scsi-generic.c   |  2 +-
 hw/usb/dev-storage.c     |  2 +-
 include/hw/block/block.h |  3 ++-
 10 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index bf56c7612b..23fbe4d567 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -86,7 +86,8 @@ void blkconf_blocksizes(BlockConf *conf)
 }
 
 bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
-                                   bool resizable, Error **errp)
+                                   bool resizable, bool zoned_support,
+                                   Error **errp)
 {
     BlockBackend *blk = conf->blk;
     BlockdevOnError rerror, werror;
@@ -98,9 +99,12 @@ bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
     if (!readonly) {
         perm |= BLK_PERM_WRITE;
     }
+    if (zoned_support) {
+        perm |= BLK_PERM_SUPPORT_ZONED;
+    }
 
     shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
-                  BLK_PERM_GRAPH_MOD;
+                  BLK_PERM_GRAPH_MOD | BLK_PERM_SUPPORT_ZONED;
     if (resizable) {
         shared_perm |= BLK_PERM_RESIZE;
     }
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 77af9979de..85efc80992 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -474,7 +474,7 @@ static void fd_change_cb(void *opaque, bool load, Error **errp)
     } else {
         if (!blkconf_apply_backend_options(drive->conf,
                                            blk_is_read_only(drive->blk), false,
-                                           errp)) {
+                                           false, errp)) {
             return;
         }
     }
@@ -561,7 +561,7 @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
 
     if (!blkconf_apply_backend_options(&dev->conf,
                                        blk_is_read_only(dev->conf.blk),
-                                       false, errp)) {
+                                       false, false, errp)) {
         return;
     }
 
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 36d6a8bb3a..71b35bf4e7 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1333,7 +1333,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
     }
     blkconf_blocksizes(&n->conf);
     if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
-                                       false, errp)) {
+                                       false, false, errp)) {
         return;
     }
 
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index cbb3729158..8894bdbb0c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1123,7 +1123,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
 
     if (!blkconf_apply_backend_options(&conf->conf,
                                        blk_is_read_only(conf->conf.blk), true,
-                                       errp)) {
+                                       false, errp)) {
         return;
     }
     s->original_wce = blk_enable_write_cache(conf->conf.blk);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 69d73196e2..8ed5e9d832 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -228,7 +228,7 @@ static void xen_block_realize(XenDevice *xendev, Error **errp)
     }
 
     if (!blkconf_apply_backend_options(conf, blockdev->info & VDISK_READONLY,
-                                       true, errp)) {
+                                       true, false, errp)) {
         return;
     }
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 9d8502785d..c0b4a445e4 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -197,7 +197,7 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
         }
     }
     if (!blkconf_apply_backend_options(&dev->conf, kind == IDE_CD,
-                                       kind != IDE_CD, errp)) {
+                                       kind != IDE_CD, false, errp)) {
         return;
     }
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 8e95e3e38d..f20815b1d7 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2315,7 +2315,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
     }
 }
 
-static void scsi_realize(SCSIDevice *dev, Error **errp)
+static void scsi_realize(SCSIDevice *dev, bool zoned_support, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
 
@@ -2353,7 +2353,8 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
     }
     if (!blkconf_apply_backend_options(&dev->conf,
                                        blk_is_read_only(s->qdev.conf.blk),
-                                       dev->type == TYPE_DISK, errp)) {
+                                       dev->type == TYPE_DISK, zoned_support,
+                                       errp)) {
         return;
     }
 
@@ -2412,7 +2413,7 @@ static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
     if (!s->product) {
         s->product = g_strdup("QEMU HARDDISK");
     }
-    scsi_realize(&s->qdev, errp);
+    scsi_realize(&s->qdev, false, errp);
     if (ctx) {
         aio_context_release(ctx);
     }
@@ -2440,7 +2441,7 @@ static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
     if (!s->product) {
         s->product = g_strdup("QEMU CD-ROM");
     }
-    scsi_realize(&s->qdev, errp);
+    scsi_realize(&s->qdev, false, errp);
     aio_context_release(ctx);
 }
 
@@ -2450,7 +2451,7 @@ static void scsi_disk_realize(SCSIDevice *dev, Error **errp)
     Error *local_err = NULL;
 
     if (!dev->conf.blk) {
-        scsi_realize(dev, &local_err);
+        scsi_realize(dev, false, &local_err);
         assert(local_err);
         error_propagate(errp, local_err);
         return;
@@ -2643,7 +2644,7 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
      */
     s->features |= (1 << SCSI_DISK_F_NO_REMOVABLE_DEVOPS);
 
-    scsi_realize(&s->qdev, errp);
+    scsi_realize(&s->qdev, true, errp);
     scsi_generic_read_device_inquiry(&s->qdev);
 
 out:
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index c11a0c9a84..50b30e9541 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -690,7 +690,7 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp)
     }
     if (!blkconf_apply_backend_options(&s->conf,
                                        blk_is_read_only(s->conf.blk),
-                                       true, errp)) {
+                                       true, true, errp)) {
         return;
     }
 
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 9ffb88ea5b..60d6a92ce1 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -601,7 +601,7 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
 
     blkconf_blocksizes(&s->conf);
     if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,
-                                       errp)) {
+                                       false, errp)) {
         return;
     }
 
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 607539057a..f988edc87e 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -85,7 +85,8 @@ bool blkconf_geometry(BlockConf *conf, int *trans,
                       Error **errp);
 void blkconf_blocksizes(BlockConf *conf);
 bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
-                                   bool resizable, Error **errp);
+                                   bool resizable, bool zoned_support,
+                                   Error **errp);
 
 /* Hard disk geometry */
 
-- 
2.21.0

[Qemu-devel] [PATCH v3 4/4] raw: Don't open ZBDs if backend can't handle them

[Dmitry Fomichev]

Abort opening a zoned device as a raw file in case the chosen
block backend driver lacks proper support for this type of
storage.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 block/file-posix.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index e307cab7a4..507dba98c5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2870,6 +2870,20 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared,
             goto fail;
         }
     }
+
+    /*
+     * If we are opening a zoned block device, check if the backend
+     * driver can properly handle such devices, abort if not.
+     */
+    if (bdrv_is_zoned(bs) &&
+        (shared & BLK_PERM_SUPPORT_ZONED) &&
+        !(perm & BLK_PERM_SUPPORT_ZONED)) {
+        error_setg(errp,
+                   "block backend driver doesn't support HM zoned devices");
+        ret = -ENOTSUP;
+        goto fail;
+    }
+
     return 0;
 
 fail:
-- 
2.21.0

Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/4] virtio/block: handle zoned backing devices

[John Snow]

On 7/23/19 6:19 PM, Dmitry Fomichev wrote:
> Currently, attaching zoned block devices (i.e., storage devices
> compliant to ZAC/ZBC standards) using several virtio methods doesn't
> work properly as zoned devices appear as regular block devices at the
> guest. This may cause unexpected i/o errors and, potentially, some
> data corruption.
> 

Hi, I'm quite uninitiated here, what's a zoned block device? What are
the ZAC/ZBC standards?

I've found this:
https://www.snia.org/sites/default/files/SDC/2016/presentations/smr/DamienLeMoal_ZBC-ZAC_Linux.pdf

It looks like ZAC/ZBC are new commands -- what happens if we just don't
use them, exactly?

> To be more precise, attaching a zoned device via virtio-pci-blk,
> virtio-scsi-pci/scsi-disk or virtio-scsi-pci/scsi-hd demonstrates the
> above behavior. The virtio-scsi-pci/scsi-block method works with a
> recent patch. The virtio-scsi-pci/scsi-generic method also appears to
> handle zoned devices without problems.
> 

What exactly fails, out of curiosity?

Naively, it seems strange to me that you'd have something that presents
itself as a block device but can't be used like one. Usually I expect to
see new features / types of devices used inefficiently when we aren't
aware of a special attribute/property they have, but not create data
corruption.

The only reason I ask is because it seems odd that you need to add a
special flag to e.g. legacy IDE devices that explicitly says they don't
support zoned block devices -- instead of adding flags to virtio devices
that say they explicitly do support that feature set.

--js

> This patch set adds code to check if the backing device that is being
> opened is a zoned Host Managed device. If this is the case, the patch
> prohibits attaching such device for all use cases lacking proper
> zoned support.
> 
> Host Aware zoned block devices are designed to work as regular block
> devices at a guest system that does not support ZBD. Therefore, this
> patch set doesn't prohibit attachment of Host Aware devices.
> 
> Considering that there is still a couple of different working ways
> to attach a ZBD, this patch set provides a reasonable short-term
> solution for this problem. What about long term?
> 
> It appears to be beneficial to add proper ZBD support to virtio-blk.
> In order to support this use case properly, some virtio-blk protocol
> changes will be necessary. They are needed to allow the host code to
> propagate some ZBD properties that are required for virtio guest
> driver to configure the guest block device as ZBD, such as zoned
> device model, zone size and the total number of zones. Further, some
> support needs to be added for REPORT ZONES command as well as for zone
> operations, such as OPEN ZONE, CLOSE ZONE, FINISH ZONE and RESET ZONE.
> 
> These additions to the protocol are relatively straightforward, but
> they need to be approved by the virtio TC and the whole process may
> take some time.
> 
> ZBD support for virtio-scsi-pci/scsi-disk and virtio-scsi-pci/scsi-hd
> does not seem as necessary. Users will be expected to attach zoned
> block devices via virtio-scsi-pci/scsi-block instead.
> 
> This patch set contains some Linux-specific code. This code is
> necessary to obtain Zoned Block Device model value from Linux sysfs.
> 
> History:
> 
> v1 -> v2:
> - rework the code to be permission-based
> - always allow Host Aware devices to be attached
> - add fix for Host Aware attachments aka RCAP output snoop
> 
> v2 -> v3:
> - drop the patch for RCAP output snoop - merged separately
> 
> 
> Dmitry Fomichev (4):
>   block: Add zoned device model property
>   raw: Recognize zoned backing devices
>   block/ide/scsi: Set BLK_PERM_SUPPORT_ZONED
>   raw: Don't open ZBDs if backend can't handle them
> 
>  block.c                   | 19 +++++++++
>  block/file-posix.c        | 88 +++++++++++++++++++++++++++++++++------
>  block/raw-format.c        |  8 ++++
>  hw/block/block.c          |  8 +++-
>  hw/block/fdc.c            |  4 +-
>  hw/block/nvme.c           |  2 +-
>  hw/block/virtio-blk.c     |  2 +-
>  hw/block/xen-block.c      |  2 +-
>  hw/ide/qdev.c             |  2 +-
>  hw/scsi/scsi-disk.c       | 13 +++---
>  hw/scsi/scsi-generic.c    |  2 +-
>  hw/usb/dev-storage.c      |  2 +-
>  include/block/block.h     | 21 +++++++++-
>  include/block/block_int.h |  4 ++
>  include/hw/block/block.h  |  3 +-
>  15 files changed, 150 insertions(+), 30 deletions(-)
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/4] virtio/block: handle zoned backing devices

[Dmitry Fomichev]

John, please see inline...

Regards,
Dmitry

On Thu, 2019-07-25 at 13:58 -0400, John Snow wrote:
> 
> On 7/23/19 6:19 PM, Dmitry Fomichev wrote:
> > Currently, attaching zoned block devices (i.e., storage devices
> > compliant to ZAC/ZBC standards) using several virtio methods doesn't
> > work properly as zoned devices appear as regular block devices at the
> > guest. This may cause unexpected i/o errors and, potentially, some
> > data corruption.
> > 
> 
> Hi, I'm quite uninitiated here, what's a zoned block device? What are
> the ZAC/ZBC standards?
Zoned block devices (ZBDs) are HDDs that use SMR (shingled magnetic
recording). This type of recording, if applied to the entire drive, would
only allow the drive to be written sequentially. To make such devices more
practical, the entire LBA range of the drive is divided into zones. All
writes within a particular zone must be sequential, but writing different
zones can be done concurrently in random manner. This sounds like a lot of
hassle, but in return SMR can achieve up to 20% better areal data density
compared to the most common PMR recording.

The same zoned model is used in up-and-coming NVMe ZNS standard, even
though the reason for using it in ZNS is different compared to SMR HDDs -
easier flash erase block management.

ZBC is an INCITS T10 (SCSI) standard and ZAC is the corresponding T13 (ATA)
standard.

The lack of limelight for these standards is explained by the fact that
these devices are mostly used by cloud infrastructure providers for "cold"
data storage, a purely enterprise application. Currently, both WDC and
Seagate produce SMR drives in significant quantities and Toshiba has
announced support for ZBDs in their future products.

> > 
> I've found this:
> https://www.snia.org/sites/default/files/SDC/2016/presentations/smr/DamienLeMoal_ZBC-ZAC_Linux.pdf
> 
AFAIK, the most useful collection of public resources about zoned block
devices is this website -
http://zonedstorage.io
The site is maintained by our group at WDC (shameless plug here :) ).
BTW, here is the page containing the links to T10/T13 standards
(the access might be restricted for non-members of T10/T13 committees) -
http://zonedstorage.io/introduction/smr/#governing-standards

> It looks like ZAC/ZBC are new commands -- what happens if we just don't
> use them, exactly?
The standards define three models of zoned block devices: drive-managed,
host-aware and host-managed.

Drive-managed zoned devices behave just like regular SCSI/ATA devices and
don't require any additional support. There is no point for manufacturers
to market such devices as zoned. Host-managed and host-aware devices can
read data exactly the same way as common SCSI/ATA drives, but there are
I/O pattern limitations in the write path that the host must adhere to.

Host-aware drives will work without I/O errors under purely random write
workload, but their performance might be significantly degraded
compared to running them under zone-sequential workload. With
host-managed drives, any non-sequential writes within zones will lead
to an I/O error, most likely, "unaligned write".

It is important to mention that almost all zoned devices that are
currently on the market are host-managed.

ZAC/ZBC standards do add some new commands to the common SCSI/ACS command
sets, but, at least for the host-managed model, it wouldn't be sufficient
just to never issue these commands to be able to utilize these devices.

> 
> > To be more precise, attaching a zoned device via virtio-pci-blk,
> > virtio-scsi-pci/scsi-disk or virtio-scsi-pci/scsi-hd demonstrates the
> > above behavior. The virtio-scsi-pci/scsi-block method works with a
> > recent patch. The virtio-scsi-pci/scsi-generic method also appears to
> > handle zoned devices without problems.
> > 
> 
> What exactly fails, out of curiosity?
The current Linux kernel is able to recognize zoned block devices and
provide some means for the user to see that a particular device is zoned.
For example, lsscsi will show "zbc" instead of "disk" for zoned devices.
Another useful value is the "zoned" sysfs attribute that carries the
zoned model of the drive. Without this patch, the attachment methods
mentioned above present host-managed drives as regular drives at the
guest system. There is no way for the user to figure out that they are
dealing with a ZBD besides starting I/O and getting "unaligned write"
error.

The folks who designed ZAC/ZBC were very careful about this not to
happen and this doesn't happen on bare metal. Host-managed drives have
a distinctive SCSI device type, 0x14, and old kernels without zoned
device support simply are not be able to classify these drives during
the device scan. The kernels with ZBD support are able to recognize
a host-managed drive by its SCSI type and read some additional
protocol-specific info from the drive that is necessary for the kernel
to support it (how? see http://zonedstorage.io/linux/sched/).
In QEMU, this SCSI device type mechanism currently only works for
attachment methods that directly pass SCSI commands to the host OS
during the initial device scan, i.e. scsi-block and scsi-generic.
All other methods should be disabled until a meaningful way of handling
ZBDs is developed for each of them (or disabled permanently for "legacy"
attachment methods).

> 
> Naively, it seems strange to me that you'd have something that presents
> itself as a block device but can't be used like one. Usually I expect to
> see new features / types of devices used inefficiently when we aren't
> aware of a special attribute/property they have, but not create data
> corruption.
Data corruption can theoretically happen, for example, if a regular hard
drive is accidentally swapped for a zoned one in a complex environment
under I/O. Any environment where this can potentially be a problem must
have udev rules defined to prevent this situation. Without this type of
patch, these udev rules will not work.
> 
> The only reason I ask is because it seems odd that you need to add a
> special flag to e.g. legacy IDE devices that explicitly says they don't
> support zoned block devices -- instead of adding flags to virtio devices
> that say they explicitly do support that feature set.
The initial version of the patch set had some bits of code added in the
drivers that are not capable of supporting zoned devices to check if the
device is zoned and abort if it is. Kevin and Paolo suggested the current
approach and I think it's a lot cleaner than the initial attempt since it
minimizes the necessary changes across the whole set of block drivers. The
flag is a true/false setting that is set individually by each driver. It
is in line with two existing flags in blkconf_apply_backend_options(),
"readonly" and "resizable". There is no "default" setting for any of these.
> 
> --js
> 
> > This patch set adds code to check if the backing device that is being
> > opened is a zoned Host Managed device. If this is the case, the patch
> > prohibits attaching such device for all use cases lacking proper
> > zoned support.
> > 
> > Host Aware zoned block devices are designed to work as regular block
> > devices at a guest system that does not support ZBD. Therefore, this
> > patch set doesn't prohibit attachment of Host Aware devices.
> > 
> > Considering that there is still a couple of different working ways
> > to attach a ZBD, this patch set provides a reasonable short-term
> > solution for this problem. What about long term?
> > 
> > It appears to be beneficial to add proper ZBD support to virtio-blk.
> > In order to support this use case properly, some virtio-blk protocol
> > changes will be necessary. They are needed to allow the host code to
> > propagate some ZBD properties that are required for virtio guest
> > driver to configure the guest block device as ZBD, such as zoned
> > device model, zone size and the total number of zones. Further, some
> > support needs to be added for REPORT ZONES command as well as for zone
> > operations, such as OPEN ZONE, CLOSE ZONE, FINISH ZONE and RESET ZONE.
> > 
> > These additions to the protocol are relatively straightforward, but
> > they need to be approved by the virtio TC and the whole process may
> > take some time.
> > 
> > ZBD support for virtio-scsi-pci/scsi-disk and virtio-scsi-pci/scsi-hd
> > does not seem as necessary. Users will be expected to attach zoned
> > block devices via virtio-scsi-pci/scsi-block instead.
> > 
> > This patch set contains some Linux-specific code. This code is
> > necessary to obtain Zoned Block Device model value from Linux sysfs.
> > 
> > History:
> > 
> > v1 -> v2:
> > - rework the code to be permission-based
> > - always allow Host Aware devices to be attached
> > - add fix for Host Aware attachments aka RCAP output snoop
> > 
> > v2 -> v3:
> > - drop the patch for RCAP output snoop - merged separately
> > 
> > 
> > Dmitry Fomichev (4):
> >   block: Add zoned device model property
> >   raw: Recognize zoned backing devices
> >   block/ide/scsi: Set BLK_PERM_SUPPORT_ZONED
> >   raw: Don't open ZBDs if backend can't handle them
> > 
> >  block.c                   | 19 +++++++++
> >  block/file-posix.c        | 88 +++++++++++++++++++++++++++++++++------
> >  block/raw-format.c        |  8 ++++
> >  hw/block/block.c          |  8 +++-
> >  hw/block/fdc.c            |  4 +-
> >  hw/block/nvme.c           |  2 +-
> >  hw/block/virtio-blk.c     |  2 +-
> >  hw/block/xen-block.c      |  2 +-
> >  hw/ide/qdev.c             |  2 +-
> >  hw/scsi/scsi-disk.c       | 13 +++---
> >  hw/scsi/scsi-generic.c    |  2 +-
> >  hw/usb/dev-storage.c      |  2 +-
> >  include/block/block.h     | 21 +++++++++-
> >  include/block/block_int.h |  4 ++
> >  include/hw/block/block.h  |  3 +-
> >  15 files changed, 150 insertions(+), 30 deletions(-)
> > 

Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/4] virtio/block: handle zoned backing devices

[John Snow]

On 7/26/19 7:42 PM, Dmitry Fomichev wrote:
> John, please see inline...
> 
> Regards,
> Dmitry
> 
> On Thu, 2019-07-25 at 13:58 -0400, John Snow wrote:
>>
>> On 7/23/19 6:19 PM, Dmitry Fomichev wrote:
>>> Currently, attaching zoned block devices (i.e., storage devices
>>> compliant to ZAC/ZBC standards) using several virtio methods doesn't
>>> work properly as zoned devices appear as regular block devices at the
>>> guest. This may cause unexpected i/o errors and, potentially, some
>>> data corruption.
>>>
>>
>> Hi, I'm quite uninitiated here, what's a zoned block device? What are
>> the ZAC/ZBC standards?
> Zoned block devices (ZBDs) are HDDs that use SMR (shingled magnetic
> recording). This type of recording, if applied to the entire drive, would
> only allow the drive to be written sequentially. To make such devices more
> practical, the entire LBA range of the drive is divided into zones. All
> writes within a particular zone must be sequential, but writing different
> zones can be done concurrently in random manner. This sounds like a lot of
> hassle, but in return SMR can achieve up to 20% better areal data density
> compared to the most common PMR recording.
> 
> The same zoned model is used in up-and-coming NVMe ZNS standard, even
> though the reason for using it in ZNS is different compared to SMR HDDs -
> easier flash erase block management.
> 
> ZBC is an INCITS T10 (SCSI) standard and ZAC is the corresponding T13 (ATA)
> standard.
> 
> The lack of limelight for these standards is explained by the fact that
> these devices are mostly used by cloud infrastructure providers for "cold"
> data storage, a purely enterprise application. Currently, both WDC and
> Seagate produce SMR drives in significant quantities and Toshiba has
> announced support for ZBDs in their future products.
> 
>>>
>> I've found this:
>> https://www.snia.org/sites/default/files/SDC/2016/presentations/smr/DamienLeMoal_ZBC-ZAC_Linux.pdf
>>
> AFAIK, the most useful collection of public resources about zoned block
> devices is this website -
> http://zonedstorage.io
> The site is maintained by our group at WDC (shameless plug here :) ).
> BTW, here is the page containing the links to T10/T13 standards
> (the access might be restricted for non-members of T10/T13 committees) -
> http://zonedstorage.io/introduction/smr/#governing-standards
> 
>> It looks like ZAC/ZBC are new commands -- what happens if we just don't
>> use them, exactly?
> The standards define three models of zoned block devices: drive-managed,
> host-aware and host-managed.
> 
> Drive-managed zoned devices behave just like regular SCSI/ATA devices and
> don't require any additional support. There is no point for manufacturers
> to market such devices as zoned. Host-managed and host-aware devices can
> read data exactly the same way as common SCSI/ATA drives, but there are
> I/O pattern limitations in the write path that the host must adhere to.
> 
> Host-aware drives will work without I/O errors under purely random write
> workload, but their performance might be significantly degraded
> compared to running them under zone-sequential workload. With
> host-managed drives, any non-sequential writes within zones will lead
> to an I/O error, most likely, "unaligned write".
> 
> It is important to mention that almost all zoned devices that are
> currently on the market are host-managed.
> 

OK, understood.

> ZAC/ZBC standards do add some new commands to the common SCSI/ACS command
> sets, but, at least for the host-managed model, it wouldn't be sufficient
> just to never issue these commands to be able to utilize these devices.
> 
>>
>>> To be more precise, attaching a zoned device via virtio-pci-blk,
>>> virtio-scsi-pci/scsi-disk or virtio-scsi-pci/scsi-hd demonstrates the
>>> above behavior. The virtio-scsi-pci/scsi-block method works with a
>>> recent patch. The virtio-scsi-pci/scsi-generic method also appears to
>>> handle zoned devices without problems.
>>>
>>
>> What exactly fails, out of curiosity?
> The current Linux kernel is able to recognize zoned block devices and
> provide some means for the user to see that a particular device is zoned.
> For example, lsscsi will show "zbc" instead of "disk" for zoned devices.
> Another useful value is the "zoned" sysfs attribute that carries the
> zoned model of the drive. Without this patch, the attachment methods
> mentioned above present host-managed drives as regular drives at the
> guest system. There is no way for the user to figure out that they are
> dealing with a ZBD besides starting I/O and getting "unaligned write"
> error.
> 

Mmhmm...

> The folks who designed ZAC/ZBC were very careful about this not to
> happen and this doesn't happen on bare metal. Host-managed drives have
> a distinctive SCSI device type, 0x14, and old kernels without zoned
> device support simply are not be able to classify these drives during
> the device scan. The kernels with ZBD support are able to recognize
> a host-managed drive by its SCSI type and read some additional
> protocol-specific info from the drive that is necessary for the kernel
> to support it (how? see http://zonedstorage.io/linux/sched/).
> In QEMU, this SCSI device type mechanism currently only works for
> attachment methods that directly pass SCSI commands to the host OS
> during the initial device scan, i.e. scsi-block and scsi-generic.
> All other methods should be disabled until a meaningful way of handling
> ZBDs is developed for each of them (or disabled permanently for "legacy"
> attachment methods).
> 
>>
>> Naively, it seems strange to me that you'd have something that presents
>> itself as a block device but can't be used like one. Usually I expect to
>> see new features / types of devices used inefficiently when we aren't
>> aware of a special attribute/property they have, but not create data
>> corruption.
> Data corruption can theoretically happen, for example, if a regular hard
> drive is accidentally swapped for a zoned one in a complex environment
> under I/O. Any environment where this can potentially be a problem must
> have udev rules defined to prevent this situation. Without this type of
> patch, these udev rules will not work.
>>
>> The only reason I ask is because it seems odd that you need to add a
>> special flag to e.g. legacy IDE devices that explicitly says they don't
>> support zoned block devices -- instead of adding flags to virtio devices
>> that say they explicitly do support that feature set.
> The initial version of the patch set had some bits of code added in the
> drivers that are not capable of supporting zoned devices to check if the
> device is zoned and abort if it is. Kevin and Paolo suggested the current
> approach and I think it's a lot cleaner than the initial attempt since it
> minimizes the necessary changes across the whole set of block drivers. The
> flag is a true/false setting that is set individually by each driver. It
> is in line with two existing flags in blkconf_apply_backend_options(),
> "readonly" and "resizable". There is no "default" setting for any of these.

Thank you for the detailed explanation! This is good information to have
on the ML archive.

I'm still surprised that we need to prohibit IDE specifically from
interacting with drives of this type, as I would have hoped that the
kernel driver beneath our feet would have managed the access for us, but
I guess that's not true?

(If it isn't, I worry what happens if we have a format layer between us
and the baremetal: if we write qcow2 to the block device instead of raw,
even if we advertise to the emulated guest that we're using a zoned
device, we might remap things in/outside of zones and that coordination
is lost, wouldn't it?)

Not that I really desire people to use IDE emulators with fancy new
disks, it just seemed like an unusual patch.

If Kevin and Paolo are on board with the design, it's not my place to
try to begin managing this, it just caught my eye because it touched
something as old as IDE.

Thanks,
--js