diff --git a/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.cpp b/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.cpp index 66606aeb04..ee67354819 100644 --- a/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.cpp @@ -140,6 +140,27 @@ UnpackingDirectory::IsOnlyPackageNode(PackageNode* node) const } +bool +UnpackingDirectory::WillBeFirstPackageNode(PackageNode* packageNode) const +{ + PackageDirectory* packageDirectory + = dynamic_cast(packageNode); + if (packageDirectory == NULL) + return false; + + PackageDirectory* other = fPackageDirectories.Head(); + return other == NULL + || packageDirectory->ModifiedTime() > other->ModifiedTime(); +} + + +void +UnpackingDirectory::PrepareForRemoval() +{ + fPackageDirectories.MakeEmpty(); +} + + status_t UnpackingDirectory::OpenAttributeDirectory(AttributeDirectoryCookie*& _cookie) { diff --git a/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.h b/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.h index 61d73034fc..baad89b1d5 100644 --- a/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.h +++ b/src/add-ons/kernel/file_systems/packagefs/UnpackingDirectory.h @@ -29,6 +29,10 @@ public: virtual PackageNode* GetPackageNode(); virtual bool IsOnlyPackageNode(PackageNode* node) const; + virtual bool WillBeFirstPackageNode( + PackageNode* packageNode) const; + + virtual void PrepareForRemoval(); virtual status_t OpenAttributeDirectory( AttributeDirectoryCookie*& _cookie); diff --git a/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.cpp b/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.cpp index 9d671a389b..3ed264b84c 100644 --- a/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.cpp @@ -9,6 +9,7 @@ #include #include +#include #include "UnpackingAttributeCookie.h" #include "UnpackingAttributeDirectoryCookie.h" @@ -17,20 +18,23 @@ UnpackingLeafNode::UnpackingLeafNode(ino_t id) : - Node(id) + Node(id), + fFinalPackageNode(NULL) { } UnpackingLeafNode::~UnpackingLeafNode() { + if (fFinalPackageNode != NULL) + fFinalPackageNode->ReleaseReference(); } status_t UnpackingLeafNode::VFSInit(dev_t deviceID) { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->VFSInit(deviceID, fID); return B_OK; } @@ -39,7 +43,7 @@ UnpackingLeafNode::VFSInit(dev_t deviceID) void UnpackingLeafNode::VFSUninit() { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) packageNode->VFSUninit(); } @@ -47,7 +51,7 @@ UnpackingLeafNode::VFSUninit() mode_t UnpackingLeafNode::Mode() const { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->Mode(); return S_IFREG | S_IRUSR | S_IRGRP | S_IROTH; } @@ -56,7 +60,7 @@ UnpackingLeafNode::Mode() const uid_t UnpackingLeafNode::UserID() const { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->UserID(); return 0; } @@ -65,7 +69,7 @@ UnpackingLeafNode::UserID() const gid_t UnpackingLeafNode::GroupID() const { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->GroupID(); return 0; } @@ -74,7 +78,7 @@ UnpackingLeafNode::GroupID() const timespec UnpackingLeafNode::ModifiedTime() const { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->ModifiedTime(); timespec time = { 0, 0 }; @@ -85,7 +89,7 @@ UnpackingLeafNode::ModifiedTime() const off_t UnpackingLeafNode::FileSize() const { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->FileSize(); return 0; } @@ -101,6 +105,8 @@ UnpackingLeafNode::GetNode() status_t UnpackingLeafNode::AddPackageNode(PackageNode* packageNode) { + ASSERT(fFinalPackageNode == NULL); + if (S_ISDIR(packageNode->Mode())) return B_BAD_VALUE; @@ -127,6 +133,8 @@ UnpackingLeafNode::AddPackageNode(PackageNode* packageNode) void UnpackingLeafNode::RemovePackageNode(PackageNode* packageNode) { + ASSERT(fFinalPackageNode == NULL); + bool isNewest = packageNode == fPackageNodes.Head(); fPackageNodes.Remove(dynamic_cast(packageNode)); @@ -152,22 +160,78 @@ UnpackingLeafNode::RemovePackageNode(PackageNode* packageNode) PackageNode* UnpackingLeafNode::GetPackageNode() { - return fPackageNodes.Head(); + return _ActivePackageNode(); } bool UnpackingLeafNode::IsOnlyPackageNode(PackageNode* node) const { + ASSERT(fFinalPackageNode == NULL); + PackageLeafNode* head = fPackageNodes.Head(); return node == head && fPackageNodes.GetNext(head) == NULL; } +bool +UnpackingLeafNode::WillBeFirstPackageNode(PackageNode* packageNode) const +{ + PackageLeafNode* packageLeafNode + = dynamic_cast(packageNode); + if (packageLeafNode == NULL) + return false; + + PackageLeafNode* headNode = fPackageNodes.Head(); + return headNode == NULL + || packageLeafNode->ModifiedTime() > headNode->ModifiedTime(); +} + +void +UnpackingLeafNode::PrepareForRemoval() +{ + ASSERT(fFinalPackageNode == NULL); + + fFinalPackageNode = fPackageNodes.Head(); + if (fFinalPackageNode != NULL) { + fFinalPackageNode->AcquireReference(); + fPackageNodes.MakeEmpty(); + } +} + + +status_t +UnpackingLeafNode::CloneTransferPackageNodes(ino_t id, UnpackingNode*& _newNode) +{ + ASSERT(fFinalPackageNode == NULL); + + UnpackingLeafNode* clone = new(std::nothrow) UnpackingLeafNode(id); + if (clone == NULL) + return B_NO_MEMORY; + + status_t error = clone->Init(Parent(), Name(), 0); + if (error != B_OK) { + delete clone; + return error; + } + + // We keep the old head as fFinalPackageNode, which will make us to behave + // exactly as before with respect to FS operations. + fFinalPackageNode = fPackageNodes.Head(); + if (fFinalPackageNode != NULL) { + fFinalPackageNode->AcquireReference(); + clone->fPackageNodes.MoveFrom(&fPackageNodes); + } + + _newNode = clone; + return B_OK; +} + + status_t UnpackingLeafNode::Read(off_t offset, void* buffer, size_t* bufferSize) { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->Read(offset, buffer, bufferSize); return B_ERROR; } @@ -176,7 +240,7 @@ UnpackingLeafNode::Read(off_t offset, void* buffer, size_t* bufferSize) status_t UnpackingLeafNode::Read(io_request* request) { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->Read(request); return EBADF; } @@ -185,7 +249,7 @@ UnpackingLeafNode::Read(io_request* request) status_t UnpackingLeafNode::ReadSymlink(void* buffer, size_t* bufferSize) { - PackageLeafNode* packageNode = fPackageNodes.Head(); + PackageLeafNode* packageNode = _ActivePackageNode(); if (packageNode == NULL) return B_BAD_VALUE; @@ -206,7 +270,7 @@ UnpackingLeafNode::ReadSymlink(void* buffer, size_t* bufferSize) status_t UnpackingLeafNode::OpenAttributeDirectory(AttributeDirectoryCookie*& _cookie) { - return UnpackingAttributeDirectoryCookie::Open(fPackageNodes.Head(), + return UnpackingAttributeDirectoryCookie::Open(_ActivePackageNode(), _cookie); } @@ -215,7 +279,7 @@ status_t UnpackingLeafNode::OpenAttribute(const char* name, int openMode, AttributeCookie*& _cookie) { - return UnpackingAttributeCookie::Open(fPackageNodes.Head(), name, openMode, + return UnpackingAttributeCookie::Open(_ActivePackageNode(), name, openMode, _cookie); } @@ -223,7 +287,7 @@ UnpackingLeafNode::OpenAttribute(const char* name, int openMode, status_t UnpackingLeafNode::IndexAttribute(AttributeIndexer* indexer) { - return UnpackingAttributeCookie::IndexAttribute(fPackageNodes.Head(), + return UnpackingAttributeCookie::IndexAttribute(_ActivePackageNode(), indexer); } @@ -231,7 +295,16 @@ UnpackingLeafNode::IndexAttribute(AttributeIndexer* indexer) void* UnpackingLeafNode::IndexCookieForAttribute(const char* name) const { - if (PackageLeafNode* packageNode = fPackageNodes.Head()) + if (PackageLeafNode* packageNode = _ActivePackageNode()) return packageNode->IndexCookieForAttribute(name); return NULL; } + + +PackageLeafNode* +UnpackingLeafNode::_ActivePackageNode() const +{ + if (PackageLeafNode* packageNode = fPackageNodes.Head()) + return packageNode; + return fFinalPackageNode; +} diff --git a/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.h b/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.h index e1b8db4340..e3bb4f175c 100644 --- a/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.h +++ b/src/add-ons/kernel/file_systems/packagefs/UnpackingLeafNode.h @@ -32,6 +32,12 @@ public: virtual PackageNode* GetPackageNode(); virtual bool IsOnlyPackageNode(PackageNode* node) const; + virtual bool WillBeFirstPackageNode( + PackageNode* packageNode) const; + + virtual void PrepareForRemoval(); + virtual status_t CloneTransferPackageNodes(ino_t id, + UnpackingNode*& _newNode); virtual status_t Read(off_t offset, void* buffer, size_t* bufferSize); @@ -48,8 +54,12 @@ public: virtual status_t IndexAttribute(AttributeIndexer* indexer); virtual void* IndexCookieForAttribute(const char* name) const; +private: + inline PackageLeafNode* _ActivePackageNode() const; + private: PackageLeafNodeList fPackageNodes; + PackageLeafNode* fFinalPackageNode; }; diff --git a/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.cpp b/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.cpp index dd99ca9dd2..f9700ab7ef 100644 --- a/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.cpp @@ -10,3 +10,10 @@ UnpackingNode::~UnpackingNode() { } + + +status_t +UnpackingNode::CloneTransferPackageNodes(ino_t id, UnpackingNode*& _newNode) +{ + return B_BAD_VALUE; +} diff --git a/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.h b/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.h index 3ac50316c7..28130ca661 100644 --- a/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.h +++ b/src/add-ons/kernel/file_systems/packagefs/UnpackingNode.h @@ -24,6 +24,12 @@ public: virtual PackageNode* GetPackageNode() = 0; virtual bool IsOnlyPackageNode(PackageNode* node) const = 0; + virtual bool WillBeFirstPackageNode( + PackageNode* packageNode) const = 0; + + virtual void PrepareForRemoval() = 0; + virtual status_t CloneTransferPackageNodes(ino_t id, + UnpackingNode*& _newNode); }; diff --git a/src/add-ons/kernel/file_systems/packagefs/Volume.cpp b/src/add-ons/kernel/file_systems/packagefs/Volume.cpp index 88e96cf3a0..8e7c70e576 100644 --- a/src/add-ons/kernel/file_systems/packagefs/Volume.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/Volume.cpp @@ -1243,21 +1243,59 @@ Volume::_AddPackageNode(Directory* directory, PackageNode* packageNode, newNode = true; } - // TODO: The non-new part is broken for files. If a node is already known to - // the VFS, we can't just change the file content. The file might be an - // executable or library that is currently in use (i.e. mapped) and when - // just changing the file content we break things horribly. In fact we don't - // even do that correctly in UnpackingLeafNode::AddPackageNode() -- neither - // the former nor the new head package node is notified. - BReference nodeReference(node); NodeWriteLocker nodeWriteLocker(node); + BReference newNodeReference; + NodeWriteLocker newNodeWriteLocker; + Node* oldNode = NULL; + + if (!newNode && !S_ISDIR(node->Mode()) && oldPackageNode != NULL + && unpackingNode->WillBeFirstPackageNode(packageNode)) { + // The package node we're going to add will represent the node, + // replacing the current head package node. Since the node isn't a + // directory, we must make sure that clients having opened or mapped the + // node won't be surprised. So we create a new node and remove the + // current one. + // create a new node and transfer the package nodes to it + UnpackingNode* newUnpackingNode; + status_t error = unpackingNode->CloneTransferPackageNodes( + fNextNodeID++, newUnpackingNode); + if (error != B_OK) + RETURN_ERROR(error); + + // remove the old node + _NotifyNodeRemoved(node); + _RemoveNodeAndVNode(node); + oldNode = node; + + // add the new node + unpackingNode = newUnpackingNode; + node = unpackingNode->GetNode(); + newNodeReference.SetTo(node); + newNodeWriteLocker.SetTo(node, false); + + directory->AddChild(node); + fNodes.Insert(node); + newNode = true; + } + status_t error = unpackingNode->AddPackageNode(packageNode); if (error != B_OK) { - // remove the node, if created before - if (newNode) - _RemoveNode(node); + // Remove the node, if created before. If the node was created to + // replace the previous node, send out notifications instead. + if (newNode) { + if (oldNode != NULL) { + _NotifyNodeAdded(node); + if (notify) { + notify_entry_removed(ID(), directory->ID(), oldNode->Name(), + oldNode->ID()); + notify_entry_created(ID(), directory->ID(), node->Name(), + node->ID()); + } + } else + _RemoveNode(node); + } RETURN_ERROR(error); } @@ -1270,21 +1308,18 @@ Volume::_AddPackageNode(Directory* directory, PackageNode* packageNode, if (notify) { if (newNode) { + if (oldNode != NULL) { + notify_entry_removed(ID(), directory->ID(), oldNode->Name(), + oldNode->ID()); + } notify_entry_created(ID(), directory->ID(), node->Name(), node->ID()); } else if (packageNode == unpackingNode->GetPackageNode()) { // The new package node has become the one representing the node. // Send stat changed notification for directories and entry // removed + created notifications for files and symlinks. - if (S_ISDIR(packageNode->Mode())) { - notify_stat_changed(ID(), node->ID(), kAllStatFields); - // TODO: Actually the attributes might change, too! - } else { - notify_entry_removed(ID(), directory->ID(), node->Name(), - node->ID()); - notify_entry_created(ID(), directory->ID(), node->Name(), - node->ID()); - } + notify_stat_changed(ID(), node->ID(), kAllStatFields); + // TODO: Actually the attributes might change, too! } } @@ -1304,11 +1339,12 @@ Volume::_RemovePackageNode(Directory* directory, PackageNode* packageNode, BReference nodeReference(node); NodeWriteLocker nodeWriteLocker(node); - // TODO: This is broken for files that are in use. Cf. _AddPackageNode() for - // details. - PackageNode* headPackageNode = unpackingNode->GetPackageNode(); bool nodeRemoved = false; + Node* newNode = NULL; + + BReference newNodeReference; + NodeWriteLocker newNodeWriteLocker; // If this is the last package node of the node, remove it completely. if (unpackingNode->IsOnlyPackageNode(packageNode)) { @@ -1316,29 +1352,54 @@ Volume::_RemovePackageNode(Directory* directory, PackageNode* packageNode, // find the node anymore. _NotifyNodeRemoved(node); - unpackingNode->RemovePackageNode(packageNode); + unpackingNode->PrepareForRemoval(); - // we get and put the vnode to notify the VFS - // TODO: We should probably only do that, if the node is known to the - // VFS in the first place. - Node* dummyNode; - bool gotVNode = GetVNode(node->ID(), dummyNode) == B_OK; - - _RemoveNode(node); + _RemoveNodeAndVNode(node); nodeRemoved = true; - - if (gotVNode) { - RemoveVNode(node->ID()); - PutVNode(node->ID()); - } - } else { - // The node does at least have one more package node. - unpackingNode->RemovePackageNode(packageNode); - - if (packageNode == headPackageNode) { + } else if (packageNode == headPackageNode) { + // The node does at least have one more package node, but the one to be + // removed is the head. Unless it's a directory, we replace the node + // with a completely new one and let the old one die. This is necessary + // to avoid surprises for clients that have opened/mapped the node. + if (S_ISDIR(packageNode->Mode())) { + unpackingNode->RemovePackageNode(packageNode); _NotifyNodeChanged(node, kAllStatFields, OldUnpackingNodeAttributes(headPackageNode)); + } else { + // create a new node and transfer the package nodes to it + UnpackingNode* newUnpackingNode; + status_t error = unpackingNode->CloneTransferPackageNodes( + fNextNodeID++, newUnpackingNode); + if (error == B_OK) { + // remove the package node + newUnpackingNode->RemovePackageNode(packageNode); + + // remove the old node + _NotifyNodeRemoved(node); + _RemoveNodeAndVNode(node); + + // add the new node + newNode = newUnpackingNode->GetNode(); + newNodeReference.SetTo(newNode); + newNodeWriteLocker.SetTo(newNode, false); + + directory->AddChild(newNode); + fNodes.Insert(newNode); + _NotifyNodeAdded(newNode); + } else { + // There's nothing we can do. Remove the node completely. + _NotifyNodeRemoved(node); + + unpackingNode->PrepareForRemoval(); + + _RemoveNodeAndVNode(node); + nodeRemoved = true; + } } + } else { + // The package node to remove is not the head of the node. This change + // doesn't have any visible effect. + unpackingNode->RemovePackageNode(packageNode); } if (!notify) @@ -1348,7 +1409,7 @@ Volume::_RemovePackageNode(Directory* directory, PackageNode* packageNode, if (nodeRemoved) { notify_entry_removed(ID(), directory->ID(), node->Name(), node->ID()); } else if (packageNode == headPackageNode) { - // The new package node was the one representing the node. + // The removed package node was the one representing the node. // Send stat changed notification for directories and entry // removed + created notifications for files and symlinks. if (S_ISDIR(packageNode->Mode())) { @@ -1357,8 +1418,8 @@ Volume::_RemovePackageNode(Directory* directory, PackageNode* packageNode, } else { notify_entry_removed(ID(), directory->ID(), node->Name(), node->ID()); - notify_entry_created(ID(), directory->ID(), node->Name(), - node->ID()); + notify_entry_created(ID(), directory->ID(), newNode->Name(), + newNode->ID()); } } } @@ -1410,6 +1471,24 @@ Volume::_RemoveNode(Node* node) } +void +Volume::_RemoveNodeAndVNode(Node* node) +{ + // we get and put the vnode to notify the VFS + // TODO: We should probably only do that, if the node is known to the + // VFS in the first place. + Node* dummyNode; + bool gotVNode = GetVNode(node->ID(), dummyNode) == B_OK; + + _RemoveNode(node); + + if (gotVNode) { + RemoveVNode(node->ID()); + PutVNode(node->ID()); + } +} + + void Volume::_DomainListenerEventOccurred(PackageDomain* domain, const KMessage* event) diff --git a/src/add-ons/kernel/file_systems/packagefs/Volume.h b/src/add-ons/kernel/file_systems/packagefs/Volume.h index d939b66a8f..d2356180ac 100644 --- a/src/add-ons/kernel/file_systems/packagefs/Volume.h +++ b/src/add-ons/kernel/file_systems/packagefs/Volume.h @@ -1,5 +1,5 @@ /* - * Copyright 2009, Ingo Weinhold, ingo_weinhold@gmx.de. + * Copyright 2009-2011, Ingo Weinhold, ingo_weinhold@gmx.de. * Distributed under the terms of the MIT License. */ #ifndef VOLUME_H @@ -157,6 +157,8 @@ private: UnpackingNode*& _node); // does *not* return a reference void _RemoveNode(Node* node); + void _RemoveNodeAndVNode(Node* node); + // caller must hold a reference void _DomainListenerEventOccurred( PackageDomain* domain,