qemu-devel
[Top][All Lists]
Advanced

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

Re: [RFC v3 4/5] file-posix: introduce get_sysfs_str_val for device zone


From: Hannes Reinecke
Subject: Re: [RFC v3 4/5] file-posix: introduce get_sysfs_str_val for device zoned model.
Date: Mon, 27 Jun 2022 09:41:03 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0

On 6/27/22 02:19, Sam Li wrote:
---
  block/file-posix.c           | 60 ++++++++++++++++++++++++++++++++++++
  include/block/block-common.h |  4 +--
  2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 73c2cdfbca..74c0245e0f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1277,6 +1277,66 @@ out:
  #endif
  }
+/*
+ * Convert the zoned attribute file in sysfs to internal value.
+ */
+static zone_model get_sysfs_str_val(int fd, struct stat *st) {
+#ifdef CONFIG_LINUX
+    char buf[32];
+    char *sysfspath = NULL;
+    int ret;
+    int sysfd = -1;
+
+    if (S_ISCHR(st->st_mode)) {
+        if (ioctl(fd, SG_GET_SG_TABLESIZE, &ret) == 0) {
+            return ret;
+        }
+        return -ENOTSUP;
+    }
+
+    if (!S_ISBLK(st->st_mode)) {
+        return -ENOTSUP;
+    }
+
+    sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/zoned",
+                                major(st->st_rdev), minor(st->st_rdev));
+    sysfd = open(sysfspath, O_RDONLY);
+    if (sysfd == -1) {
+        ret = -errno;
+        goto out;
+    }
+    do {
+        ret = read(sysfd, buf, sizeof(buf) - 1);
+    } while (ret == -1 && errno == EINTR);

This is wrong.
read() might return a value smaller than the 'len' argument (sizeof(buf) -1 in your case). But in that case it's a short read, and one need to call 'read()' again to fetch the remaining bytes.

So the correct code would be something like:

offset = 0;
do {
    ret = read(sysfd, buf + offset, sizeof(buf) - 1 + offset);
    if (ret > 0)
        offset += ret;
} while (ret > 0);

Not that you'd actually need it; reads from sysfs are basically never interrupted, so you should be able to read from an attribute in one go.
IE alternatively you can drop the 'while' loop and just call read().

+    if (ret < 0) {
+        ret = -errno;
+        goto out;
+    } else if (ret == 0) {
+        ret = -EIO;
+        goto out;
+    }
+    buf[ret] = 0;
+
+    /* The file is ended with '\n' */

I'd rather check if the string ends with an '\n', and overwrite
it with a '\0'. That way you'd be insulated against any changes
to sysfs.

+    if (strcmp(buf, "host-managed\n") == 0) {
+        return BLK_Z_HM;
+    } else if (strcmp(buf, "host-aware\n") == 0) {
+        return BLK_Z_HA;
+    } else {
+        return -ENOTSUP;
+    }
+
+out:
+    if (sysfd != -1) {
+        close(sysfd);
+    }
+    g_free(sysfspath);
+    return ret;
+#else
+    return -ENOTSUP;
+#endif
+}
+
  static int hdev_get_max_segments(int fd, struct stat *st) {
      int ret;
      ret = get_sysfs_long_val(fd, st, "max_segments");

And as you already set a precedent in your previous patch, I'd recommend split this in two patches, one introducing a generic function 'get_sysfs_str_val()' which returns a string and another function (eg hdev_get_zone_model()) which calls this function to fetch the device zoned model.

diff --git a/include/block/block-common.h b/include/block/block-common.h
index 78cddeeda5..35e00afe8e 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -56,8 +56,8 @@ typedef enum zone_op {
  } zone_op;
typedef enum zone_model {
-    BLK_Z_HM,
-    BLK_Z_HA,
+    BLK_Z_HM = 0x1,
+    BLK_Z_HA = 0x2,
  } zone_model;
typedef enum BlkZoneCondition {

Cheers,

Hannes
--
Dr. Hannes Reinecke                        Kernel Storage Architect
hare@suse.de                                      +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer



reply via email to

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