From 023c62798ca4e59f2367333f2587b092617e775f Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Mon, 16 Nov 2020 17:28:29 +0000 Subject: [PATCH] libsnapshot: snapuserd: Handle flush request Handle flush operations by dm-snap post merge and the ABI changes from dm-user. This is now in sync with the latest dm-user patch (patch 25). In case of any failures observed in daemon in the IO path, return error code back to dm-user which will eventually fail the IO. Bug: 168311203 Test: vts_libsnapshot_test, cow_snapuserd_test Signed-off-by: Akilesh Kailash Change-Id: I4af63845f8c3e1c445f6c55374ea58b6f3454795 --- .../include/libsnapshot/snapuserd.h | 20 +- .../include/libsnapshot/snapuserd_kernel.h | 7 + fs_mgr/libsnapshot/snapuserd.cpp | 199 ++++++++++-------- fs_mgr/libsnapshot/snapuserd_server.cpp | 4 +- 4 files changed, 127 insertions(+), 103 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h index aa7dc405a..24b44fae7 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h @@ -65,27 +65,27 @@ class Snapuserd final { const std::string& backing_device); bool InitBackingAndControlDevice(); bool InitCowDevice(); - int Run(); + bool Run(); const std::string& GetControlDevicePath() { return control_device_; } const std::string& GetMiscName() { return misc_name_; } uint64_t GetNumSectors() { return num_sectors_; } bool IsAttached() const { return ctrl_fd_ >= 0; } private: - int ReadDmUserHeader(); + bool ReadDmUserHeader(); bool ReadDmUserPayload(void* buffer, size_t size); - int WriteDmUserPayload(size_t size); - int ConstructKernelCowHeader(); + bool WriteDmUserPayload(size_t size); + void ConstructKernelCowHeader(); bool ReadMetadata(); - int ZerofillDiskExceptions(size_t read_size); - int ReadDiskExceptions(chunk_t chunk, size_t size); - int ReadData(chunk_t chunk, size_t size); + bool ZerofillDiskExceptions(size_t read_size); + bool ReadDiskExceptions(chunk_t chunk, size_t size); + bool ReadData(chunk_t chunk, size_t size); bool IsChunkIdMetadata(chunk_t chunk); chunk_t GetNextAllocatableChunkId(chunk_t chunk); - int ProcessReplaceOp(const CowOperation* cow_op); - int ProcessCopyOp(const CowOperation* cow_op); - int ProcessZeroOp(); + bool ProcessReplaceOp(const CowOperation* cow_op); + bool ProcessCopyOp(const CowOperation* cow_op); + bool ProcessZeroOp(); loff_t GetMergeStartOffset(void* merged_buffer, void* unmerged_buffer, int* unmerged_exceptions); diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h index 1037c126b..e8dbe6e95 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h @@ -17,6 +17,13 @@ namespace android { namespace snapshot { +#define DM_USER_REQ_MAP_READ 0 +#define DM_USER_REQ_MAP_WRITE 1 + +#define DM_USER_RESP_SUCCESS 0 +#define DM_USER_RESP_ERROR 1 +#define DM_USER_RESP_UNSUPPORTED 2 + // Kernel COW header fields static constexpr uint32_t SNAP_MAGIC = 0x70416e53; diff --git a/fs_mgr/libsnapshot/snapuserd.cpp b/fs_mgr/libsnapshot/snapuserd.cpp index 80b573415..954699c53 100644 --- a/fs_mgr/libsnapshot/snapuserd.cpp +++ b/fs_mgr/libsnapshot/snapuserd.cpp @@ -28,9 +28,6 @@ using namespace android; using namespace android::dm; using android::base::unique_fd; -#define DM_USER_MAP_READ 0 -#define DM_USER_MAP_WRITE 1 - static constexpr size_t PAYLOAD_SIZE = (1UL << 16); static_assert(PAYLOAD_SIZE >= BLOCK_SIZE); @@ -78,7 +75,7 @@ Snapuserd::Snapuserd(const std::string& misc_name, const std::string& cow_device // This header will be in sector 0. The IO // request will always be 4k. After constructing // the header, zero out the remaining block. -int Snapuserd::ConstructKernelCowHeader() { +void Snapuserd::ConstructKernelCowHeader() { void* buffer = bufsink_.GetPayloadBuffer(BLOCK_SIZE); CHECK(buffer != nullptr); @@ -90,25 +87,23 @@ int Snapuserd::ConstructKernelCowHeader() { dh->valid = SNAPSHOT_VALID; dh->version = SNAPSHOT_DISK_VERSION; dh->chunk_size = CHUNK_SIZE; - - return BLOCK_SIZE; } // Start the replace operation. This will read the // internal COW format and if the block is compressed, // it will be de-compressed. -int Snapuserd::ProcessReplaceOp(const CowOperation* cow_op) { +bool Snapuserd::ProcessReplaceOp(const CowOperation* cow_op) { if (!reader_->ReadData(*cow_op, &bufsink_)) { LOG(ERROR) << "ReadData failed for chunk: " << cow_op->new_block; - return -EIO; + return false; } - return BLOCK_SIZE; + return true; } // Start the copy operation. This will read the backing // block device which is represented by cow_op->source. -int Snapuserd::ProcessCopyOp(const CowOperation* cow_op) { +bool Snapuserd::ProcessCopyOp(const CowOperation* cow_op) { void* buffer = bufsink_.GetPayloadBuffer(BLOCK_SIZE); CHECK(buffer != nullptr); @@ -117,19 +112,19 @@ int Snapuserd::ProcessCopyOp(const CowOperation* cow_op) { if (!android::base::ReadFullyAtOffset(backing_store_fd_, buffer, BLOCK_SIZE, cow_op->source * BLOCK_SIZE)) { LOG(ERROR) << "Copy-op failed. Read from backing store at: " << cow_op->source; - return -1; + return false; } - return BLOCK_SIZE; + return true; } -int Snapuserd::ProcessZeroOp() { +bool Snapuserd::ProcessZeroOp() { // Zero out the entire block void* buffer = bufsink_.GetPayloadBuffer(BLOCK_SIZE); CHECK(buffer != nullptr); memset(buffer, 0, BLOCK_SIZE); - return BLOCK_SIZE; + return true; } /* @@ -154,11 +149,9 @@ int Snapuserd::ProcessZeroOp() { * 3: Zero operation * */ -int Snapuserd::ReadData(chunk_t chunk, size_t size) { - int ret = 0; - +bool Snapuserd::ReadData(chunk_t chunk, size_t size) { size_t read_size = size; - + bool ret = true; chunk_t chunk_key = chunk; uint32_t stride; lldiv_t divresult; @@ -169,41 +162,39 @@ int Snapuserd::ReadData(chunk_t chunk, size_t size) { while (read_size > 0) { const CowOperation* cow_op = chunk_map_[chunk_key]; CHECK(cow_op != nullptr); - int result; switch (cow_op->type) { case kCowReplaceOp: { - result = ProcessReplaceOp(cow_op); + ret = ProcessReplaceOp(cow_op); break; } case kCowZeroOp: { - result = ProcessZeroOp(); + ret = ProcessZeroOp(); break; } case kCowCopyOp: { - result = ProcessCopyOp(cow_op); + ret = ProcessCopyOp(cow_op); break; } default: { LOG(ERROR) << "Unknown operation-type found: " << cow_op->type; - ret = -EIO; - goto done; + ret = false; + break; } } - if (result < 0) { - ret = result; - goto done; + if (!ret) { + LOG(ERROR) << "ReadData failed for operation: " << cow_op->type; + return false; } // Update the buffer offset bufsink_.UpdateBufferOffset(BLOCK_SIZE); read_size -= BLOCK_SIZE; - ret += BLOCK_SIZE; // Start iterating the chunk incrementally; Since while // constructing the metadata, we know that the chunk IDs @@ -231,8 +222,6 @@ int Snapuserd::ReadData(chunk_t chunk, size_t size) { } } -done: - // Reset the buffer offset bufsink_.ResetBufferOffset(); return ret; @@ -249,16 +238,18 @@ done: * When dm-snap starts parsing the buffer, it will stop * reading metadata page once the buffer content is zero. */ -int Snapuserd::ZerofillDiskExceptions(size_t read_size) { +bool Snapuserd::ZerofillDiskExceptions(size_t read_size) { size_t size = exceptions_per_area_ * sizeof(struct disk_exception); - if (read_size > size) return -EINVAL; + if (read_size > size) { + return false; + } void* buffer = bufsink_.GetPayloadBuffer(size); CHECK(buffer != nullptr); memset(buffer, 0, size); - return size; + return true; } /* @@ -274,7 +265,7 @@ int Snapuserd::ZerofillDiskExceptions(size_t read_size) { * Convert the chunk ID to index into the vector which gives us * the metadata page. */ -int Snapuserd::ReadDiskExceptions(chunk_t chunk, size_t read_size) { +bool Snapuserd::ReadDiskExceptions(chunk_t chunk, size_t read_size) { uint32_t stride = exceptions_per_area_ + 1; size_t size; @@ -284,17 +275,19 @@ int Snapuserd::ReadDiskExceptions(chunk_t chunk, size_t read_size) { if (divresult.quot < vec_.size()) { size = exceptions_per_area_ * sizeof(struct disk_exception); - if (read_size > size) return -EINVAL; + if (read_size > size) { + return false; + } void* buffer = bufsink_.GetPayloadBuffer(size); CHECK(buffer != nullptr); memcpy(buffer, vec_[divresult.quot].get(), size); } else { - size = ZerofillDiskExceptions(read_size); + return ZerofillDiskExceptions(read_size); } - return size; + return true; } loff_t Snapuserd::GetMergeStartOffset(void* merged_buffer, void* unmerged_buffer, @@ -648,27 +641,24 @@ void MyLogger(android::base::LogId, android::base::LogSeverity severity, const c // Read Header from dm-user misc device. This gives // us the sector number for which IO is issued by dm-snapshot device -int Snapuserd::ReadDmUserHeader() { - int ret; - - ret = read(ctrl_fd_, bufsink_.GetBufPtr(), sizeof(struct dm_user_header)); - if (ret < 0) { - PLOG(ERROR) << "Control-read failed with: " << ret; - return ret; +bool Snapuserd::ReadDmUserHeader() { + if (!android::base::ReadFully(ctrl_fd_, bufsink_.GetBufPtr(), sizeof(struct dm_user_header))) { + PLOG(ERROR) << "ReadDmUserHeader failed"; + return false; } - return sizeof(struct dm_user_header); + return true; } // Send the payload/data back to dm-user misc device. -int Snapuserd::WriteDmUserPayload(size_t size) { +bool Snapuserd::WriteDmUserPayload(size_t size) { if (!android::base::WriteFully(ctrl_fd_, bufsink_.GetBufPtr(), sizeof(struct dm_user_header) + size)) { PLOG(ERROR) << "Write to dm-user failed"; - return -1; + return false; } - return sizeof(struct dm_user_header) + size; + return true; } bool Snapuserd::ReadDmUserPayload(void* buffer, size_t size) { @@ -713,17 +703,15 @@ bool Snapuserd::InitBackingAndControlDevice() { return true; } -int Snapuserd::Run() { - int ret = 0; - +bool Snapuserd::Run() { struct dm_user_header* header = bufsink_.GetHeaderPtr(); bufsink_.Clear(); - ret = ReadDmUserHeader(); - if (ret < 0) return ret; - - LOG(DEBUG) << "dm-user returned " << ret << " bytes"; + if (!ReadDmUserHeader()) { + LOG(ERROR) << "ReadDmUserHeader failed"; + return false; + } LOG(DEBUG) << "msg->seq: " << std::hex << header->seq; LOG(DEBUG) << "msg->type: " << std::hex << header->type; @@ -732,12 +720,12 @@ int Snapuserd::Run() { LOG(DEBUG) << "msg->len: " << std::hex << header->len; switch (header->type) { - case DM_USER_MAP_READ: { + case DM_USER_REQ_MAP_READ: { size_t remaining_size = header->len; loff_t offset = 0; - ret = 0; do { size_t read_size = std::min(PAYLOAD_SIZE, remaining_size); + header->type = DM_USER_RESP_SUCCESS; // Request to sector 0 is always for kernel // representation of COW header. This IO should be only @@ -747,8 +735,8 @@ int Snapuserd::Run() { if (header->sector == 0) { CHECK(metadata_read_done_ == true); CHECK(read_size == BLOCK_SIZE); - ret = ConstructKernelCowHeader(); - if (ret < 0) return ret; + ConstructKernelCowHeader(); + LOG(DEBUG) << "Kernel header constructed"; } else { // Convert the sector number to a chunk ID. // @@ -758,71 +746,100 @@ int Snapuserd::Run() { chunk_t chunk = SectorToChunk(header->sector); if (chunk_map_.find(chunk) == chunk_map_.end()) { - ret = ReadDiskExceptions(chunk, read_size); - if (ret < 0) { - LOG(ERROR) << "ReadDiskExceptions failed"; - return ret; + if (!ReadDiskExceptions(chunk, read_size)) { + LOG(ERROR) << "ReadDiskExceptions failed for chunk id: " << chunk + << "Sector: " << header->sector; + header->type = DM_USER_RESP_ERROR; + } else { + LOG(DEBUG) << "ReadDiskExceptions success for chunk id: " << chunk + << "Sector: " << header->sector; } } else { chunk_t num_chunks_read = (offset >> BLOCK_SHIFT); - ret = ReadData(chunk + num_chunks_read, read_size); - if (ret < 0) { - LOG(ERROR) << "ReadData failed"; - // TODO: Bug 168259959: All the error paths from this function - // should send error code to dm-user thereby IO - // terminates with an error from dm-user. Returning - // here without sending error code will block the - // IO. - return ret; + if (!ReadData(chunk + num_chunks_read, read_size)) { + LOG(ERROR) << "ReadData failed for chunk id: " << chunk + << "Sector: " << header->sector; + header->type = DM_USER_RESP_ERROR; + } else { + LOG(DEBUG) << "ReadData success for chunk id: " << chunk + << "Sector: " << header->sector; } } } - ssize_t written = WriteDmUserPayload(ret); - if (written < 0) return written; - - remaining_size -= ret; - offset += ret; - if (remaining_size) { - LOG(DEBUG) << "Write done ret: " << ret - << " remaining size: " << remaining_size; + // Daemon will not be terminated if there is any error. We will + // just send the error back to dm-user. + if (!WriteDmUserPayload(read_size)) { + return false; } + + remaining_size -= read_size; + offset += read_size; } while (remaining_size); break; } - case DM_USER_MAP_WRITE: { + case DM_USER_REQ_MAP_WRITE: { + // device mapper has the capability to allow + // targets to flush the cache when writes are completed. This + // is controlled by each target by a flag "flush_supported". + // This flag is set by dm-user. When flush is supported, + // a number of zero-length bio's will be submitted to + // the target for the purpose of flushing cache. It is the + // responsibility of the target driver - which is dm-user in this + // case, to remap these bio's to the underlying device. Since, + // there is no underlying device for dm-user, this zero length + // bio's gets routed to daemon. + // + // Flush operations are generated post merge by dm-snap by having + // REQ_PREFLUSH flag set. Snapuser daemon doesn't have anything + // to flush per se; hence, just respond back with a success message. + if (header->sector == 0) { + CHECK(header->len == 0); + header->type = DM_USER_RESP_SUCCESS; + if (!WriteDmUserPayload(0)) { + return false; + } + break; + } + size_t remaining_size = header->len; size_t read_size = std::min(PAYLOAD_SIZE, remaining_size); CHECK(read_size == BLOCK_SIZE); + CHECK(header->sector > 0); chunk_t chunk = SectorToChunk(header->sector); CHECK(chunk_map_.find(chunk) == chunk_map_.end()); void* buffer = bufsink_.GetPayloadBuffer(read_size); CHECK(buffer != nullptr); + header->type = DM_USER_RESP_SUCCESS; if (!ReadDmUserPayload(buffer, read_size)) { - return 1; + LOG(ERROR) << "ReadDmUserPayload failed for chunk id: " << chunk + << "Sector: " << header->sector; + header->type = DM_USER_RESP_ERROR; } - if (!ProcessMergeComplete(chunk, buffer)) { - LOG(ERROR) << "ProcessMergeComplete failed..."; - return 1; + if (header->type == DM_USER_RESP_SUCCESS && !ProcessMergeComplete(chunk, buffer)) { + LOG(ERROR) << "ProcessMergeComplete failed for chunk id: " << chunk + << "Sector: " << header->sector; + header->type = DM_USER_RESP_ERROR; + } else { + LOG(DEBUG) << "ProcessMergeComplete success for chunk id: " << chunk + << "Sector: " << header->sector; } - // Write the header only. - ssize_t written = WriteDmUserPayload(0); - if (written < 0) return written; + if (!WriteDmUserPayload(0)) { + return false; + } break; } } - LOG(DEBUG) << "read() finished, next message"; - - return 0; + return true; } } // namespace snapshot diff --git a/fs_mgr/libsnapshot/snapuserd_server.cpp b/fs_mgr/libsnapshot/snapuserd_server.cpp index 9d065b6b1..9d57ab00b 100644 --- a/fs_mgr/libsnapshot/snapuserd_server.cpp +++ b/fs_mgr/libsnapshot/snapuserd_server.cpp @@ -226,8 +226,8 @@ void SnapuserdServer::RunThread(std::shared_ptr handler) { LOG(INFO) << "Entering thread for handler: " << handler->GetMiscName(); while (!StopRequested()) { - if (handler->snapuserd()->Run() < 0) { - LOG(INFO) << "Snapuserd: Thread terminating as control device is de-registered"; + if (!handler->snapuserd()->Run()) { + LOG(INFO) << "Snapuserd: Thread terminating"; break; } }