Re: [uml-devel] do_one_request to prepare_request

[Jeff D. <jd...@ka...>]

jam...@ho... said:

> This moves the error tests and sync io call to do_io/write to
> io_thread up  into do_ubd_request and makes do_one_request into
> prepare_request like you  mentioned a few days ago.
> prepare_request(struct request *req,struct  io_request *io_req) now
> just takes the req and builds a io_req.  Which lets  do_ubd_request do
> the loop, errorchecking and exit tests all in one place  and only
> once.

Nice.  I structured it a bit differently than in the patch though.  Let
me know what you think.  My biggest objection was to that big while(1){ ... },
which is deceptive because that will hardly ever loop.  The overall
structure of the code should reflect how it will actually run.

> I also made block into a __u64 so that attempts to access block #>2G
> should  not wrap anymore, block was a int and then cast to __u64 for
> the << 9.  

Good spotting.

> I  moved the req assignment to the top of the functions and
> sync io loop, and  added a ubd_dev pointer *up which made several
> lines much shorter and  removed the dev variabile.  

I like the name "dev" better :-)  So I resurrected it in place of up.

> I also noted the the CLEAR_INTR had dissappeared from the QUEUE_EMPTY
> test  exit?

That should be unnecessary.  ubd_finish does it when it receives a request.

> In ubd_handler if a spurious interrupt occurs you issue a io error on
> the  current block being transfered and dequeue it, is this intended? 

Unexpected interrupts should not happen, so it's not clear what the right
thing to do is.  It probably depends on why it happened, which is a little
tough to predict for something that shouldn't happen.  At least the current
behavior lets the block layer know that something is seriously wrong.

> The other exit I was unclear on is if ubd_dev[dev].is_dir is set then
> "HOSTFS:" + ubd_dev[dev].file is copied to the buffer and the request
> is  marked as completed,  I am not clear on the intended purpose
> there, to me it  appears that this would be for the hostfs support but
> I dont see a obvious  connection from hostfs to a removed request.

This is a kludge, but it's a good kludge :-)

The way root hostfs works is that hostfs registers two filesystems, the 
normal virtual non-device hostfs, and a device filesystem - "root-hostfs".
Obviously, that's a lie because hostfs can't be a device filesystem because
it makes libc calls into the host, not IO calls into the block layer.

When you ask for your root filesystem to be mounted from a directory rather
than a filesystem image, the kernel asks the boot device for the filesystem
superblock and then asks all the filesystems if they recognize it.  So,
what "ubd0=/path/to/uml/root/dir" does is tell the ubd driver to make a fake
superblock that only hostfs recognizes.  Once hostfs tells vfs that it
recognizes that superblock, it is mounted, and does libc calls to the host
and never makes any block IO requests.

So, that request has to be marked as finished, because there is no way it
could proceed.  It makes no sense to try to read blocks out of a directory.

				Jeff