From 29162ba05cab721d7a93312077264849b0faee20 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 7 Apr 2023 03:37:03 +0900 Subject: [PATCH 01/10] udev-rules: add missing paren --- src/udev/udev-rules.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 19bbf295c5..e30c11f2a2 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -35,7 +35,7 @@ #include "user-util.h" #include "virt.h" -#define RULES_DIRS (const char* const*) CONF_PATHS_STRV("udev/rules.d") +#define RULES_DIRS ((const char* const*) CONF_PATHS_STRV("udev/rules.d")) typedef enum { OP_MATCH, /* == */ From 03ff9c70ceefde175bac3de52416bce38b0031a0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 7 Apr 2023 03:33:15 +0900 Subject: [PATCH 02/10] udev-rules: add/update comments --- src/udev/udev-rules.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index e30c11f2a2..30f3ab6c0c 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2554,17 +2554,18 @@ static int udev_rule_apply_token_to_event( if (IN_SET(token->op, OP_ASSIGN, OP_ASSIGN_FINAL)) device_cleanup_devlinks(dev); - /* allow multiple symlinks separated by spaces */ - (void) udev_event_apply_format(event, token->value, buf, sizeof(buf), event->esc != ESCAPE_NONE, &truncated); + (void) udev_event_apply_format(event, token->value, buf, sizeof(buf), + /* replace_whitespace = */ event->esc != ESCAPE_NONE, &truncated); if (truncated) { log_event_truncated(dev, token, "symbolic link path", token->value, "SYMLINK", /* is_match = */ false); break; } + /* By default or string_escape=none, allow multiple symlinks separated by spaces. */ if (event->esc == ESCAPE_UNSET) - count = udev_replace_chars(buf, "/ "); + count = udev_replace_chars(buf, /* allow = */ "/ "); else if (event->esc == ESCAPE_REPLACE) - count = udev_replace_chars(buf, "/"); + count = udev_replace_chars(buf, /* allow = */ "/"); else count = 0; if (count > 0) From f17af9c927636fdd4275eabf129d6bfa3367e1e9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 7 Apr 2023 08:36:15 +0900 Subject: [PATCH 03/10] udev-rules: rename variable "filename" -> "path" --- src/udev/udev-rules.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 30f3ab6c0c..b410216396 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2575,7 +2575,7 @@ static int udev_rule_apply_token_to_event( p = skip_leading_chars(buf, NULL); while (!isempty(p)) { - char filename[UDEV_PATH_SIZE], *next; + char path[UDEV_PATH_SIZE], *next; next = strchr(p, ' '); if (next) { @@ -2583,19 +2583,19 @@ static int udev_rule_apply_token_to_event( next = skip_leading_chars(next, NULL); } - strscpyl_full(filename, sizeof(filename), &truncated, "/dev/", p, NULL); + strscpyl_full(path, sizeof(path), &truncated, "/dev/", p, NULL); if (truncated) continue; if (token->op == OP_REMOVE) { - device_remove_devlink(dev, filename); - log_event_debug(dev, token, "Dropped SYMLINK '%s'", p); + device_remove_devlink(dev, path); + log_event_debug(dev, token, "Dropped SYMLINK '%s'", path); } else { - r = device_add_devlink(dev, filename); + r = device_add_devlink(dev, path); if (r < 0) - return log_event_error_errno(dev, token, r, "Failed to add devlink '%s': %m", filename); + return log_event_error_errno(dev, token, r, "Failed to add devlink '%s': %m", path); - log_event_debug(dev, token, "Added SYMLINK '%s'", p); + log_event_debug(dev, token, "Added SYMLINK '%s'", path); } p = next; From 733b7bfd79a9bf7c46e8930ca5f235507aeed6fc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 7 Apr 2023 03:31:01 +0900 Subject: [PATCH 04/10] udev-rules: replace ingrowing word extractor with extract_first_word() No functional change, just refactoring. --- src/udev/udev-rules.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index b410216396..58c5c81574 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2541,7 +2541,7 @@ static int udev_rule_apply_token_to_event( break; } case TK_A_DEVLINK: { - char buf[UDEV_PATH_SIZE], *p; + char buf[UDEV_PATH_SIZE]; bool truncated; size_t count; @@ -2573,19 +2573,22 @@ static int udev_rule_apply_token_to_event( "Replaced %zu character(s) from result of SYMLINK=\"%s\"", count, token->value); - p = skip_leading_chars(buf, NULL); - while (!isempty(p)) { - char path[UDEV_PATH_SIZE], *next; + for (const char *p = buf;;) { + _cleanup_free_ char *word = NULL, *path = NULL; - next = strchr(p, ' '); - if (next) { - *next++ = '\0'; - next = skip_leading_chars(next, NULL); + r = extract_first_word(&p, &word, NULL, EXTRACT_RETAIN_ESCAPE); + if (r == -ENOMEM) + return log_oom(); + if (r < 0) { + log_warning_errno(r, "Failed to extract first path in SYMLINK=, ignoring: %m"); + break; } + if (r == 0) + break; - strscpyl_full(path, sizeof(path), &truncated, "/dev/", p, NULL); - if (truncated) - continue; + path = path_join("/dev/", word); + if (!path) + return log_oom(); if (token->op == OP_REMOVE) { device_remove_devlink(dev, path); @@ -2597,8 +2600,6 @@ static int udev_rule_apply_token_to_event( log_event_debug(dev, token, "Added SYMLINK '%s'", path); } - - p = next; } break; } From 2c5f119c3cc78bd7da0c7c56b57eca43bac464c1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 7 Apr 2023 08:44:00 +0900 Subject: [PATCH 05/10] sd-device,udev: refuse invalid devlink and store in normalized form This is especially for the case that the path contains "..". Prompted by https://github.com/systemd/systemd/pull/27164#issuecomment-1498863858. This also makes SYMLINK= gracefully handle paths prefixed with "/dev/", and manage devlink paths with path_hash_ops. --- src/libsystemd/sd-device/device-private.h | 2 +- src/libsystemd/sd-device/sd-device.c | 36 +++++++++++++++++++---- src/udev/udev-rules.c | 25 +++++++++------- test/udev-test.pl | 30 +++++++++++++++++-- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 740c58438c..b903d1afd6 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -38,7 +38,7 @@ void device_set_db_persist(sd_device *device); void device_set_devlink_priority(sd_device *device, int priority); int device_ensure_usec_initialized(sd_device *device, sd_device *device_old); int device_add_devlink(sd_device *device, const char *devlink); -void device_remove_devlink(sd_device *device, const char *devlink); +int device_remove_devlink(sd_device *device, const char *devlink); bool device_has_devlink(sd_device *device, const char *devlink); int device_add_property(sd_device *device, const char *property, const char *value); int device_add_propertyf(sd_device *device, const char *key, const char *format, ...) _printf_(3, 4); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index e86ec3e75b..2a5defca65 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1473,34 +1473,58 @@ int device_add_tag(sd_device *device, const char *tag, bool both) { return 0; } +static char *prefix_dev(const char *p) { + assert(p); + + if (path_startswith(p, "/dev/")) + return strdup(p); + + return path_join("/dev/", p); +} + int device_add_devlink(sd_device *device, const char *devlink) { + char *p; int r; assert(device); assert(devlink); - r = set_put_strdup(&device->devlinks, devlink); + if (!path_is_safe(devlink)) + return -EINVAL; + + p = prefix_dev(devlink); + if (!p) + return -ENOMEM; + + path_simplify(p); + + r = set_ensure_consume(&device->devlinks, &path_hash_ops_free, p); if (r < 0) return r; device->devlinks_generation++; device->property_devlinks_outdated = true; - return 0; + return r; /* return 1 when newly added, 0 when already exists */ } -void device_remove_devlink(sd_device *device, const char *devlink) { - _cleanup_free_ char *s = NULL; +int device_remove_devlink(sd_device *device, const char *devlink) { + _cleanup_free_ char *p = NULL, *s = NULL; assert(device); assert(devlink); - s = set_remove(device->devlinks, devlink); + p = prefix_dev(devlink); + if (!p) + return -ENOMEM; + + s = set_remove(device->devlinks, p); if (!s) - return; + return 0; /* does not exist */ device->devlinks_generation++; device->property_devlinks_outdated = true; + return 1; /* removed */ } bool device_has_devlink(sd_device *device, const char *devlink) { diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 58c5c81574..68f0a363be 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2574,9 +2574,9 @@ static int udev_rule_apply_token_to_event( count, token->value); for (const char *p = buf;;) { - _cleanup_free_ char *word = NULL, *path = NULL; + _cleanup_free_ char *path = NULL; - r = extract_first_word(&p, &word, NULL, EXTRACT_RETAIN_ESCAPE); + r = extract_first_word(&p, &path, NULL, EXTRACT_RETAIN_ESCAPE); if (r == -ENOMEM) return log_oom(); if (r < 0) { @@ -2586,19 +2586,22 @@ static int udev_rule_apply_token_to_event( if (r == 0) break; - path = path_join("/dev/", word); - if (!path) - return log_oom(); - if (token->op == OP_REMOVE) { - device_remove_devlink(dev, path); - log_event_debug(dev, token, "Dropped SYMLINK '%s'", path); + r = device_remove_devlink(dev, path); + if (r == -ENOMEM) + return log_oom(); + if (r < 0) + log_event_warning_errno(dev, token, r, "Failed to remove devlink '%s', ignoring: %m", path); + else if (r > 0) + log_event_debug(dev, token, "Dropped SYMLINK '%s'", path); } else { r = device_add_devlink(dev, path); + if (r == -ENOMEM) + return log_oom(); if (r < 0) - return log_event_error_errno(dev, token, r, "Failed to add devlink '%s': %m", path); - - log_event_debug(dev, token, "Added SYMLINK '%s'", path); + log_event_warning_errno(dev, token, r, "Failed to add devlink '%s', ignoring: %m", path); + else if (r > 0) + log_event_debug(dev, token, "Added SYMLINK '%s'", path); } } break; diff --git a/test/udev-test.pl b/test/udev-test.pl index a493485fc1..9b3641b3d9 100755 --- a/test/udev-test.pl +++ b/test/udev-test.pl @@ -212,13 +212,37 @@ EOF { devpath => "/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/sda1", exp_links => ["boot_disk1", "boot_diskXY1"], - not_exp_links => ["boot_diskXX1", "hoge"], + not_exp_links => ["boot_diskXX1"], }], rules => < "SYMLINK tests", + devices => [ + { + devpath => "/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/sda1", + exp_links => ["link1", "link2/foo", "link3/aaa/bbb", + "default___replace_test/foo_aaa", + "string_escape___replace/foo_bbb", + "env_with_space", + "default/replace/mode_foo__hoge", + "replace_env_harder_foo__hoge"], + not_exp_links => ["removed", "unsafe/../../path"], + }], + rules => < Date: Fri, 7 Apr 2023 08:45:35 +0900 Subject: [PATCH 06/10] sd-device,udev: tag must be a valid filename All tags are managed under /run/udev/tags, and the directories there are named with tags. Hence, each tag must be a valid filename. This also makes all validity check moved to sd-device side, and makes failure caused by setting invalid tags non-critical. With this change, an empty string cannot be assigned to TAG=, hence the test cases are adjusted. --- src/libsystemd/sd-device/device-private.c | 2 ++ src/libsystemd/sd-device/sd-device.c | 2 +- src/udev/udev-rules.c | 8 ++--- test/udev-test.pl | 37 +++++++++++++++++------ 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index e9d54f514e..9ded8c17a1 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -351,6 +351,8 @@ static int device_amend(sd_device *device, const char *key, const char *value) { return r; if (r == 0) break; + if (isempty(word)) + continue; r = device_add_tag(device, word, streq(key, "CURRENT_TAGS")); if (r < 0) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 2a5defca65..5d486957ce 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1439,7 +1439,7 @@ _public_ int sd_device_get_diskseq(sd_device *device, uint64_t *ret) { static bool is_valid_tag(const char *tag) { assert(tag); - return !strchr(tag, ':') && !strchr(tag, ' '); + return in_charset(tag, ALPHANUMERICAL "-_") && filename_is_valid(tag); } int device_add_tag(sd_device *device, const char *tag, bool both) { diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 68f0a363be..e3d2adbafd 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2487,16 +2487,14 @@ static int udev_rule_apply_token_to_event( if (token->op == OP_ASSIGN) device_cleanup_tags(dev); - if (buf[strspn(buf, ALPHANUMERICAL "-_")] != '\0') { - log_event_error(dev, token, "Invalid tag name '%s', ignoring", buf); - break; - } if (token->op == OP_REMOVE) device_remove_tag(dev, buf); else { r = device_add_tag(dev, buf, true); + if (r == -ENOMEM) + return log_oom(); if (r < 0) - return log_event_error_errno(dev, token, r, "Failed to add tag '%s': %m", buf); + log_event_warning_errno(dev, token, r, "Failed to add tag '%s', ignoring: %m", buf); } break; } diff --git a/test/udev-test.pl b/test/udev-test.pl index 9b3641b3d9..16c82bec0c 100755 --- a/test/udev-test.pl +++ b/test/udev-test.pl @@ -1808,9 +1808,9 @@ EOF not_exp_name => "bad", }], rules => < "bad", }], rules => < "bad", }], rules => < "TAG refuses invalid string", + devices => [ + { + devpath => "/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda", + exp_links => ["valid", "found"], + not_exp_links => ["empty", "invalid_char", "path", "bad", "bad2"], + }], + rules => < Date: Fri, 7 Apr 2023 08:50:54 +0900 Subject: [PATCH 07/10] sd-device: manage cached sysattr values with path_hash_ops As here keys are relative paths to sysattrs. --- src/libsystemd/sd-device/sd-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 5d486957ce..58067414cd 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2294,7 +2294,7 @@ int device_cache_sysattr_value(sd_device *device, const char *key, char *value) return -ENOMEM; } - r = hashmap_ensure_put(&device->sysattr_values, &string_hash_ops_free_free, new_key, value); + r = hashmap_ensure_put(&device->sysattr_values, &path_hash_ops_free_free, new_key, value); if (r < 0) return r; From d05e1be86e6b14bd22d57af17efcc3b8fb7ecd82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Apr 2023 12:28:14 +0200 Subject: [PATCH 08/10] udev: restore compat symlink for nvme devices In 5118e8e71dda211d20e34ec8d3012186ba27d3d3, the rules were changed to add OPTIONS="string_escape=replace" to creation of ENV{ID_SERIAL}="$env{ID_MODEL}_$env{ID_SERIAL_SHORT}", so that "/" would be escaped. But this also changes how the symlink looks for devices that do not have "/". This adds back the old symlink for compat, except when a slash is present. In the meantime, we changed the symlink format to include ${ND_NSID}. Since the symlink with unescaped characters are older than that, for compat we only need to cover the older type. (Symlinks without escaping and with ${ND_NSID} were never created.) This makes it slightly easier on users: the non-deprecated symlinks are with "_${ND_NSID}", so they are easier to distinguish. Fixes #27155. Mostly untested :( I only have a boring nvme device with no special characters in the id, and the symlinks are unchanged for it by this patch. --- rules.d/60-persistent-storage.rules.in | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rules.d/60-persistent-storage.rules.in b/rules.d/60-persistent-storage.rules.in index 51589686af..a944d7c157 100644 --- a/rules.d/60-persistent-storage.rules.in +++ b/rules.d/60-persistent-storage.rules.in @@ -38,6 +38,10 @@ KERNEL=="nvme*[0-9]n*[0-9]", ENV{DEVTYPE}=="disk", ATTRS{wwid}=="?*", ENV{ID_WWN KERNEL=="nvme*[0-9]n*[0-9]", ENV{DEVTYPE}=="disk", ATTRS{model}=="?*", ENV{ID_MODEL}="$attr{model}" KERNEL=="nvme*[0-9]n*[0-9]", ENV{DEVTYPE}=="disk", ATTRS{firmware_rev}=="?*", ENV{ID_REVISION}="$attr{firmware_rev}" KERNEL=="nvme*[0-9]n*[0-9]", ENV{DEVTYPE}=="disk", ATTRS{nsid}=="?*", ENV{ID_NSID}="$attr{nsid}" +# obsolete symlink with non-escaped characters, kept for backward compatiblity +KERNEL=="nvme*[0-9]n*[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_MODEL}=="?*", ENV{ID_SERIAL_SHORT}=="?*", \ + ENV{ID_MODEL}!="*/*", ENV{ID_SERIAL_SHORT}!="*/*", \ + ENV{ID_SERIAL}="$env{ID_MODEL}_$env{ID_SERIAL_SHORT}", SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}" # obsolete symlink that might get overridden on adding a new nvme controller, kept for backward compatibility KERNEL=="nvme*[0-9]n*[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_MODEL}=="?*", ENV{ID_SERIAL_SHORT}=="?*", \ OPTIONS="string_escape=replace", ENV{ID_SERIAL}="$env{ID_MODEL}_$env{ID_SERIAL_SHORT}", SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}" @@ -48,6 +52,10 @@ KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ATTRS{serial}=="? KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ATTRS{model}=="?*", ENV{ID_MODEL}="$attr{model}" KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ATTRS{firmware_rev}=="?*", ENV{ID_REVISION}="$attr{firmware_rev}" KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ATTRS{nsid}=="?*", ENV{ID_NSID}="$attr{nsid}" +# obsolete symlink with non-escaped characters, kept for backward compatiblity +KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ENV{ID_MODEL}=="?*", ENV{ID_SERIAL_SHORT}=="?*", \ + ENV{ID_MODEL}!="*/*", ENV{ID_SERIAL_SHORT}!="*/*", \ + ENV{ID_SERIAL}="$env{ID_MODEL}_$env{ID_SERIAL_SHORT}", SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}-part%n" # obsolete symlink that might get overridden on adding a new nvme controller, kept for backward compatibility KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ENV{ID_MODEL}=="?*", ENV{ID_SERIAL_SHORT}=="?*", \ OPTIONS="string_escape=replace", ENV{ID_SERIAL}="$env{ID_MODEL}_$env{ID_SERIAL_SHORT}", SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}-part%n" From 49e3e219b01132ef269297574a9bc7b7b34d9398 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 8 Apr 2023 03:36:44 +0900 Subject: [PATCH 09/10] rules: drop doubled space --- rules.d/60-persistent-storage.rules.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules.d/60-persistent-storage.rules.in b/rules.d/60-persistent-storage.rules.in index a944d7c157..c093e21500 100644 --- a/rules.d/60-persistent-storage.rules.in +++ b/rules.d/60-persistent-storage.rules.in @@ -59,7 +59,7 @@ KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ENV{ID_MODEL}=="? # obsolete symlink that might get overridden on adding a new nvme controller, kept for backward compatibility KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ENV{ID_MODEL}=="?*", ENV{ID_SERIAL_SHORT}=="?*", \ OPTIONS="string_escape=replace", ENV{ID_SERIAL}="$env{ID_MODEL}_$env{ID_SERIAL_SHORT}", SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}-part%n" -KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ENV{ID_MODEL}=="?*", ENV{ID_SERIAL_SHORT}=="?*", ENV{ID_NSID}=="?*", \ +KERNEL=="nvme*[0-9]n*[0-9]p*[0-9]", ENV{DEVTYPE}=="partition", ENV{ID_MODEL}=="?*", ENV{ID_SERIAL_SHORT}=="?*", ENV{ID_NSID}=="?*", \ OPTIONS="string_escape=replace", ENV{ID_SERIAL}="$env{ID_MODEL}_$env{ID_SERIAL_SHORT}_$env{ID_NSID}", SYMLINK+="disk/by-id/nvme-$env{ID_SERIAL}-part%n" # virtio-blk From 18a6cd4ba331eb3d75b04e5a99483fff0f9bd812 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 8 Apr 2023 03:33:32 +0900 Subject: [PATCH 10/10] test-64: add tests for compat devlinks for NVMe drive --- test/TEST-64-UDEV-STORAGE/test.sh | 29 ++++++++++++++++--- test/test-functions | 1 + test/units/testsuite-64.sh | 48 ++++++++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/test/TEST-64-UDEV-STORAGE/test.sh b/test/TEST-64-UDEV-STORAGE/test.sh index 2d77bb678f..d41a4f00f9 100755 --- a/test/TEST-64-UDEV-STORAGE/test.sh +++ b/test/TEST-64-UDEV-STORAGE/test.sh @@ -88,6 +88,8 @@ _image_cleanup() { # Clean up certain "problematic" files which may be left over by failing tests : >"${initdir:?}/etc/fstab" : >"${initdir:?}/etc/crypttab" + # Clear previous assignment + QEMU_OPTIONS_ARRAY=() } test_run_one() { @@ -193,15 +195,34 @@ testcase_nvme_basic() { local i local qemu_opts=() - for i in {0..27}; do + for (( i = 0; i < 5; i++ )); do qemu_opts+=( - "-device nvme,drive=nvme$i,serial=deadbeef$i,num_queues=8" - "-drive format=raw,cache=unsafe,file=${TESTDIR:?}/disk$i.img,if=none,id=nvme$i" + "-device" "nvme,drive=nvme$i,serial=deadbeef$i,num_queues=8" + "-drive" "format=raw,cache=unsafe,file=${TESTDIR:?}/disk$i.img,if=none,id=nvme$i" + ) + done + for (( i = 5; i < 10; i++ )); do + qemu_opts+=( + "-device" "nvme,drive=nvme$i,serial= deadbeef $i ,num_queues=8" + "-drive" "format=raw,cache=unsafe,file=${TESTDIR:?}/disk$i.img,if=none,id=nvme$i" + ) + done + for (( i = 10; i < 15; i++ )); do + qemu_opts+=( + "-device" "nvme,drive=nvme$i,serial= dead/beef/$i ,num_queues=8" + "-drive" "format=raw,cache=unsafe,file=${TESTDIR:?}/disk$i.img,if=none,id=nvme$i" + ) + done + for (( i = 15; i < 20; i++ )); do + qemu_opts+=( + "-device" "nvme,drive=nvme$i,serial=dead/../../beef/$i,num_queues=8" + "-drive" "format=raw,cache=unsafe,file=${TESTDIR:?}/disk$i.img,if=none,id=nvme$i" ) done KERNEL_APPEND="systemd.setenv=TEST_FUNCTION_NAME=${FUNCNAME[0]} ${USER_KERNEL_APPEND:-}" - QEMU_OPTIONS="${qemu_opts[*]} ${USER_QEMU_OPTIONS:-}" + QEMU_OPTIONS="${USER_QEMU_OPTIONS}" + QEMU_OPTIONS_ARRAY=("${qemu_opts[@]}") test_run_one "${1:?}" } diff --git a/test/test-functions b/test/test-functions index 35a10868b6..0ea1635b0d 100644 --- a/test/test-functions +++ b/test/test-functions @@ -512,6 +512,7 @@ run_qemu() { read -ra user_qemu_options <<< "$QEMU_OPTIONS" qemu_options+=("${user_qemu_options[@]}") fi + qemu_options+=(${QEMU_OPTIONS_ARRAY:+"${QEMU_OPTIONS_ARRAY[@]}"}) if [[ -n "${KERNEL_APPEND:=}" ]]; then local user_kernel_append diff --git a/test/units/testsuite-64.sh b/test/units/testsuite-64.sh index 03d2fcb4ef..015b6b69b5 100755 --- a/test/units/testsuite-64.sh +++ b/test/units/testsuite-64.sh @@ -174,8 +174,54 @@ testcase_megasas2_basic() { } testcase_nvme_basic() { + local expected_symlinks=() + local i + + for (( i = 0; i < 5; i++ )); do + expected_symlinks+=( + # both replace mode provides the same devlink + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_deadbeef"$i" + # with nsid + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_deadbeef"$i"_1 + ) + done + for (( i = 5; i < 10; i++ )); do + expected_symlinks+=( + # old replace mode + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl__deadbeef_"$i" + # newer replace mode + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_____deadbeef__"$i" + # with nsid + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_____deadbeef__"$i"_1 + ) + done + for (( i = 10; i < 15; i++ )); do + expected_symlinks+=( + # old replace mode does not provide devlink, as serial contains "/" + # newer replace mode + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_____dead_beef_"$i" + # with nsid + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_____dead_beef_"$i"_1 + ) + done + for (( i = 15; i < 20; i++ )); do + expected_symlinks+=( + # old replace mode does not provide devlink, as serial contains "/" + # newer replace mode + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_dead_.._.._beef_"$i" + # with nsid + /dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_dead_.._.._beef_"$i"_1 + ) + done + + udevadm settle + ls /dev/disk/by-id + for i in "${expected_symlinks[@]}"; do + udevadm wait --settle --timeout=30 "$i" + done + lsblk --noheadings | grep "^nvme" - [[ "$(lsblk --noheadings | grep -c "^nvme")" -ge 28 ]] + [[ "$(lsblk --noheadings | grep -c "^nvme")" -ge 20 ]] } testcase_nvme_subsystem() {