From 5cc1e898b997376e43b94e0c82fffe9f1904d0e6 Mon Sep 17 00:00:00 2001 From: Alexander Shaduri Date: Sat, 30 Jul 2011 16:16:21 +0000 Subject: [PATCH] Use model name as well when detecting S/N duplicates. --- gsmartcontrol/TODO | 3 +- .../src/applib/storage_detector_linux.cpp | 2 +- .../src/applib/storage_detector_win32.cpp | 56 ++++++++++++++++--- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/gsmartcontrol/TODO b/gsmartcontrol/TODO index 9fbdd0e..a647350 100644 --- a/gsmartcontrol/TODO +++ b/gsmartcontrol/TODO @@ -37,8 +37,7 @@ Bugs / patches: TODO: -Delete drivedb.h (and all related drivedb* files) on uninstall - windows. - add menu item - "update drive database" (not sure about linux, should +Add menu item - "update drive database" (not sure about linux, should check if it's supported first; also need to run it in terminal). diff --git a/gsmartcontrol/src/applib/storage_detector_linux.cpp b/gsmartcontrol/src/applib/storage_detector_linux.cpp index b86a9d7..02cb543 100644 --- a/gsmartcontrol/src/applib/storage_detector_linux.cpp +++ b/gsmartcontrol/src/applib/storage_detector_linux.cpp @@ -232,7 +232,7 @@ inline std::string smartctl_get_drives(const std::string& dev, const std::string } if (!error_msg.empty()) { - debug_out_info("app", "Smartctl returned with an error: " << error_msg); + debug_out_info("app", "Smartctl returned with an error: " << error_msg << "\n"); } else { drives.push_back(drive); } diff --git a/gsmartcontrol/src/applib/storage_detector_win32.cpp b/gsmartcontrol/src/applib/storage_detector_win32.cpp index 10db305..abc299d 100644 --- a/gsmartcontrol/src/applib/storage_detector_win32.cpp +++ b/gsmartcontrol/src/applib/storage_detector_win32.cpp @@ -10,6 +10,7 @@ #include // CreateFileA(), CloseHandle(), etc... #include +#include #include "hz/win32_tools.h" #include "hz/string_sprintf.h" @@ -19,7 +20,7 @@ /* -3ware Windows (XP so far, maybe the same under the others): +3ware Windows: For 3ware 9xxx only. Call as: smartctl -i sd[a-z],N N is port, a-z is logical drive (unit) provided by controller. @@ -72,7 +73,7 @@ namespace { /// a port parameter. We don't pick the others because the may /// conflict with pd* devices, and we like pd* better than sd*. std::string get_scan_open_multiport_devices(std::vector& drives, - ExecutorFactoryRefPtr ex_factory, std::vector& equivalent_pds) + ExecutorFactoryRefPtr ex_factory, std::set& equivalent_pds) { hz::intrusive_ptr smartctl_ex = ex_factory->create_executor(ExecutorFactory::ExecutorSmartctl); @@ -128,7 +129,7 @@ std::string get_scan_open_multiport_devices(std::vector& dr std::string letter; if (dev_re.PartialMatch(dev, &letter)) { // don't use pd* devices equivalent to these sd* devices. - equivalent_pds.push_back(letter.at(0) - 'a'); + equivalent_pds.insert(letter.at(0) - 'a'); } std::string full_dev = dev + "," + port_str; @@ -153,16 +154,32 @@ std::string get_scan_open_multiport_devices(std::vector& dr std::string detect_drives_win32(std::vector& drives, ExecutorFactoryRefPtr ex_factory) { - std::vector used_pds; + hz::intrusive_ptr smartctl_ex = ex_factory->create_executor(ExecutorFactory::ExecutorSmartctl); + + // Fetch multiport devices using --scan-open. + std::set used_pds; std::string error_msg = get_scan_open_multiport_devices(drives, ex_factory, used_pds); bool multiport_found = !drives.empty(); + // Find out their serial numbers + std::set serials; + for (std::size_t i = 0; i < drives.size(); ++i) { + std::string local_error = drives.at(i)->fetch_basic_data_and_parse(smartctl_ex); + if (!local_error.empty()) { + debug_out_info("app", "Smartctl returned with an error: " << error_msg << "\n"); + // Don't exit, just report it. + } + if (!drives.at(i)->get_serial_number().empty()) { + serials.insert(drives.at(i)->get_model_name() + "_" + drives.at(i)->get_serial_number()); + } + } + + // Scan PhysicalDrive entries for (int drive_num = 0; ; ++drive_num) { std::string name = hz::string_sprintf("\\\\.\\PhysicalDrive%d", drive_num); - // If the drive is openable, then it's there. + // If the drive is openable, then it's there. Yes, CreateFile() is open, not create. // NOTE: Administrative privileges are required to open it. - // Yes, CreateFile() is open, not create. Yes, it's silly (ah, win32...). // We don't use any long/unopenable files here, so use the ANSI version. HANDLE h = CreateFileA(name.c_str(), 0, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL); @@ -173,10 +190,31 @@ std::string detect_drives_win32(std::vector& drives, Execut CloseHandle(h); - if (std::find(used_pds.begin(), used_pds.end(), drive_num) == used_pds.end()) { - std::string dev = hz::string_sprintf("pd%d", drive_num); - drives.push_back(new StorageDevice(dev)); + // If the drive was already encountered in --scan-open (with a port number), skip it. + if (used_pds.count(drive_num) > 0) { + continue; } + + StorageDeviceRefPtr drive(new StorageDevice(hz::string_sprintf("pd%d", drive_num))); + + // Sometimes, a single physical drive may be accessible from both "/.//PhysicalDriveN" + // and "/.//Scsi2" (e.g. pd0 and csmi2,1). Prefer the port-having one (which is from --scan-open). + // The only way to detect these duplicates is to compare them using serial numbers. + if (multiport_found) { + std::string local_error = drive->fetch_basic_data_and_parse(smartctl_ex); + if (!local_error.empty()) { + debug_out_info("app", "Smartctl returned with an error: " << error_msg << "\n"); + // Don't exit, just report it. + } + if (!drive->get_serial_number().empty() + && serials.count(drive->get_model_name() + "_" + drive->get_serial_number()) > 0) { + debug_out_info("app", "Skipping duplicate drive: model: \"" << drive->get_model_name() + << "\", S/N: \"" << drive->get_serial_number() << "\".\n"); + continue; + } + } + + drives.push_back(drive); }