From 73463e5c23660a06979d128061bca746575c36ae Mon Sep 17 00:00:00 2001 From: Alexander Shaduri Date: Wed, 20 Mar 2024 20:57:16 +0400 Subject: [PATCH] Cleanup of output format / device type handling. --- src/applib/selftest.cpp | 4 +- src/applib/smartctl_json_ata_parser.cpp | 2 +- src/applib/smartctl_parser.cpp | 37 +++++++++++------ src/applib/smartctl_parser.h | 4 +- src/applib/smartctl_parser_types.h | 49 ++++++----------------- src/applib/smartctl_text_ata_parser.cpp | 2 +- src/applib/smartctl_version_parser.cpp | 27 +++++++------ src/applib/smartctl_version_parser.h | 6 +-- src/applib/storage_device.cpp | 27 +++++++------ src/applib/tests/test_smartctl_parser.cpp | 8 ++-- 10 files changed, 79 insertions(+), 87 deletions(-) diff --git a/src/applib/selftest.cpp b/src/applib/selftest.cpp index 556d0b6..2952581 100644 --- a/src/applib/selftest.cpp +++ b/src/applib/selftest.cpp @@ -19,7 +19,7 @@ Copyright: #include "smartctl_text_ata_parser.h" #include "selftest.h" #include "ata_storage_property_descr.h" - +#include "smartctl_version_parser.h" @@ -243,7 +243,7 @@ std::string SelfTest::update(const std::shared_ptr& smartctl_ex return error_msg; const AtaStorageAttribute::DiskType disk_type = drive_->get_is_hdd() ? AtaStorageAttribute::DiskType::Hdd : AtaStorageAttribute::DiskType::Ssd; - auto parser = SmartctlParser::create(SmartctlParserType::TextAta); + auto parser = SmartctlParser::create(SmartctlParserType::Ata, SmartctlVersionParser::get_default_format(SmartctlParserType::Ata)); DBG_ASSERT_RETURN(parser, "Cannot create parser"); auto parse_status = parser->parse(output); diff --git a/src/applib/smartctl_json_ata_parser.cpp b/src/applib/smartctl_json_ata_parser.cpp index 3a7a071..87337f8 100644 --- a/src/applib/smartctl_json_ata_parser.cpp +++ b/src/applib/smartctl_json_ata_parser.cpp @@ -153,7 +153,7 @@ hz::ExpectedVoid SmartctlJsonAtaParser::parse_version(const p.section = AtaStorageProperty::Section::info; // add to info section add_property(p); } - if (!SmartctlVersionParser::check_parsed_version(SmartctlParserType::JsonAta, smartctl_version)) { + if (!SmartctlVersionParser::check_format_supported(SmartctlOutputFormat::Json, smartctl_version)) { debug_out_warn("app", DBG_FUNC_MSG << "Incompatible smartctl version. Returning.\n"); return hz::Unexpected(SmartctlParserError::IncompatibleVersion, "Incompatible smartctl version."); } diff --git a/src/applib/smartctl_parser.cpp b/src/applib/smartctl_parser.cpp index 8f09437..28a97ef 100644 --- a/src/applib/smartctl_parser.cpp +++ b/src/applib/smartctl_parser.cpp @@ -21,20 +21,31 @@ Copyright: -std::unique_ptr SmartctlParser::create(SmartctlParserType type) +std::unique_ptr SmartctlParser::create(SmartctlParserType type, SmartctlOutputFormat format) { switch(type) { - case SmartctlParserType::JsonBasic: - return std::make_unique(); + case SmartctlParserType::Basic: + switch(format) { + case SmartctlOutputFormat::Json: + return std::make_unique(); + break; + case SmartctlOutputFormat::Text: + return std::make_unique(); + break; + } break; - case SmartctlParserType::JsonAta: - return std::make_unique(); + case SmartctlParserType::Ata: + switch(format) { + case SmartctlOutputFormat::Json: + return std::make_unique(); + break; + case SmartctlOutputFormat::Text: + return std::make_unique(); + break; + } break; - case SmartctlParserType::TextBasic: - return std::make_unique(); - break; - case SmartctlParserType::TextAta: - return std::make_unique(); + case SmartctlParserType::Nvme: + // TODO break; } return nullptr; @@ -42,7 +53,7 @@ std::unique_ptr SmartctlParser::create(SmartctlParserType type) -hz::ExpectedValue SmartctlParser::detect_output_format(std::string_view smartctl_output) +hz::ExpectedValue SmartctlParser::detect_output_format(std::string_view smartctl_output) { // Look for the first non-whitespace symbol const auto* first_symbol = std::find_if(smartctl_output.begin(), smartctl_output.end(), [&](char c) { @@ -50,10 +61,10 @@ hz::ExpectedValue SmartctlParser::det }); if (first_symbol != smartctl_output.end()) { if (*first_symbol == '{') { - return SmartctlParserFormat::Json; + return SmartctlOutputFormat::Json; } if (smartctl_output.rfind("smartctl", static_cast(first_symbol - smartctl_output.begin())) == 0) { - return SmartctlParserFormat::Text; + return SmartctlOutputFormat::Text; } return hz::Unexpected(SmartctlParserError::UnsupportedFormat, "Unsupported format while trying to detect smartctl output format."); } diff --git a/src/applib/smartctl_parser.h b/src/applib/smartctl_parser.h index 784551d..509c780 100644 --- a/src/applib/smartctl_parser.h +++ b/src/applib/smartctl_parser.h @@ -66,7 +66,7 @@ class SmartctlParser { /// Create an instance of this class. /// \return nullptr if no such class exists - static std::unique_ptr create(SmartctlParserType type); + static std::unique_ptr create(SmartctlParserType type, SmartctlOutputFormat format); /// Parse full "smartctl -x" output. @@ -75,7 +75,7 @@ class SmartctlParser { /// Detect smartctl output type (text, json). - [[nodiscard]] static hz::ExpectedValue detect_output_format(std::string_view smartctl_output); + [[nodiscard]] static hz::ExpectedValue detect_output_format(std::string_view smartctl_output); /// Get parsed properties. diff --git a/src/applib/smartctl_parser_types.h b/src/applib/smartctl_parser_types.h index 37262aa..7bc02cb 100644 --- a/src/applib/smartctl_parser_types.h +++ b/src/applib/smartctl_parser_types.h @@ -19,45 +19,22 @@ Copyright: enum class SmartctlParserType { - JsonBasic, ///< Info only - JsonAta, - TextBasic, ///< Info only - TextAta, + Basic, ///< Info only, supports all types of devices + Ata, ///< (S)ATA + Nvme, ///< NVMe +// Scsi, ///< SCSI }; -/// Helper structure for enum-related functions -struct SmartctlParserTypeExt - : public hz::EnumHelper< - SmartctlParserType, - SmartctlParserTypeExt, - Glib::ustring> -{ - static constexpr inline SmartctlParserType default_value = SmartctlParserType::JsonAta; - - static std::unordered_map> build_enum_map() - { - return { - {SmartctlParserType::JsonBasic, {"json_basic", _("JSON Basic")}}, - {SmartctlParserType::JsonAta, {"json_ata", _("JSON ATA")}}, - {SmartctlParserType::TextBasic, {"text_basic", _("Text Basic")}}, - {SmartctlParserType::TextAta, {"text_ata", _("Text ATA")}}, - }; - } - -}; - - - -enum class SmartctlParserFormat { +enum class SmartctlOutputFormat { Json, Text, }; -enum class SmartctlParserSettingType { +enum class SmartctlParserPreferenceType { Auto, Json, Text, @@ -66,20 +43,20 @@ enum class SmartctlParserSettingType { /// Helper structure for enum-related functions -struct SmartctlParserSettingTypeExt +struct SmartctlParserPreferenceTypeExt : public hz::EnumHelper< - SmartctlParserSettingType, - SmartctlParserSettingTypeExt, + SmartctlParserPreferenceType, + SmartctlParserPreferenceTypeExt, Glib::ustring> { - static constexpr inline SmartctlParserSettingType default_value = SmartctlParserSettingType::Auto; + static constexpr inline SmartctlParserPreferenceType default_value = SmartctlParserPreferenceType::Auto; static std::unordered_map> build_enum_map() { return { - {SmartctlParserSettingType::Auto, {"auto", _("Automatic")}}, - {SmartctlParserSettingType::Json, {"json", _("JSON")}}, - {SmartctlParserSettingType::Text, {"text", _("Text")}}, + {SmartctlParserPreferenceType::Auto, {"auto", _("Automatic")}}, + {SmartctlParserPreferenceType::Json, {"json", _("JSON")}}, + {SmartctlParserPreferenceType::Text, {"text", _("Text")}}, }; } diff --git a/src/applib/smartctl_text_ata_parser.cpp b/src/applib/smartctl_text_ata_parser.cpp index 8deaf81..9bb5096 100644 --- a/src/applib/smartctl_text_ata_parser.cpp +++ b/src/applib/smartctl_text_ata_parser.cpp @@ -223,7 +223,7 @@ hz::ExpectedVoid SmartctlTextAtaParser::parse(std::string_v add_property(p); } - if (!SmartctlVersionParser::check_parsed_version(SmartctlParserType::TextAta, version)) { + if (!SmartctlVersionParser::check_format_supported(SmartctlOutputFormat::Text, version)) { debug_out_warn("app", DBG_FUNC_MSG << "Incompatible smartctl version. Returning.\n"); return hz::Unexpected(SmartctlParserError::IncompatibleVersion, "Incompatible smartctl version."); } diff --git a/src/applib/smartctl_version_parser.cpp b/src/applib/smartctl_version_parser.cpp index fa4451d..b35acce 100644 --- a/src/applib/smartctl_version_parser.cpp +++ b/src/applib/smartctl_version_parser.cpp @@ -53,16 +53,14 @@ std::optional SmartctlVersionParser::get_numeric_version(const std::stri -bool SmartctlVersionParser::check_parsed_version(SmartctlParserType parser_type, const std::string& version_only) +bool SmartctlVersionParser::check_format_supported(SmartctlOutputFormat format, const std::string& version_only) { if (auto numeric_version = get_numeric_version(version_only); numeric_version.has_value()) { - switch(parser_type) { - case SmartctlParserType::JsonBasic: - case SmartctlParserType::JsonAta: - return numeric_version.value() >= minimum_req_json_version; - case SmartctlParserType::TextBasic: - case SmartctlParserType::TextAta: + switch(format) { + case SmartctlOutputFormat::Text: return numeric_version.value() >= minimum_req_text_version; + case SmartctlOutputFormat::Json: + return numeric_version.value() >= minimum_req_json_version; } } return false; @@ -70,14 +68,17 @@ bool SmartctlVersionParser::check_parsed_version(SmartctlParserType parser_type, -std::optional SmartctlVersionParser::detect_supported_parser_type(const std::string& version_only) +SmartctlOutputFormat SmartctlVersionParser::get_default_format(SmartctlParserType parser_type) { - for (auto type : SmartctlParserTypeExt::getAllValues()) { - if (check_parsed_version(type, version_only)) { - return type; - } + switch (parser_type) { + case SmartctlParserType::Basic: + return SmartctlOutputFormat::Json; + case SmartctlParserType::Ata: + return SmartctlOutputFormat::Json; + case SmartctlParserType::Nvme: + return SmartctlOutputFormat::Json; } - return std::nullopt; + return SmartctlOutputFormat::Json; } diff --git a/src/applib/smartctl_version_parser.h b/src/applib/smartctl_version_parser.h index 8d1c7e0..faea864 100644 --- a/src/applib/smartctl_version_parser.h +++ b/src/applib/smartctl_version_parser.h @@ -42,11 +42,11 @@ class SmartctlVersionParser { /// Check that the version of smartctl output can be parsed with a parser. - static bool check_parsed_version(SmartctlParserType parser_type, const std::string& version_only); + static bool check_format_supported(SmartctlOutputFormat format, const std::string& version_only); - /// Detect smartctl parser type based on smartctl version - static std::optional detect_supported_parser_type(const std::string& version_only); + /// Get default output format for a parser type. + static SmartctlOutputFormat get_default_format(SmartctlParserType parser_type); private: diff --git a/src/applib/storage_device.cpp b/src/applib/storage_device.cpp index af6e8e9..dd0a086 100644 --- a/src/applib/storage_device.cpp +++ b/src/applib/storage_device.cpp @@ -23,7 +23,7 @@ Copyright: #include "storage_settings.h" #include "smartctl_executor.h" #include "smartctl_version_parser.h" -#include "smartctl_text_parser_helper.h" +//#include "smartctl_text_parser_helper.h" #include "ata_storage_property_descr.h" @@ -143,7 +143,7 @@ std::string StorageDevice::parse_basic_data(bool do_set_properties, bool emit_si AtaStorageAttribute::DiskType disk_type = AtaStorageAttribute::DiskType::Any; // Try the basic parser first. If it succeeds, use the specialized parser. - auto basic_parser = SmartctlParser::create(SmartctlParserType::TextBasic); + auto basic_parser = SmartctlParser::create(SmartctlParserType::Basic, SmartctlOutputFormat::Json); DBG_ASSERT_RETURN(basic_parser, "Cannot create parser"); auto parse_status = basic_parser->parse(this->get_info_output()); @@ -200,7 +200,7 @@ std::string StorageDevice::parse_basic_data(bool do_set_properties, bool emit_si // Note that this may try to parse data the second time (it may already have // been parsed by parse_data() which failed at it). if (do_set_properties) { - auto parser = SmartctlParser::create(SmartctlParserType::TextAta); + auto parser = SmartctlParser::create(SmartctlParserType::Ata, SmartctlOutputFormat::Json); DBG_ASSERT_RETURN(parser, "Cannot create parser"); if (parser->parse(this->info_output_)) { // try to parse it @@ -230,19 +230,25 @@ std::string StorageDevice::fetch_data_and_parse(const std::shared_ptrget_type_argument() == "scsi") { // not sure about correctness... FIXME probably fails with RAID/scsi + const auto default_parser_type = SmartctlVersionParser::get_default_format(SmartctlParserType::Basic); // This doesn't do much yet, but just in case... // SCSI equivalent of -x: - error_msg = execute_device_smartctl("--health --info --attributes --log=error --log=selftest --log=background --log=sasphy", smartctl_ex, output); + std::string command_options = "--health --info --attributes --log=error --log=selftest --log=background --log=sasphy"; + if (default_parser_type == SmartctlOutputFormat::Json) { + // --json flags: o means include original output (just in case). + command_options += " --json=o"; + } + + error_msg = execute_device_smartctl(command_options, smartctl_ex, output); } else { + const auto default_parser_type = SmartctlVersionParser::get_default_format(SmartctlParserType::Ata); // ATA equivalent of -x. std::string command_options = "--health --info --get=all --capabilities --attributes --format=brief --log=xerror,50,error --log=xselftest,50,selftest --log=selective --log=directory --log=scttemp --log=scterc --log=devstat --log=sataphy"; - if (default_parser_type == SmartctlParserSettingType::Json) { + if (default_parser_type == SmartctlOutputFormat::Json) { // --json flags: o means include original output (just in case). command_options += " --json=o"; } @@ -284,12 +290,9 @@ std::string StorageDevice::parse_data() } // TODO Choose format according to device type - SmartctlParserType parser_type = SmartctlParserType::TextAta; - if (parser_format == SmartctlParserFormat::Json) { - parser_type = SmartctlParserType::JsonAta; - } + SmartctlParserType parser_type = SmartctlParserType::Ata; - auto parser = SmartctlParser::create(parser_type); + auto parser = SmartctlParser::create(parser_type, parser_format.value()); DBG_ASSERT_RETURN(parser, "Cannot create parser"); // Try to parse it (parse only, set the properties after basic parsing). diff --git a/src/applib/tests/test_smartctl_parser.cpp b/src/applib/tests/test_smartctl_parser.cpp index 2b5d226..61722b6 100644 --- a/src/applib/tests/test_smartctl_parser.cpp +++ b/src/applib/tests/test_smartctl_parser.cpp @@ -25,17 +25,17 @@ TEST_CASE("SmartctlFormatDetection", "[app][parser]") REQUIRE(SmartctlParser::detect_output_format("smart").error().data() == SmartctlParserError::UnsupportedFormat); - REQUIRE(SmartctlParser::detect_output_format("{ }").value() == SmartctlParserFormat::Json); + REQUIRE(SmartctlParser::detect_output_format("{ }").value() == SmartctlOutputFormat::Json); - REQUIRE(SmartctlParser::detect_output_format(" \n { } ").value() == SmartctlParserFormat::Json); + REQUIRE(SmartctlParser::detect_output_format(" \n { } ").value() == SmartctlOutputFormat::Json); - REQUIRE(SmartctlParser::detect_output_format("smartctl").value() == SmartctlParserFormat::Text); + REQUIRE(SmartctlParser::detect_output_format("smartctl").value() == SmartctlOutputFormat::Text); REQUIRE(SmartctlParser::detect_output_format( R"(smartctl 7.2 2020-12-30 r5155 [x86_64-linux-5.3.18-lp152.66-default] (SUSE RPM) Copyright (C) 2002-20, Bruce Allen, Christian Franke, www.smartmontools.org -)").value() == SmartctlParserFormat::Text); +)").value() == SmartctlOutputFormat::Text); }