Fixed a bug that could lead to the wrong last_modified time in an inode;

bfs_close() updated the last_modified time & index, but it didn't write
the inode back to disk.
Furthermore, bfs_write() should have already updated the last_modified
time, not bfs_close.


git-svn-id: file:///srv/svn/repos/haiku/trunk/current@725 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/file_systems/bfs/Index.cpp

@@ -284,7 +284,7 @@ Index::RemoveSize(Transaction *transaction, Inode *inode)
 
 
 status_t
-Index::UpdateSize(Transaction *transaction,Inode *inode)
+Index::UpdateSize(Transaction *transaction, Inode *inode)
 {
 	off_t oldSize = inode->OldSize();
 	off_t newSize = inode->Size();
@@ -301,7 +301,7 @@ Index::UpdateSize(Transaction *transaction,Inode *inode)
 status_t 
 Index::InsertLastModified(Transaction *transaction, Inode *inode)
 {
-	off_t modified = inode->Node()->last_modified_time;
+	off_t modified = inode->LastModified();
 	return Update(transaction,"last_modified",B_INT64_TYPE,NULL,0,(uint8 *)&modified,sizeof(int64),inode);
 }
 

src/add-ons/kernel/file_systems/bfs/Inode.cpp

@@ -90,7 +90,7 @@ Inode::Inode(Volume *volume,vnode_id id,bool empty,uint8 reenter)
 
 	// these two will help to maintain the indices
 	fOldSize = Size();
-	fOldLastModified = Node()->last_modified_time;
+	fOldLastModified = LastModified();
 }
 
 
@@ -884,6 +884,10 @@ Inode::WriteAt(Transaction *transaction,off_t pos,const uint8 *buffer,size_t *_l
 {
 	// call the right WriteAt() method, depending on the inode flags
 
+	// update the last modification time in memory, it will be written
+	// back to the inode, and the index when the file is closed
+	Node()->last_modified_time = (bigtime_t)time(NULL) << INODE_TIME_SHIFT;
+
 	if (Flags() & INODE_NO_CACHE)
 		return ((Stream<Access::Uncached> *)this)->WriteAt(transaction, pos, buffer, _length);
 
@@ -1320,14 +1324,18 @@ Inode::SetFileSize(Transaction *transaction, off_t size)
 status_t 
 Inode::Append(Transaction *transaction,off_t bytes)
 {
-	return SetFileSize(transaction,Size() + bytes);
+	return SetFileSize(transaction, Size() + bytes);
 }
 
 
 status_t 
 Inode::Trim(Transaction *transaction)
 {
-	return ShrinkStream(transaction,Size());
+	status_t status = ShrinkStream(transaction, Size());
+	if (status < B_OK)
+		return status;
+
+	return WriteBack(transaction);
 }
 
 

src/add-ons/kernel/file_systems/bfs/Inode.h

@@ -138,6 +138,7 @@ class Inode : public CachedBlock {
 			// currently only files can be accessed with bfs_read()/bfs_write()
 
 		off_t Size() const { return Node()->data.size; }
+		off_t LastModified() const { return Node()->last_modified_time; }
 
 		block_run &BlockRun() const { return Node()->inode_num; }
 		block_run &Parent() const { return Node()->parent; }

src/add-ons/kernel/file_systems/bfs/ToDo

@@ -1,4 +1,4 @@
-BFS - ToDo, June 23th, 2002
+BFS - ToDo, August 12th, 2002
 -----
 
 BlockAllocator
@@ -43,12 +43,14 @@ BPlusTree
 
 Inode
 
- - sometimes the inode's last modified time seems to be wrong, and is therefore not found in the b+tree (assuming that the b+tree is working correctly, what I do)
+ - exchange Inode::OldLastModified() with Inode::NewLastModified(), and don't change the last_modified field directly in Inode::WriteAt() for consistency in case of a crash
+ - the size is only updated in bfs_close() - but if the system crashes before, the entry in the size index doesn't match the one in the inode anymore - it would be better to let the data.size not reflect the real file size in this case (since the max_xxx_range entries are always correct)
  - Inode::FillGapWithZeros() currently disabled; apart from being slow, it really shouldn't be executed while a transaction is running, because that stops all other threads from doing anything (which can be a long time for a 100 MB file)
 
 
 Indices
 
+ - consider Index::UpdateLastModified() writing back the updated inode
 
 
 Attributes
@@ -61,7 +63,7 @@ Volume
 
 kernel_interface
 
- - missing functions, maybe they are not all needed (but most of them are): bfs_rename_attr(), bfs_rename_index(), bfs_initialize(), bfs_setflags(), bfs_link()
+ - missing functions, maybe they are not really needed: bfs_rename_attr(), bfs_rename_index(), bfs_initialize(), bfs_setflags(), bfs_link()
  - bfs_rename() currently doesn't respect any permissions
 
 

src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp

@@ -1146,6 +1146,7 @@ bfs_write(void *_ns, void *_node, void *_cookie, off_t pos, const void *buffer,
 		transaction.Done();
 
 	// periodically notify if the file size has changed
+	// ToDo: should we better test for a change in the last_modified time only?
 	if (cookie->last_size != inode->Size()
 		&& system_time() > cookie->last_notification + INODE_NOTIFICATION_INTERVAL) {
 		notify_listener(B_STAT_CHANGED,volume->ID(),0,0,inode->ID(),NULL);
@@ -1183,20 +1184,32 @@ bfs_close(void *_ns, void *_node, void *_cookie)
 		// trim the preallocated blocks and update the size,
 		// and last_modified indices if needed
 
-		Transaction transaction(volume,inode->BlockNumber());
-
-		status_t status = inode->Trim(&transaction);
-		if (status < B_OK)
-			FATAL(("Could not trim preallocated blocks!"));
-
+		Transaction transaction(volume, inode->BlockNumber());
+		status_t status = B_OK;
+		bool changed = false;
 		Index index(volume);
-		index.UpdateSize(&transaction,inode);
-		index.UpdateLastModified(&transaction,inode);
+
+		if (inode->OldSize() != inode->Size()) {
+			status = inode->Trim(&transaction);
+			if (status < B_OK)
+				FATAL(("Could not trim preallocated blocks!"));
+	
+			index.UpdateSize(&transaction, inode);
+			changed = true;
+		}
+		if (inode->OldLastModified() != inode->LastModified()) {
+			index.UpdateLastModified(&transaction, inode, inode->LastModified());
+			changed = true;
+			
+			// updating the index doesn't write back the inode
+			inode->WriteBack(&transaction);
+		}
 
 		if (status == B_OK)
 			transaction.Done();
 
-		notify_listener(B_STAT_CHANGED,volume->ID(),0,0,inode->ID(),NULL);
+		if (changed)
+			notify_listener(B_STAT_CHANGED, volume->ID(), 0, 0, inode->ID(), NULL);
 	}
 
 	if (inode->Flags() & INODE_NO_CACHE) {