Improve error position detention at WRITE PERM (#357)

* Correct miscalculation of last block on tape
* Consider the final index on the partition as error position even if very small number is returned
* Never adjust the force_writeperm threshold for better debug
* Stop checking the I/F of the ltfs-backends repository

Author: Atsushi Abe

.github/workflows/build-centos7.yml

@@ -12,6 +12,6 @@ jobs:
         uses: actions/checkout@v1
       - name: Build LTFS
         id: build
-        uses: LinearTapeFileSystem/CentOS7-Build@v1.3
+        uses: LinearTapeFileSystem/CentOS7-Build@v1.4
         with:
           destination: '/tmp/ltfs'

.github/workflows/build-centos8.yml

@@ -12,6 +12,6 @@ jobs:
         uses: actions/checkout@v1
       - name: Build LTFS
         id: build
-        uses: LinearTapeFileSystem/CentOS8-Build@v1.5
+        uses: LinearTapeFileSystem/CentOS8-Build@v1.6
         with:
           destination: '/tmp/ltfs'

messages/iosched_unified/root.txt

@@ -62,7 +62,8 @@ root:table {
 		13022W:string { "Freeing a dentry priv with outstanding write requests. This is a bug." }
 		//unused 13023W:string { "Failed to copy a request: will stop writing file to the index partition." }
 		13024I:string { "Clean up extents and append index at index partition (%d)." }
-		13025I:string { "Get error position (%d, %d)." }
+		13025I:string { "Truncate extents larger than position (%d, %lld), block size = %ld." }
 		13026E:string { "Write perm handling error : %s (%d)." }
+		13027I:string { "Error position is larger than last index position: (%d, %lld), last index = %lld." }
 	}
 }

messages/libltfs/root.txt

