Wouter,

> Setting _FILE_OFFSET_BITS changes the definition of off_t to a 64-bit
> integer on 32-bit systems (rather than a 32-bit one) and changes all
> relevant syscalls to behave like their ...64() variants. This does
> change semantics, so setting it indiscriminately might not be a very
> good idea.

Sure.

> Yeah. OTOH, I don't think I still support non-LFS operation these days.
> Initially the option to disable it was so that one could get the 'old'
> behaviour back when I was initially adding LFS support, precisely
> because it was causing issues. But that time is long past now, and I
> don't see why anyone would want to say "please build me an nbd-server
> that cannot read files with a size larger than 2GB".

Well, on the principle of least change, what I am doing now is
ignoring HAVE_SYNC_FILE_RANGE (and thus avoiding setting _GNU_SOURCE)
unless we are compiling with NBD_LFS. As sync_file_range is a 64
bit call, I would not like to bet on it working well in combination with
non-LFS builds. This means that if you are building without NBD_LFS
you get fdatasync() instead, which is not the end of the world.

>> I'm happy to recode this bit to use a FUA bit from the top of the
>> command 32bit int. However, if we do that, we should probably reserve
>> a fixed number of bits there (e.g.  16 bits - I can't imagine more
>> than 65,536 different command types) for per-command flags).
>
> Obviously, and 16 bits sounds like a good amount.

OK, I will recode it to do that.

>> We should then probably have a (separate) NBD_ flag that says "the
>> server supports flags being passed in the the command type". It seemed
>> a bit much to change just to save one command. What do you think?
>>
>> Incidentally, I think that block could be better written as a switch
>> on request.type. Assuming it is a read if it's not anything else
>> is perhaps not the safest.
>
> Yeah, absolutely; I'll be the first to admit that nbd-server is a bit
> hairy in some places.
>
> Originally it only supported the READ and WRITE commands, in which case
> an if was reasonable; later, the DISC was added, and another if block
> tacked along. As more protocol commands become available, however,
> changing that to a switch is indeed the most sensible thing to do.

OK, I might refactor that then.

> Okay; I'll hold off on applying this until that's been done.

That is wise :-)

> If relevant and possible, I'd appreciate it if you could also add
> something to nbd-tester-client and the simple_test script, so that I
> don't accidentally break it in the future. But don't sweat if it turns
> out to be too hard.

Yes, I'll look at that too.

-- 
Alex Bligh

------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
Nbd-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/nbd-general

Reply via email to