[haiku-development] Re: chroot and package daemon bug
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Wed, 17 Aug 2016 20:21:58 +0200
On 08/16/2016 10:46 PM, Adrien Destugues wrote:
On Fri, Aug 12, 2016 at 09:02:32PM +0200, Ingo Weinhold wrote:
On 08/11/2016 10:38 PM, Adrien Destugues wrote:
You can just
struct stat st;
stat("/", &st);
and use (st.st_dev, st.st_ino) (a node_ref).
2) The vnode (or most likely, the dev and inode numbers) are sent to the
package daemon in GetInstallLocationInfo
3) The package daemon uses the given node as a root, instead of using
the default fSystemRoot (PackageDaemon.cpp:87)
4) The correct package dirs are returned, the code works as expected.
Sounds good.
Attached is what I have so far.
In case you intended to attach a patch to the mail, it didn't get here.
It turned out a bit more complex on
package daemon side:
- I tried to create a Root and a Volume on the fly when the request is
sent.
That doesn't sound right.
As far as I can see, the Root being a chroot, there is no mount
notification when it is created. I'm not sure if the package daemon
can infer the root when it sees a volume being created.
A Root object in the package daemon represents the base directory of a
standard directory layout that contains a mounted system and/or a home
packagefs volume. Whenever the package daemon encounters a packagefs
volume (initial scan on startup or when it gets a B_DEVICE_MOUNTED
notification), a Volume object is created for it and, if the volume is
of type "system" or "home", that Volume is either added to the matching
preexisting Root object or a new Root object is created.
So there's initially a Root object for "/" and as soon as a packagefs
for a (future or already existing) chroot is mounted a Root object for
the chroot base directory will be created. So you don't have to create
it; it should already exist.
- I tried passing the request to the Volume directly, but 1) this is not
allowed as per comments in Root and Volume (operations on the Volume
should always run in the Root's job worker thread)
The thread/locking policy is documented in Volume.h. Only write
operations need to be performed by the Root's thread. Not sure, if
there's a stray obsolete comment somewhere. IIRC initially it did indeed
work differently, but having "get location info" requests wait for
pending write operations (which could block indefinitely due to possibly
required user interaction) wasn't a good idea.
, and 2) it doesn't
work anyway because populating the volume with the list of packages is
done in the job queue, and there is no way to wait for that operation
to complete.
- So, I changed the Root class to process the request asynchronously by
posting it to the job queue. This serializes things as designed and
possibly fixes other concurrency problems.
I'm afraid, I don't understand, why you trying that. Maybe I'm missing
something, but the approach you proposed in your previous mail sounded
good. If you've implemented step 2) (adding the node_ref to the client's
message), implementing step 3) should be fairly simple: In the
B_MESSAGE_GET_INSTALLATION_LOCATION_INFO/B_MESSAGE_COMMIT_TRANSACTION
case in PackageDaemon::MessageReceived() extract the node_ref from the
message, use _FindRoot() to get the corresponding Root object and call
HandleRequest() on it (instead of on fSystemRoot).
CU, Ingo
Other related posts:
