feat(azure-nvme-id): refactor and introduce unit tests

Refactor main.c into several modules to support unit
testing and add unit test coverage for all methods.

No functional changes except a debug log or two.

CI changes:
- add Ubuntu 24.04 image to os matrix
- move dependency installation into build-deb.sh
- run unit tests in deb
- enforce clang-format
- enforce cppcheck
- enforce unit tests

Signed-off-by: Chris Patterson <[email protected]>

.clang-format

@@ -0,0 +1 @@
+BasedOnStyle: Microsoft

.github/workflows/debs.yml

@@ -7,7 +7,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-20.04, ubuntu-22.04]
+        os: [ubuntu-20.04, ubuntu-22.04, ubuntu-24.04]
     defaults:
       run:
         shell: bash
@@ -24,10 +24,6 @@ jobs:
       run: |
         git remote add upstream https://github.com/Azure/azure-vm-utils.git
         git fetch upstream --tags
-    - name: Setup
-      run: |
-        sudo apt update
-        sudo apt install gcc pandoc cmake devscripts debhelper -y
     - name: Build debs
       run: |
         DEBEMAIL="Azure VM Utils CI <azure/[email protected]>" ./scripts/build-deb.sh

.github/workflows/main.yml

@@ -7,7 +7,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-20.04, ubuntu-22.04]
+        os: [ubuntu-20.04, ubuntu-22.04, ubuntu-24.04]
     defaults:
       run:
         shell: bash
@@ -27,7 +27,7 @@ jobs:
     - name: Setup
       run: |
         sudo apt update
-        sudo apt install gcc pandoc cmake -y
+        sudo apt install gcc pandoc clang-format cmake -y
     - name: Build & install project with cmake
       run: |
         cmake -B build -S .
@@ -57,3 +57,18 @@ jobs:
         cmake -DGENERATE_MANPAGES=1 -B build -S .
         make -C build
         if ! grep -c "Pandoc" build/doc/azure-nvme-id.8; then echo "manpage not generated by pandoc"; exit 1; fi
+    - name: Run tests
+      run: |
+        set -x
+        sudo apt install -y libcmocka-dev cppcheck clang-format
+        rm -rf build
+        cmake -DENABLE_TESTS=1 -B build -S .
+        cd build
+        make
+        ctest --verbose -j
+    - name: Check source formatting with clang-format
+      run: |
+        make -C build check-clang-format || (echo "Run 'make clang-format' to fix formatting issues" && exit 1)
+    - name: Check cppcheck
+      run: |
+        make -C build cppcheck

CMakeLists.txt

@@ -18,7 +18,7 @@ include(CTest)
 enable_testing()
 
 add_compile_options(-Wextra -Wall $<$<CONFIG:Debug>:-Werror> -std=gnu11 -D_GNU_SOURCE=1)
