From 8c867a69d4a93cf81b89a612764eb0be902b7407 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 29 Dec 2022 23:38:39 +0100 Subject: [PATCH] Recover better errors from ext4. --- cscript | 2 +- src/lib/copy_to_drive_job.cc | 2 +- src/lib/disk_writer_messages.cc | 5 ++-- src/lib/disk_writer_messages.h | 17 +++++++++---- src/lib/exceptions.cc | 7 +++--- src/lib/exceptions.h | 13 +++++++--- src/lib/ext.cc | 40 +++++++++++++++--------------- src/tools/dcpomatic_disk_writer.cc | 16 ++++++------ 8 files changed, 58 insertions(+), 44 deletions(-) diff --git a/cscript b/cscript index c25d82cab..b7163330f 100644 --- a/cscript +++ b/cscript @@ -467,7 +467,7 @@ def dependencies(target, options): # the appropriate place later deps.append(('openssl', '7f29dd5')) if can_build_disk(target): - deps.append(('lwext4', '3dc752da8948564456ce2d721c266b4e1a50e7a1')) + deps.append(('lwext4', 'b98f55b806f9dfe9e9374faceed99b689c29f28e')) deps.append(('ffcmp', 'da96af56f3ddf074f2044a0cd6e50c95184fd169')) return deps diff --git a/src/lib/copy_to_drive_job.cc b/src/lib/copy_to_drive_job.cc index 9d3a6721a..7d208e0ec 100644 --- a/src/lib/copy_to_drive_job.cc +++ b/src/lib/copy_to_drive_job.cc @@ -109,7 +109,7 @@ CopyToDriveJob::run () case DiskWriterBackEndResponse::Type::PONG: break; case DiskWriterBackEndResponse::Type::ERROR: - throw CopyError(response->error_message(), response->error_number()); + throw CopyError(response->error_message(), response->ext4_error_number(), response->platform_error_number()); case DiskWriterBackEndResponse::Type::FORMAT_PROGRESS: if (state == SETUP) { sub (_("Formatting drive")); diff --git a/src/lib/disk_writer_messages.cc b/src/lib/disk_writer_messages.cc index a862f2fc6..7bccdd9fc 100644 --- a/src/lib/disk_writer_messages.cc +++ b/src/lib/disk_writer_messages.cc @@ -41,7 +41,8 @@ DiskWriterBackEndResponse::read_from_nanomsg(Nanomsg& nanomsg, int timeout) } else if (*s == DISK_WRITER_ERROR) { auto const m = nanomsg.receive(500); auto const n = nanomsg.receive(500); - return DiskWriterBackEndResponse::error(m.get_value_or(""), dcp::raw_convert(n.get_value_or("0"))); + auto const p = nanomsg.receive(500); + return DiskWriterBackEndResponse::error(m.get_value_or(""), dcp::raw_convert(n.get_value_or("0")), dcp::raw_convert(p.get_value_or("0"))); } else if (*s == DISK_WRITER_PONG) { return DiskWriterBackEndResponse::pong(); } else if (*s == DISK_WRITER_FORMAT_PROGRESS) { @@ -73,7 +74,7 @@ DiskWriterBackEndResponse::write_to_nanomsg(Nanomsg& nanomsg, int timeout) const message = String::compose("%1\n", DISK_WRITER_OK); break; case Type::ERROR: - message = String::compose("%1\n%2\n%3\n", DISK_WRITER_ERROR, _error_message, _error_number); + message = String::compose("%1\n%2\n%3\n%4\n", DISK_WRITER_ERROR, _error_message, _ext4_error_number, _platform_error_number); break; case Type::PONG: message = String::compose("%1\n", DISK_WRITER_PONG); diff --git a/src/lib/disk_writer_messages.h b/src/lib/disk_writer_messages.h index 8bd1837a8..2fa225d85 100644 --- a/src/lib/disk_writer_messages.h +++ b/src/lib/disk_writer_messages.h @@ -58,6 +58,7 @@ class Nanomsg; #define DISK_WRITER_ERROR "E" // Error message // Error number +// Additional error number (a platform-specific error from lwext4) // the drive is being formatted, 40% done #define DISK_WRITER_FORMAT_PROGRESS "F" @@ -106,10 +107,11 @@ public: return DiskWriterBackEndResponse(Type::OK); } - static DiskWriterBackEndResponse error(std::string message, int number) { + static DiskWriterBackEndResponse error(std::string message, int ext4_number, int platform_number) { auto r = DiskWriterBackEndResponse(Type::ERROR); r._error_message = message; - r._error_number = number; + r._ext4_error_number = ext4_number; + r._platform_error_number = platform_number; return r; } @@ -147,8 +149,12 @@ public: return _error_message; } - int error_number() const { - return _error_number; + int ext4_error_number() const { + return _ext4_error_number; + } + + int platform_error_number() const { + return _platform_error_number; } float progress() const { @@ -162,7 +168,8 @@ private: Type _type; std::string _error_message; - int _error_number = 0; + int _ext4_error_number = 0; + int _platform_error_number = 0; float _progress = 0; }; diff --git a/src/lib/exceptions.cc b/src/lib/exceptions.cc index 66db9fda7..a51842b80 100644 --- a/src/lib/exceptions.cc +++ b/src/lib/exceptions.cc @@ -148,10 +148,11 @@ GLError::GLError (char const* message) } -CopyError::CopyError (string m, optional n) - : runtime_error (String::compose("%1%2", m, n ? String::compose(" (%1)", *n) : "")) +CopyError::CopyError(string m, optional ext4, optional platform) + : runtime_error(String::compose("%1%2%3", m, ext4 ? String::compose(" (%1)", *ext4) : "", platform ? String::compose(" (%1)", *platform) : "")) , _message (m) - , _number (n) + , _ext4_number(ext4) + , _platform_number(platform) { } diff --git a/src/lib/exceptions.h b/src/lib/exceptions.h index 618a03f43..ed002d0a9 100644 --- a/src/lib/exceptions.h +++ b/src/lib/exceptions.h @@ -407,20 +407,25 @@ public: class CopyError : public std::runtime_error { public: - CopyError (std::string s, boost::optional n = boost::optional()); + CopyError (std::string s, boost::optional ext4_error = boost::optional(), boost::optional platform_error = boost::optional()); virtual ~CopyError () throw () {} std::string message () const { return _message; } - boost::optional number () const { - return _number; + boost::optional ext4_number() const { + return _ext4_number; + } + + boost::optional platform_number() const { + return _platform_number; } private: std::string _message; - boost::optional _number; + boost::optional _ext4_number; + boost::optional _platform_number; }; diff --git a/src/lib/ext.cc b/src/lib/ext.cc index 29f44d90e..2ef3df6bb 100644 --- a/src/lib/ext.cc +++ b/src/lib/ext.cc @@ -113,13 +113,13 @@ write (boost::filesystem::path from, boost::filesystem::path to, uint64_t& total ext4_file out; int r = ext4_fopen(&out, to.generic_string().c_str(), "wb"); if (r != EOK) { - throw CopyError (String::compose("Failed to open file %1", to.generic_string()), r); + throw CopyError(String::compose("Failed to open file %1", to.generic_string()), r, ext4_blockdev_errno); } dcp::File in(from, "rb"); if (!in) { ext4_fclose (&out); - throw CopyError (String::compose("Failed to open file %1", from.string()), 0); + throw CopyError(String::compose("Failed to open file %1", from.string()), 0); } std::vector buffer(block_size); @@ -133,7 +133,7 @@ write (boost::filesystem::path from, boost::filesystem::path to, uint64_t& total size_t read = in.read(buffer.data(), 1, this_time); if (read != this_time) { ext4_fclose (&out); - throw CopyError (String::compose("Short read; expected %1 but read %2", this_time, read), 0); + throw CopyError(String::compose("Short read; expected %1 but read %2", this_time, read), 0, ext4_blockdev_errno); } digester.add (buffer.data(), this_time); @@ -142,11 +142,11 @@ write (boost::filesystem::path from, boost::filesystem::path to, uint64_t& total r = ext4_fwrite (&out, buffer.data(), this_time, &written); if (r != EOK) { ext4_fclose (&out); - throw CopyError ("Write failed", r); + throw CopyError("Write failed", r, ext4_blockdev_errno); } if (written != this_time) { ext4_fclose (&out); - throw CopyError (String::compose("Short write; expected %1 but wrote %2", this_time, written), 0); + throw CopyError(String::compose("Short write; expected %1 but wrote %2", this_time, written), 0, ext4_blockdev_errno); } remaining -= this_time; total_remaining -= this_time; @@ -237,7 +237,7 @@ copy (boost::filesystem::path from, boost::filesystem::path to, uint64_t& total_ if (is_directory(from)) { int r = ext4_dir_mk (cr.generic_string().c_str()); if (r != EOK) { - throw CopyError (String::compose("Failed to create directory %1", cr.generic_string()), r); + throw CopyError(String::compose("Failed to create directory %1", cr.generic_string()), r, ext4_blockdev_errno); } set_timestamps_to_now (cr); @@ -317,7 +317,7 @@ try #endif if (!bd) { - throw CopyError ("Failed to open drive", 0); + throw CopyError("Failed to open drive", 0, ext4_blockdev_errno); } LOG_DISK_NC ("Opened drive"); @@ -330,14 +330,14 @@ try /* XXX: not sure if disk_id matters */ int r = ext4_mbr_write (bd, &parts, 0); if (r) { - throw CopyError ("Failed to write MBR", r); + throw CopyError("Failed to write MBR", r, ext4_blockdev_errno); } LOG_DISK_NC ("Wrote MBR"); struct ext4_mbr_bdevs bdevs; r = ext4_mbr_scan (bd, &bdevs); if (r != EOK) { - throw CopyError ("Failed to read MBR", r); + throw CopyError("Failed to read MBR", r, ext4_blockdev_errno); } #ifdef DCPOMATIC_LINUX @@ -369,25 +369,25 @@ try #endif if (!bd) { - throw CopyError ("Failed to open partition", 0); + throw CopyError("Failed to open partition", 0, ext4_blockdev_errno); } LOG_DISK_NC ("Opened partition"); r = ext4_mkfs(&fs, bd, &info, F_SET_EXT2, format_progress, nanomsg); if (r != EOK) { - throw CopyError ("Failed to make filesystem", r); + throw CopyError("Failed to make filesystem", r, ext4_blockdev_errno); } LOG_DISK_NC ("Made filesystem"); r = ext4_device_register(bd, "ext4_fs"); if (r != EOK) { - throw CopyError ("Failed to register device", r); + throw CopyError("Failed to register device", r, ext4_blockdev_errno); } LOG_DISK_NC ("Registered device"); r = ext4_mount("ext4_fs", "/mp/", false); if (r != EOK) { - throw CopyError ("Failed to mount device", r); + throw CopyError("Failed to mount device", r, ext4_blockdev_errno); } LOG_DISK_NC ("Mounted device"); @@ -403,11 +403,11 @@ try /* Unmount and re-mount to make sure the write has finished */ r = ext4_umount("/mp/"); if (r != EOK) { - throw CopyError ("Failed to unmount device", r); + throw CopyError("Failed to unmount device", r, ext4_blockdev_errno); } r = ext4_mount("ext4_fs", "/mp/", false); if (r != EOK) { - throw CopyError ("Failed to mount device", r); + throw CopyError("Failed to mount device", r, ext4_blockdev_errno); } LOG_DISK_NC ("Re-mounted device"); @@ -415,7 +415,7 @@ try r = ext4_umount("/mp/"); if (r != EOK) { - throw CopyError ("Failed to unmount device", r); + throw CopyError("Failed to unmount device", r, ext4_blockdev_errno); } ext4_device_unregister("ext4_fs"); @@ -425,19 +425,19 @@ try disk_write_finished (); } catch (CopyError& e) { - LOG_DISK("CopyError (from write): %1 %2", e.message(), e.number().get_value_or(0)); + LOG_DISK("CopyError (from write): %1 %2 %3", e.message(), e.ext4_number().get_value_or(0), e.platform_number().get_value_or(0)); if (nanomsg) { - DiskWriterBackEndResponse::error(e.message(), e.number().get_value_or(0)).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error(e.message(), e.ext4_number().get_value_or(0), e.platform_number().get_value_or(0)).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); } } catch (VerifyError& e) { LOG_DISK("VerifyError (from write): %1 %2", e.message(), e.number()); if (nanomsg) { - DiskWriterBackEndResponse::error(e.message(), e.number()).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error(e.message(), e.number(), 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); } } catch (exception& e) { LOG_DISK("Exception (from write): %1", e.what()); if (nanomsg) { - DiskWriterBackEndResponse::error(e.what(), 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error(e.what(), 0, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); } } diff --git a/src/tools/dcpomatic_disk_writer.cc b/src/tools/dcpomatic_disk_writer.cc index 5a10d75cb..3535ffa1c 100644 --- a/src/tools/dcpomatic_disk_writer.cc +++ b/src/tools/dcpomatic_disk_writer.cc @@ -181,7 +181,7 @@ try if (success) { sent_reply = DiskWriterBackEndResponse::ok().write_to_nanomsg(*nanomsg, LONG_TIMEOUT); } else { - sent_reply = DiskWriterBackEndResponse::error("Could not unmount drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + sent_reply = DiskWriterBackEndResponse::error("Could not unmount drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); } if (!sent_reply) { LOG_DISK_NC("CommunicationFailedError in unmount_finished"); @@ -189,7 +189,7 @@ try } }, []() { - if (!DiskWriterBackEndResponse::error("Could not get permission to unmount drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT)) { + if (!DiskWriterBackEndResponse::error("Could not get permission to unmount drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT)) { LOG_DISK_NC("CommunicationFailedError in unmount_finished"); throw CommunicationFailedError (); } @@ -221,21 +221,21 @@ try #ifdef DCPOMATIC_OSX if (!starts_with(device, "/dev/disk")) { LOG_DISK ("Will not write to %1", device); - DiskWriterBackEndResponse::error("Refusing to write to this drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error("Refusing to write to this drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); return true; } #endif #ifdef DCPOMATIC_LINUX if (!starts_with(device, "/dev/sd") && !starts_with(device, "/dev/hd")) { LOG_DISK ("Will not write to %1", device); - DiskWriterBackEndResponse::error("Refusing to write to this drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error("Refusing to write to this drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); return true; } #endif #ifdef DCPOMATIC_WINDOWS if (!starts_with(device, "\\\\.\\PHYSICALDRIVE")) { LOG_DISK ("Will not write to %1", device); - DiskWriterBackEndResponse::error("Refusing to write to this drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error("Refusing to write to this drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); return true; } #endif @@ -251,12 +251,12 @@ try if (!on_drive_list) { LOG_DISK ("Will not write to %1 as it's not recognised as a drive", device); - DiskWriterBackEndResponse::error("Refusing to write to this drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error("Refusing to write to this drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); return true; } if (mounted) { LOG_DISK ("Will not write to %1 as it's mounted", device); - DiskWriterBackEndResponse::error("Refusing to write to this drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error("Refusing to write to this drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); return true; } @@ -286,7 +286,7 @@ try }, []() { if (nanomsg) { - DiskWriterBackEndResponse::error("Could not obtain authorization to write to the drive", 1).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); + DiskWriterBackEndResponse::error("Could not obtain authorization to write to the drive", 1, 0).write_to_nanomsg(*nanomsg, LONG_TIMEOUT); } }); } -- 2.30.2