On Mon, 18.05.15 22:43, Per Bergqvist (p...@bst.lu) wrote:
heya,
I talked to Kay now. He suggested that we should not add new "xyz_id"
tools anymore to udev, but actually rather remove more of them, and
have the kernel drivers export everything via sysfs, so that the
fields can be directly be accessed with ATTRS{} and $attrs{}, without
indirecting things via another tool, and more udev properties.
He suggested two options:
a) the preferred one: update the kernel driver to export the relevant
fields in sysfs, so that udev can read them from there. when that's
in place we can hook that up from udev without any new C code, and
all is great.
b) maintain this outside of udev: introduce a new package nvme-tools
or so, ship the tool there, and ask distros to add that to their
base set of tools.
Of course, a) is much preferrably I think, and the kernel patch
shouldn't be particularly complex...
Sorry for the delay, and for putting this through multiple rouns of
reviews before then denying that...
Sorry!
Lennart
> ---
> Makefile.am | 11 +++
> rules/60-persistent-storage.rules | 5 ++
> src/udev/nvme_id/nvme_id.c | 147
> ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 163 insertions(+)
> create mode 100644 src/udev/nvme_id/nvme_id.c
>
> diff --git a/Makefile.am b/Makefile.am
> index bf04d31..0de0d18 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3904,6 +3904,17 @@ dist_udevrules_DATA += \
> rules/60-persistent-v4l.rules
>
> #
> ------------------------------------------------------------------------------
> +nvme_id_SOURCES = \
> + src/udev/nvme_id/nvme_id.c
> +
> +nvme_id_LDADD = \
> + libudev-internal.la \
> + libsystemd-shared.la
> +
> +udevlibexec_PROGRAMS += \
> + nvme_id
> +
> +#
> ------------------------------------------------------------------------------
> accelerometer_SOURCES = \
> src/udev/accelerometer/accelerometer.c
>
> diff --git a/rules/60-persistent-storage.rules
> b/rules/60-persistent-storage.rules
> index 25b44a5..d3368a5 100644
> --- a/rules/60-persistent-storage.rules
> +++ b/rules/60-persistent-storage.rules
> @@ -42,6 +42,11 @@ KERNEL=="cciss*", ENV{DEVTYPE}=="disk",
> ENV{ID_SERIAL}!="?*", IMPORT{program}="s
> KERNEL=="sd*|sr*|cciss*", ENV{DEVTYPE}=="disk", ENV{ID_SERIAL}=="?*",
> SYMLINK+="disk/by-id/$env{ID_BUS}-$env{ID_SERIAL}"
> KERNEL=="sd*|cciss*", ENV{DEVTYPE}=="partition", ENV{ID_SERIAL}=="?*",
> SYMLINK+="disk/by-id/$env{ID_BUS}-$env{ID_SERIAL}-part%n"
>
> +# NVMe
> +KERNEL=="nvme*", ENV{ID_SERIAL}!="?*", IMPORT{program}="nvme_id --export
> $devnode"
> +KERNEL=="nvme*", ENV{DEVTYPE}=="disk", ENV{ID_SERIAL}=="?*",
> SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}"
> +KERNEL=="nvme*", ENV{DEVTYPE}=="partition", ENV{ID_SERIAL}=="?*",
> SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}-part%n"
> +
> # firewire
> KERNEL=="sd*[!0-9]|sr*", ATTRS{ieee1394_id}=="?*",
> SYMLINK+="disk/by-id/ieee1394-$attr{ieee1394_id}"
> KERNEL=="sd*[0-9]", ATTRS{ieee1394_id}=="?*",
> SYMLINK+="disk/by-id/ieee1394-$attr{ieee1394_id}-part%n"
> diff --git a/src/udev/nvme_id/nvme_id.c b/src/udev/nvme_id/nvme_id.c
> new file mode 100644
> index 0000000..c61b0b8
> --- /dev/null
> +++ b/src/udev/nvme_id/nvme_id.c
> @@ -0,0 +1,147 @@
> +/* -*- mode:c; tab-width:8; intent-tabs-mode:nil; -*-
> + *
> + * nvme_id - reads model/serial/firmware revision numbers from nvme drives
> + *
> + * Copyright (C) 2015 Per Bergqvist <p...@bst.lu>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <ctype.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <getopt.h>
> +#include <linux/nvme.h>
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +#include "libudev.h"
> +#include "libudev-private.h"
> +#include "udev-util.h"
> +#include "log.h"
> +
> +static int nvme_identify(struct udev *udev, int fd, void *buf, size_t
> buf_len) {
> + struct nvme_admin_cmd command = {
> + .opcode = nvme_admin_identify,
> + .addr = (unsigned long)buf,
> + .data_len = buf_len,
> + .cdw10 = 1 };
> +
> + if (ioctl(fd, NVME_IOCTL_ADMIN_CMD, &command) < 0)
> + return -errno;
> +
> + return 0;
> +}
> +
> +int main(int argc, char *argv[]) {
> + _cleanup_udev_unref_ struct udev *udev = NULL;
> +
> + struct nvme_id_ctrl nvme_id_ctrl = {};
> +
> + char model[sizeof(nvme_id_ctrl.mn)+1];
> + char model_enc[4*sizeof(nvme_id_ctrl.mn)+1];
> + char serial[sizeof(nvme_id_ctrl.sn)+1];
> + char revision[sizeof(nvme_id_ctrl.fr)+1];
> +
> + const char *node = NULL;
> + int export = 0;
> +
> + _cleanup_close_ int fd = -1;
> +
> + static const struct option options[] = {
> + { "export", no_argument, NULL, 'x' },
> + { "help", no_argument, NULL, 'h' },
> + {}
> + };
> +
> + log_parse_environment();
> + log_open();
> +
> + udev = udev_new();
> + if (udev == NULL)
> + return EXIT_SUCCESS;
> +
> + while (1) {
> + int option;
> +
> + option = getopt_long(argc, argv, "xh", options, NULL);
> + if (option == -1)
> + break;
> +
> + switch (option) {
> + case 'x':
> + export = 1;
> + break;
> + case 'h':
> + printf("Usage: nvme_id [--export] [--help]
> <device>\n"
> + " -x,--export print values as environment
> keys\n"
> + " -h,--help print this help text\n\n");
> + return EXIT_SUCCESS;
> + }
> + }
> +
> + node = argv[optind];
> + if (node == NULL) {
> + log_error("no node specified");
> + return EXIT_FAILURE;
> + }
> +
> + fd = open(node, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
> + if (fd < 0) {
> + log_error_errno(errno, "Unable to open '%s': %m", node);
> + return EXIT_FAILURE;
> + }
> +
> + if (nvme_identify(udev, fd, &nvme_id_ctrl, sizeof(struct
> nvme_id_ctrl)) == 0) {
> + memcpy(model, nvme_id_ctrl.mn, sizeof(nvme_id_ctrl.mn));
> + model[sizeof(nvme_id_ctrl.mn)] = '\0';
> + strstrip(model);
> + udev_util_encode_string(model, model_enc, sizeof(model_enc));
> + util_replace_whitespace((char *) nvme_id_ctrl.mn, model,
> sizeof(nvme_id_ctrl.mn));
> + util_replace_chars(model, NULL);
> + util_replace_whitespace((char *) nvme_id_ctrl.sn, serial,
> sizeof(nvme_id_ctrl.sn));
> + util_replace_chars(serial, NULL);
> + util_replace_whitespace((char *) nvme_id_ctrl.fr, revision,
> sizeof(nvme_id_ctrl.fr));
> + util_replace_chars(revision, NULL);
> + } else {
> + log_debug_errno(errno, "nvme_identify failed for '%s': %m",
> node);
> + return EXIT_FAILURE;
> + }
> +
> +
> + if (export) {
> + printf("ID_TYPE=nvme\n");
> + printf("ID_MODEL=%s\n", model);
> + printf("ID_MODEL_ENC=%s\n", model_enc);
> + printf("ID_REVISION=%s\n", revision);
> + if (serial[0] != '\0') {
> + printf("ID_SERIAL=%s_%s\n", model, serial);
> + printf("ID_SERIAL_SHORT=%s\n", serial);
> + } else
> + printf("ID_SERIAL=%s\n", model);
> + } else {
> + if (serial[0] != '\0')
> + printf("%s_%s\n", model, serial);
> + else
> + printf("%s\n", model);
> + }
> +
> + return EXIT_SUCCESS;
> +}
> --
> 2.4.0
>
>
>
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