-add_executable(azure-nvme-id src/main.c)
+add_executable(azure-nvme-id src/debug.c src/identify_disks.c src/identify_udev.c src/main.c src/nvme.c src/util.c)
 
 set(AZURE_NVME_ID_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/sbin")
 set(DRACUT_MODULES_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/lib/dracut/modules.d/97azure-disk" CACHE PATH "dracut modules installation directory")
@@ -59,3 +59,8 @@ endif()
 set(CPACK_PROJECT_NAME ${PROJECT_NAME})
 set(CPACK_PROJECT_VERSION ${PROJECT_VERSION})
 include(CPack)
+
+option(ENABLE_TESTS "Enable unit tests" OFF)
+if(ENABLE_TESTS)
+    include(cmake/tests.cmake)
+endif()

cmake/clangformat.cmake

@@ -1,9 +1,11 @@
-file(GLOB_RECURSE ALL_SOURCE_FILES *.c *.h)
+file(GLOB ALL_SOURCE_FILES src/*.[ch] tests/*.[ch])
 
-add_custom_target(
-    clangformat
-    COMMAND /usr/bin/clang-format
-    -style=Microsoft
-    -i
-    ${ALL_SOURCE_FILES}
+add_custom_target(clang-format
+    COMMAND clang-format -i ${ALL_SOURCE_FILES}
+    COMMENT "Running clang-format on source files"
+)
+
+add_custom_target(check-clang-format
+    COMMAND clang-format --dry-run --Werror ${ALL_SOURCE_FILES}
+    COMMENT "Running clang-format check on source files"
 )

cmake/cppcheck.cmake

@@ -1,9 +1,14 @@
-file(GLOB_RECURSE ALL_SOURCE_FILES *.c *.h)
+file(GLOB ALL_SOURCE_FILES src/*.[ch] tests/*.[ch])
 
-add_custom_target(
-        cppcheck
-        COMMAND /usr/bin/cppcheck
-        --enable=all
-        --suppress=missingIncludeSystem
-        ${ALL_SOURCE_FILES}
+set(CPPCHECK_COMMAND
+    cppcheck
+    --enable=all
+    --suppress=missingIncludeSystem
+    -I${CMAKE_SOURCE_DIR}/src
+    ${ALL_SOURCE_FILES}
+)
+
+add_custom_target(cppcheck
+    COMMAND ${CPPCHECK_COMMAND}
+    COMMENT "Running cppcheck on source files"
 )

cmake/tests.cmake

@@ -0,0 +1,50 @@
+find_package(PkgConfig REQUIRED)
+pkg_check_modules(CMOCKA REQUIRED cmocka)
+
+function(add_test_executable test_name)
+    set(multiValueArgs WRAPPED_FUNCTIONS SOURCES CFLAGS)
+    cmake_parse_arguments(TEST "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+
+    add_executable(${test_name} ${TEST_SOURCES} tests/capture.c)
+    add_test(NAME ${test_name} COMMAND ${test_name})
+
+    # LTO may be enabled by packaging, but it must be disabled for --wrap to work.
+    target_compile_options(${test_name} PRIVATE -g -O0 -fno-lto ${CMOCKA_CFLAGS_OTHER} ${TEST_CFLAGS})
+    target_link_options(${test_name} PRIVATE -flto=n)
+
+    target_include_directories(${test_name} PRIVATE ${CMOCKA_INCLUDE_DIRS} src tests)
+    target_link_directories(${test_name} PRIVATE ${CMOCKA_LIBRARY_DIRS})
+    target_link_libraries(${test_name} PRIVATE ${CMOCKA_LIBRARIES} dl)
+
+    if(TEST_WRAPPED_FUNCTIONS)
+        foreach(func ${TEST_WRAPPED_FUNCTIONS})
+            target_link_options(${test_name} PRIVATE -Wl,--wrap=${func})
+        endforeach()
+    endif()
+endfunction()
+
+add_test_executable(debug_tests
+    WRAPPED_FUNCTIONS
+    SOURCES src/debug.c tests/debug_tests.c
+)
+
+add_test_executable(identify_disks_tests
+    WRAPPED_FUNCTIONS nvme_identify_namespace_vs_for_namespace_device
+    SOURCES src/debug.c src/identify_disks.c src/nvme.c src/util.c tests/identify_disks_tests.c
+    CFLAGS -DUNIT_TESTING_SYS_CLASS_NVME=1
+)
+
+add_test_executable(identify_udev_tests
+    WRAPPED_FUNCTIONS nvme_identify_namespace_vs_for_namespace_device
+    SOURCES src/debug.c src/identify_udev.c src/nvme.c tests/identify_udev_tests.c
+)
+
+add_test_executable(nvme_tests
+    WRAPPED_FUNCTIONS open posix_memalign ioctl close
+    SOURCES src/nvme.c tests/nvme_tests.c
+)
+
+add_test_executable(util_tests
+    WRAPPED_FUNCTIONS fopen fseek ftell fread fclose malloc
+    SOURCES src/util.c tests/util_tests.c
+)

packaging/debian/control

@@ -2,7 +2,7 @@ Source: azure-vm-utils
 Section: utils
 Priority: optional
 Maintainer: Chris Patterson <[email protected]>
-Build-Depends: cmake, pandoc, debhelper-compat (= 12)
+Build-Depends: cmake, pandoc, debhelper-compat (= 12), libcmocka-dev
 Standards-Version: 4.5.0
 Homepage: https://github.com/Azure/azure-vm-utils
 Rules-Requires-Root: no

packaging/debian/rules

@@ -6,3 +6,9 @@ export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed
 
 %:
 	dh $@
+
+override_dh_auto_configure:
+	dh_auto_configure -- -DENABLE_TESTS=1
+
+override_dh_auto_test:
+	CTEST_OUTPUT_ON_FAILURE=1 dh_auto_test

scripts/build-deb.sh

@@ -2,6 +2,10 @@
 
 set -eux -o pipefail
 
+# Ensure dependencies are installed and up-to-date.
+sudo apt update
+sudo apt install gcc pandoc cmake devscripts debhelper libcmocka-dev build-essential clang-format cppcheck -y
+
 project_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 git_version="$(git describe --tags --always --dirty)"
 git_ref="$(echo "${git_version}" | sed 's/.*-g//' | sed 's/-dirty/DIRTY/')"

src/debug.c

@@ -0,0 +1,25 @@
+/**
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License. See LICENSE in the project root for license
+ * information.
+ */
+
+#include "debug.h"
+
+bool debug = false;
+extern char **environ;
+
+/**
+ * Dump environment variables.
+ */
+void debug_environment_variables()
+{
+    int i = 0;
+
+    DEBUG_PRINTF("Environment Variables:\n");
+    while (environ[i])
+    {
+        DEBUG_PRINTF("%s\n", environ[i]);
+        i++;
+    }
+}

src/debug.h

@@ -0,0 +1,26 @@
+/**
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License. See LICENSE in the project root for license
+ * information.
+ */
+
+#ifndef __DEBUG_H__
+#define __DEBUG_H__
+
+#include <stdbool.h>
+#include <stdio.h>
+
+extern bool debug;
+
+#define DEBUG_PRINTF(fmt, ...)                                                                                         \
+    do                                                                                                                 \
+    {                                                                                                                  \
+        if (debug == true)                                                                                             \
+        {                                                                                                              \
+            fprintf(stderr, "DEBUG: " fmt, ##__VA_ARGS__);                                                             \
+        }                                                                                                              \
+    } while (0)
+
+void debug_environment_variables();
+
+#endif // __DEBUG_H__

src/identify_disks.c

@@ -0,0 +1,124 @@
+
+#include <dirent.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "debug.h"
+#include "identify_disks.h"
+#include "nvme.h"
+#include "util.h"
+
+/**
+ * Callback for scandir() to filter for NVMe namespaces.
+ */
+int is_nvme_namespace(const struct dirent *entry)
+{
+    unsigned int ctrl, nsid;
+    char p;
+
+    // Check for NVME controller name format: nvme<ctrl id>n<ns id>
+    return sscanf(entry->d_name, "nvme%un%u%c", &ctrl, &nsid, &p) == 2;
+}
+
+/**
+ * Enumerate namespaces for a controller.
+ */
+int enumerate_namespaces_for_controller(struct nvme_controller *ctrl)
+{
+    struct dirent **namelist;
+
+    int n = scandir(ctrl->sys_path, &namelist, is_nvme_namespace, alphasort);
+    if (n < 0)
+    {
+        fprintf(stderr, "failed scandir for %s: %m\n", ctrl->sys_path);
+        return 1;
+    }
+
+    DEBUG_PRINTF("found %d namespace(s) for controller=%s:\n", n, ctrl->name);
+    for (int i = 0; i < n; i++)
+    {
+        char namespace_path[MAX_PATH];
+        snprintf(namespace_path, sizeof(namespace_path), "/dev/%s", namelist[i]->d_name);
+
+        char *vs = nvme_identify_namespace_vs_for_namespace_device(namespace_path);
+        if (vs != NULL)
+        {
+            printf("%s: %s\n", namespace_path, vs);
+            free(vs);
+        }
+        free(namelist[i]);
+    }
+
+    free(namelist);
+    return 0;
+}
+
+/**
+ * Check if NVMe vendor in sysfs matches Microsoft's vendor ID.
+ */
+int is_microsoft_nvme_device(const char *device_name)
+{
+    char vendor_id_path[MAX_PATH];
+    snprintf(vendor_id_path, sizeof(vendor_id_path), "%s/%s/device/vendor", SYS_CLASS_NVME_PATH, device_name);
+
+    char *vendor_id_string = read_file_as_string(vendor_id_path);
+    if (vendor_id_string == NULL)
+    {
+        return false;
+    }
+
+    long int vendor_id = strtol(vendor_id_string, NULL, 16);
+    free(vendor_id_string);
+
+    return vendor_id == MICROSOFT_NVME_VENDOR_ID;
+}
+
+/**
+ * Callback for scandir() to filter for Microsoft Azure NVMe controllers.
+ */
+int is_azure_nvme_controller(const struct dirent *entry)
+{
+    unsigned int ctrl;
+    char p;
+
+    // Check for NVME controller name format: nvme<int>
+    if (sscanf(entry->d_name, "nvme%u%c", &ctrl, &p) != 1)
+    {
+        return false;
+    }
+
+    return is_microsoft_nvme_device(entry->d_name);
+}
+
+/**
+ * Enumerate Microsoft Azure NVMe controllers and identify disks.
+ */
+int identify_disks(void)
+{
+    struct dirent **namelist;
+
+    int n = scandir(SYS_CLASS_NVME_PATH, &namelist, is_azure_nvme_controller, alphasort);
+    if (n < 0)
+    {
+        fprintf(stderr, "no NVMe devices in %s: %m\n", SYS_CLASS_NVME_PATH);
+        return 0;
+    }
+
+    DEBUG_PRINTF("found %d controllers\n", n);
+    for (int i = 0; i < n; i++)
+    {
+        struct nvme_controller ctrl;
+
+        strncpy(ctrl.name, namelist[i]->d_name, sizeof(ctrl.name));
+        snprintf(ctrl.dev_path, sizeof(ctrl.dev_path), "/dev/%s", ctrl.name);
+        snprintf(ctrl.sys_path, sizeof(ctrl.sys_path), "%s/%s", SYS_CLASS_NVME_PATH, ctrl.name);
+
+        enumerate_namespaces_for_controller(&ctrl);
+        free(namelist[i]);
+    }
+
+    free(namelist);
+    return 0;
+}