@@ -429,7 +429,7 @@ root:table {
 		11332I:string { "Load successful." }
 		11333I:string { "A cartridge with write-perm error is detected on %s. Seek the newest index (IP: Gen = %llu, VCR = %llu) (DP: Gen = %llu, VCR = %llu) (VCR = %llu)." }
 		11334I:string { "Remove extent : %s (%llu, %llu)." }
-		11335D:string { "Get physical block position (%d - %d)." }
+		//unused 11335D:string { "Get physical block position (%d - %d)." }
 		11336I:string { "The attribute does not exist. Ignore the expected error." }
 		11337I:string { "Update index-dirty flag (%d) - %s (0x%p)." }
 		11338I:string { "Syncing index of %s %s." }
@@ -833,6 +833,7 @@ v
 		17289I:string { "Skip parsing the final index on IP." }
 		17290I:string { "Partitioning the medium with the destructive method." }
 		17291I:string { "Unpartitioning the medium with the destructive method." }
+		17292I:string { "Current position is (%llu, %llu), Error position is (%llu, %llu)." }
 
 		// For Debug 19999I:string { "%s %s %d." }
 

src/iosched/unified.c

@@ -2250,6 +2250,7 @@ int _unified_write_index_after_perm(int write_ret, struct unified_data *priv)
 {
 	int ret = 0;
 	struct tc_position err_pos;
+	uint64_t last_index_pos = UINT64_MAX;
 	unsigned long blocksize;
 
 	if (!IS_WRITE_PERM(-write_ret)) {
@@ -2263,14 +2264,27 @@ int _unified_write_index_after_perm(int write_ret, struct unified_data *priv)
 		ltfsmsg(LTFS_ERR, 13026E, "update MAM", ret);
 
 	blocksize = ltfs_get_blocksize(priv->vol);
-	ret = tape_get_physical_block_position(priv->vol->device, &err_pos);
+
+	ret = tape_get_first_untransfered_position(priv->vol->device, &err_pos);
 	if (ret < 0) {
 		ltfsmsg(LTFS_ERR, 13026E, "get error pos", ret);
 		return ret;
 	}
 
-	ltfsmsg(LTFS_INFO, 13025I, (int)err_pos.block, (int)blocksize);
+	/* Check the err_pos is larger than the last index position of the partition */
+	if (err_pos.partition == ltfs_part_id2num(priv->vol->label->partid_ip, priv->vol)) {
+		last_index_pos = priv->vol->ip_coh.set_id;
+	} else {
+		last_index_pos = priv->vol->dp_coh.set_id;
+	}
+
+	if (last_index_pos > err_pos.block) {
+		ltfsmsg(LTFS_INFO, 13027I, (int)err_pos.partition,
+				(unsigned long long)err_pos.block, (unsigned long long)last_index_pos);
+		err_pos.block = last_index_pos + 1;
+	}
 
+	ltfsmsg(LTFS_INFO, 13025I, (int)err_pos.partition, (unsigned long long)err_pos.block, blocksize);
 	ret = ltfs_fsraw_cleanup_extent(priv->vol->index->root, err_pos, blocksize, priv->vol);
 	if (ret < 0) {
 		ltfsmsg(LTFS_ERR, 13026E, "extent cleanup", ret);

src/libltfs/ltfs_fsops_raw.c

@@ -443,8 +443,9 @@ int ltfs_fsraw_add_extent(struct dentry *d, struct extent_info *ext, bool update
 int ltfs_fsraw_cleanup_extent(struct dentry *d, struct tc_position err_pos, unsigned long blocksize, struct ltfs_volume *vol)
 {
 	int ret = 0;
-	struct name_list *entry, *tmp;
+	struct name_list   *entry, *tmp;
 	struct extent_info *ext, *preventry;
+	struct tc_position extent_last = {0, 0, UINT32_MAX, false, false};
 
 	if (HASH_COUNT(d->child_list) != 0) {
 		HASH_ITER(hh, d->child_list, entry, tmp) {
@@ -453,7 +454,24 @@ int ltfs_fsraw_cleanup_extent(struct dentry *d, struct tc_position err_pos, unsi
 			}
 			else {
                 TAILQ_FOREACH_REVERSE_SAFE(ext, &entry->d->extentlist, extent_struct, list, preventry) {
-					if (err_pos.block <= (ext->start.block + ext->bytecount/blocksize)) {
+					if (ext->start.block && ext->bytecount) {
+						extent_last.partition = ltfs_part_id2num(ext->start.partition, vol);
+						/* Calculate the last block of this extent */
+						extent_last.block = ext->start.block + (ext->bytecount / blocksize);
+						if ( (ext->bytecount % blocksize) == 0 )
+							extent_last.block--;
+					} else {
+						extent_last.partition = UINT32_MAX;
+						extent_last.block     = 0;
+					}
+
+					/*
+					 * err_pos has the first block number that tape drive has it on buffer
+					 * but not transferred to the medium.
+					 * It means position (err_pos-1) is the last block on the medium.
+					 */
+					if ( extent_last.partition == err_pos.partition && err_pos.block <= extent_last.block ) {
+
 						ltfsmsg(LTFS_INFO, 11334I, entry->name, (unsigned long long)ext->start.block, (unsigned long long)ext->bytecount);
 
 						ret = ltfs_get_volume_lock(false, vol);

src/libltfs/tape.c

@@ -1122,30 +1122,32 @@ int tape_update_position(struct device_data *dev, struct tc_position *pos)
 	return 0;
 }
 
-int tape_get_physical_block_position(struct device_data *dev, struct tc_position *pos)
+int tape_get_first_untransfered_position(struct device_data *dev, struct tc_position *pos)
 {
 	int ret;
-	 unsigned int block;
 
 	CHECK_ARG_NULL(dev, -LTFS_NULL_ARG);
 	CHECK_ARG_NULL(pos, -LTFS_NULL_ARG);
 
+	/* Update current position, just in case. Because no penalty here */
 	ret = dev->backend->readpos(dev->backend_data, &dev->position);
 	if (ret < 0) {
 		ltfsmsg(LTFS_ERR, 17132E);
 		return ret;
 	}
 
-	ret = dev->backend->get_block_in_buffer(dev->backend_data, &block);
+	/* Capture first untransferred position */
+	ret = dev->backend->get_next_block_to_xfer(dev->backend_data, pos);
 	if (ret < 0) {
 		ltfsmsg(LTFS_ERR, 17132E);
 		return ret;
 	}
 
-	memcpy(pos, &dev->position, sizeof(struct tc_position));
-
-	ltfsmsg(LTFS_DEBUG, 11335D, (int)pos->block, block);
-	pos->block -= block;
+	ltfsmsg(LTFS_INFO, 17292I,
+			(unsigned long long)dev->position.partition,
+			(unsigned long long)dev->position.block,
+			(unsigned long long)pos->partition,
+			(unsigned long long)pos->block);
 
 	return 0;
 }

src/libltfs/tape.h

@@ -165,7 +165,7 @@ int tape_update_position(struct device_data *dev, struct tc_position *pos);
 int tape_seek(struct device_data *dev, struct tc_position *pos);
 int tape_seek_eod(struct device_data *dev, tape_partition_t partition);
 int tape_seek_append_position(struct device_data *dev, tape_partition_t prt, bool unlock_write);
-int tape_get_physical_block_position(struct device_data *dev, struct tc_position *pos);
+int tape_get_first_untransfered_position(struct device_data *dev, struct tc_position *pos);
 
 int tape_spacefm(struct device_data *dev, int count);
 ssize_t tape_read(struct device_data *dev, char *buf, size_t count, const bool unusual_size,

src/libltfs/tape_ops.h

@@ -915,12 +915,12 @@ struct tape_ops {
 	int   (*set_profiler)(void *device, char *work_dir, bool enable);
 
 	/**
-	 * Get block number stored in the drive buffer
+	 * Get first block number which is not transferred to the medium yet in the buffer
 	 * @param device A pointer to the tape device
-	 * @param block Number of blocks stored in the drive buffer
+	 * @param pos Position of the record that is not transferred yet in the buffer
 	 * @return 0 on success or a negative value on error
 	 */
-	int   (*get_block_in_buffer)(void *device, unsigned int *block);
+	int   (*get_next_block_to_xfer)(void *device, struct tc_position *pos);
 
 	/**
 	 * Check if the generation of tape drive and the current loaded cartridge is read-only combination

src/tape_drivers/freebsd/cam/cam_tc.c

@@ -1193,10 +1193,10 @@ int camtape_unload(void *device, struct tc_position *pos)
 }
 
 /*
- * Get the number of blocks in the buffer on the tape drive after a write.  Eventually it would
+ * Get the first block position that is not transferred to the medium. Eventually it would
  * be nice to include this in status information returned from the sa(4) driver.
  */
-static int camtape_get_block_in_buffer(void *device, uint32_t *block)
+static int camtape_get_next_block_to_xfer(void *device, struct tc_position *pos)
 {
 	int rc;
 	union ccb *ccb = NULL;
@@ -1240,9 +1240,10 @@ static int camtape_get_block_in_buffer(void *device, uint32_t *block)
 	if (rc != DEVICE_GOOD)
 		camtape_process_errors(softc, rc, msg, "READPOS", true);
 	else {
-		*block = scsi_3btoul(ext_data.num_objects);
-		ltfsmsg(LTFS_DEBUG, 30398D, "blocks-in-buffer",
-				(unsigned long long) *block, 0, 0, softc->drive_serial);
+		pos->partition = ext_data.partition;
+		pos->block = scsi_8btou64(ext_data.last_object)
+		ltfsmsg(LTFS_DEBUG, 30398D, "next-block-to-xfer",
+				(unsigned long long) pos->block, 0, 0, softc->drive_serial);
 	}
 
 bailout:
@@ -4170,7 +4171,7 @@ struct tape_ops camtape_drive_handler = {
 	.get_serialnumber		= camtape_get_serialnumber,
 	.get_info               = camtape_get_info,
 	.set_profiler			= camtape_set_profiler,
-	.get_block_in_buffer	= camtape_get_block_in_buffer,
+	.get_next_block_to_xfer = camtape_get_next_block_to_xfer,
 	.is_readonly			= camtape_is_readonly
 };