-
Notifications
You must be signed in to change notification settings - Fork 519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add udev rule to create symlinks using EBS volumes' device names #3977
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
From a0803d1d6fb6d899cd22bd7b37ee27859bfdf155 Mon Sep 17 00:00:00 2001 | ||
From: Tomas Bzatek <[email protected]> | ||
Date: Fri, 3 May 2024 17:19:39 +0200 | ||
Subject: [PATCH] linux: Fix uninitialized variables | ||
MIME-Version: 1.0 | ||
Content-Type: text/plain; charset=UTF-8 | ||
Content-Transfer-Encoding: 8bit | ||
|
||
In file included from ../src/nvme/linux.c:40: | ||
In function ‘freep’, | ||
inlined from ‘nvme_get_telemetry_log’ at ../src/nvme/linux.c:169:23: | ||
../src/nvme/cleanup.h:24:9: warning: ‘log’ may be used uninitialized [-Wmaybe-uninitialized] | ||
24 | free(*(void **)p); | ||
| ^~~~~~~~~~~~~~~~~ | ||
../src/nvme/linux.c: In function ‘nvme_get_telemetry_log’: | ||
../src/nvme/linux.c:169:30: note: ‘log’ was declared here | ||
169 | _cleanup_free_ void *log; | ||
| ^~~ | ||
|
||
Signed-off-by: Tomas Bzatek <[email protected]> | ||
--- | ||
src/nvme/linux.c | 2 +- | ||
1 file changed, 1 insertion(+), 1 deletion(-) | ||
|
||
diff --git a/src/nvme/linux.c b/src/nvme/linux.c | ||
index 25196fd5..35976011 100644 | ||
--- a/src/nvme/linux.c | ||
+++ b/src/nvme/linux.c | ||
@@ -166,7 +166,7 @@ int nvme_get_telemetry_log(int fd, bool create, bool ctrl, bool rae, size_t max_ | ||
|
||
struct nvme_telemetry_log *telem; | ||
enum nvme_cmd_get_log_lid lid; | ||
- _cleanup_free_ void *log; | ||
+ _cleanup_free_ void *log = NULL; | ||
void *tmp; | ||
int err; | ||
size_t dalb; | ||
-- | ||
2.44.0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
[package] | ||
name = "libnvme" | ||
version = "0.1.0" | ||
edition = "2021" | ||
publish = false | ||
build = "../build.rs" | ||
|
||
[lib] | ||
path = "../packages.rs" | ||
|
||
[package.metadata.build-package] | ||
releases-url = "https://github.com/linux-nvme/libnvme/releases" | ||
|
||
[[package.metadata.build-package.external-files]] | ||
url = "https://github.com/linux-nvme/libnvme/archive/v1.9/libnvme-1.9.tar.gz" | ||
sha512 = "39a3346805143f93a17d00cfcb6fb75f82154658db6079134c09dfa989995ac5de79b1ce1ac091b4e997523d3216829ce9eac44110c9f59f9fd21636529c8b25" | ||
|
||
[build-dependencies] | ||
glibc = { path = "../glibc" } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
Name: %{_cross_os}libnvme | ||
Version: 1.9 | ||
Release: 1%{?dist} | ||
Summary: Library for NVM Express | ||
License: LGPL-2.1-only AND CC0-1.0 AND MIT | ||
URL: https://github.com/linux-nvme/libnvme | ||
Source0: https://github.com/linux-nvme/libnvme/archive/v%{version}/libnvme-%{version}.tar.gz | ||
Patch0001: 0001-linux-Fix-uninitialized-variables.patch | ||
|
||
BuildRequires: meson | ||
BuildRequires: %{_cross_os}glibc-devel | ||
|
||
%package devel | ||
Summary: Files for development using the library for NVM Express | ||
Requires: %{_cross_os}libnvme | ||
|
||
%description devel | ||
%{summary}. | ||
|
||
%description | ||
%{summary}. | ||
|
||
%prep | ||
%autosetup -n libnvme-%{version} -p1 | ||
|
||
%build | ||
CONFIGURE_OPTS=( | ||
-Dpython=disabled | ||
-Dopenssl=disabled | ||
-Djson-c=disabled | ||
-Dkeyutils=disabled | ||
|
||
-Ddocs-build=false | ||
) | ||
|
||
%cross_meson "${CONFIGURE_OPTS[@]}" | ||
%cross_meson_build | ||
|
||
%install | ||
%cross_meson_install | ||
|
||
%files | ||
%license COPYING ccan/licenses/BSD-MIT ccan/licenses/CC0 | ||
%{_cross_libdir}/*.so.* | ||
%{_cross_attribution_file} | ||
|
||
%files devel | ||
%{_cross_includedir}/*.h | ||
%{_cross_includedir}/nvme/*.h | ||
%{_cross_libdir}/*.so | ||
%{_cross_libdir}/pkgconfig/*.pc | ||
|
||
%changelog |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
[package] | ||
name = "nvme-cli" | ||
version = "0.1.0" | ||
edition = "2021" | ||
publish = false | ||
build = "../build.rs" | ||
|
||
[lib] | ||
path = "../packages.rs" | ||
|
||
[package.metadata.build-package] | ||
releases-url = "https://github.com/linux-nvme/nvme-cli/releases" | ||
|
||
[[package.metadata.build-package.external-files]] | ||
url = "https://github.com/linux-nvme/nvme-cli/archive/v2.9.1/nvme-cli-2.9.1.tar.gz" | ||
sha512 = "c9c86e7567c2d4c59aff1eb9d18f4775923db3c81a89c628b819121c32150d4bc2d65d0dacac764c64594369890b380d0fd06bc7c1f83f4a7f3e71a51a6fee24" | ||
|
||
[build-dependencies] | ||
glibc = { path = "../glibc" } | ||
libnvme = { path = "../libnvme" } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
Name: %{_cross_os}nvme-cli | ||
Version: 2.9.1 | ||
Release: 1%{?dist} | ||
Summary: CLI to interact with NVMe devices | ||
License: LGPL-2.1-only AND GPL-2.0-only AND CC0-1.0 AND MIT | ||
URL: https://github.com/linux-nvme/nvme-cli | ||
Source0: https://github.com/linux-nvme/nvme-cli/archive/v%{version}/nvme-cli-%{version}.tar.gz | ||
|
||
BuildRequires: meson | ||
BuildRequires: %{_cross_os}glibc-devel | ||
BuildRequires: %{_cross_os}libnvme-devel | ||
Requires: %{_cross_os}libnvme | ||
|
||
%description | ||
%{summary}. | ||
|
||
%prep | ||
%autosetup -n nvme-cli-%{version} -p1 | ||
|
||
%build | ||
CONFIGURE_OPTS=( | ||
-Ddocs=false | ||
-Ddocs-build=false | ||
-Djson-c=disabled | ||
) | ||
|
||
%cross_meson "${CONFIGURE_OPTS[@]}" | ||
%cross_meson_build | ||
|
||
%install | ||
%cross_meson_install | ||
# This is an empty configuration file with comments with examples of how to | ||
# configure the systemd services | ||
rm %{buildroot}%{_sysconfdir}/nvme/discovery.conf | ||
|
||
%files | ||
%license LICENSE ccan/licenses/LGPL-2.1 ccan/licenses/BSD-MIT ccan/licenses/CC0 | ||
%{_cross_attribution_file} | ||
%{_cross_sbindir}/nvme | ||
%exclude %{_cross_udevrulesdir} | ||
%exclude %{_cross_unitdir} | ||
%exclude %{_cross_datadir} | ||
%exclude %{_cross_prefix}/lib/dracut | ||
|
||
%changelog |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,15 @@ | ||
ACTION=="remove", GOTO="ebs_volumes_end" | ||
KERNEL!="nvme*", GOTO="ebs_volumes_end" | ||
SUBSYSTEM!="block", GOTO="ebs_volumes_end" | ||
ENV{DEVTYPE}!="disk", GOTO="ebs_volumes_end" | ||
ATTRS{model}!="Amazon Elastic Block Store", GOTO="ebs_volumes_end" | ||
|
||
# Follow AWS recommendation of never timing out IO on EBS volumes attached via NVMe | ||
KERNEL=="nvme*", ATTRS{model}=="Amazon Elastic Block Store", ATTR{queue/io_timeout}="4294967295" | ||
ENV{DEVTYPE}=="disk", ATTR{queue/io_timeout}="4294967295" | ||
|
||
# Add symlink for disk | ||
KERNEL=="nvme[0-9]*n[0-9]*", ENV{DEVTYPE}=="disk", IMPORT{program}="/usr/bin/ghostdog ebs-device-name $devnode", SYMLINK+="$env{XVD_DEVICE_NAME}" | ||
|
||
# Add symlink for partition | ||
KERNEL=="nvme[0-9]*n[0-9]*p[0-9]*", ENV{DEVTYPE}=="partition", IMPORT{parent}="XVD_DEVICE_NAME", SYMLINK+="$env{XVD_DEVICE_NAME}$number" | ||
|
||
LABEL="ebs_volumes_end" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,11 +8,15 @@ use gptman::GPT; | |
use hex_literal::hex; | ||
use lazy_static::lazy_static; | ||
use signpost::uuid_to_guid; | ||
use snafu::ResultExt; | ||
use snafu::{ensure, ResultExt}; | ||
use std::collections::HashSet; | ||
use std::fs; | ||
use std::io::{Read, Seek}; | ||
use std::path::PathBuf; | ||
use std::process::Command; | ||
|
||
const NVME_CLI_PATH: &str = "/sbin/nvme"; | ||
const NVME_IDENTIFY_DATA_SIZE: usize = 4096; | ||
|
||
#[derive(FromArgs, PartialEq, Debug)] | ||
/// Manage ephemeral disks. | ||
|
@@ -25,6 +29,7 @@ struct Args { | |
#[argh(subcommand)] | ||
enum SubCommand { | ||
Scan(ScanArgs), | ||
EbsDeviceName(EbsDeviceNameArgs), | ||
} | ||
|
||
#[derive(FromArgs, PartialEq, Debug)] | ||
|
@@ -35,6 +40,14 @@ struct ScanArgs { | |
device: PathBuf, | ||
} | ||
|
||
#[derive(FromArgs, PartialEq, Debug)] | ||
#[argh(subcommand, name = "ebs-device-name")] | ||
/// Returns the device name used for the EBS device | ||
struct EbsDeviceNameArgs { | ||
#[argh(positional)] | ||
device: PathBuf, | ||
} | ||
|
||
// Main entry point. | ||
fn run() -> Result<()> { | ||
let args: Args = argh::from_env(); | ||
|
@@ -45,6 +58,11 @@ fn run() -> Result<()> { | |
let device_type = find_device_type(&mut f)?; | ||
emit_device_type(&device_type); | ||
} | ||
SubCommand::EbsDeviceName(ebs_device_name) => { | ||
let path = ebs_device_name.device; | ||
let device_name = find_device_name(format!("{}", path.display()))?; | ||
emit_device_name(&device_name); | ||
} | ||
} | ||
Ok(()) | ||
} | ||
|
@@ -75,11 +93,43 @@ where | |
Ok(device_type.to_string()) | ||
} | ||
|
||
/// Finds the device name using the nvme-cli | ||
fn find_device_name(path: String) -> Result<String> { | ||
// nvme-cli writes the binary data to STDOUT | ||
let output = Command::new(NVME_CLI_PATH) | ||
.args(["id-ctrl", &path, "-b"]) | ||
.output() | ||
.context(error::NvmeCommandSnafu { path: path.clone() })?; | ||
|
||
parse_device_name(&output.stdout, path) | ||
} | ||
|
||
/// Parses the device name from the binary data returned by nvme-cli | ||
fn parse_device_name(device_info: &[u8], path: String) -> Result<String> { | ||
// Bail out if the data returned isn't complete | ||
ensure!( | ||
device_info.len() == NVME_IDENTIFY_DATA_SIZE, | ||
error::InvalidDeviceInfoSnafu { path } | ||
); | ||
|
||
// The vendor data is stored at the last 1024 bytes | ||
// The device name is stored at the first 32 bytes of the vendor data | ||
let offset = NVME_IDENTIFY_DATA_SIZE - 1024; | ||
let device_name = &device_info[offset..offset + 32]; | ||
|
||
Ok(String::from_utf8_lossy(device_name).trim_end().to_string()) | ||
} | ||
|
||
/// Print the device type in the environment key format udev expects. | ||
fn emit_device_type(device_type: &str) { | ||
println!("BOTTLEROCKET_DEVICE_TYPE={}", device_type); | ||
} | ||
|
||
/// Print the device name in the environment key format udev expects. | ||
fn emit_device_name(device_name: &str) { | ||
println!("XVD_DEVICE_NAME={}", device_name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per https://docs.aws.amazon.com/ebs/latest/userguide/nvme-ebs-volumes.html I believe this is fine because:
Therefore this will give us a valid environment key assignment of the form XVD_DEVICE_NAME=/some/stuff and udev will be happy. |
||
} | ||
|
||
// Known system partition types for Bottlerocket. | ||
lazy_static! { | ||
static ref SYSTEM_PARTITION_TYPES: HashSet<[u8; 16]> = [ | ||
|
@@ -114,6 +164,13 @@ mod error { | |
path: std::path::PathBuf, | ||
source: std::io::Error, | ||
}, | ||
#[snafu(display("Failed to execute NVMe command for device '{}': {}", path.display(), source))] | ||
NvmeCommand { | ||
path: std::path::PathBuf, | ||
source: std::io::Error, | ||
}, | ||
#[snafu(display("Invalid device info for device '{}'", path.display()))] | ||
InvalidDeviceInfo { path: std::path::PathBuf }, | ||
} | ||
} | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What form do you expect XVD_DEVICE_NAME to take? Is appending the partition number to that going to be unambiguous? I ask because if XVD_DEVICE_NAME is something like /dev/sda1, you could get a symlink for /dev/sda11 here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that this is exactly what AL2023 does, so That Will Be Fine (probably, not provably, but what do you want). The XVD_DEVICE_NAME from the controller is of the form xvda, and the partitions get named xvda1, xvda128, and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For attached-after-boot EBS, /dev/sdb and /dev/sdb1 appear. Also fine.