Re: [Nbd] [RFC] [PATCH] support FUA and FLUSH
- To: Wouter Verhelst <w@...112...>
- Cc: nbd-general@lists.sourceforge.net
- Subject: Re: [Nbd] [RFC] [PATCH] support FUA and FLUSH
- From: Alex Bligh <alex@...872...>
- Date: Mon, 16 May 2011 17:37:37 +0100
- Message-id: <32F8A0FBF03A063723FAC3CF@...873...>
- Reply-to: Alex Bligh <alex@...872...>
- In-reply-to: <20110516162646.GF2954@...510...>
- References: <6FF961C550246D012FE2FC87@...873...> <20110516162646.GF2954@...510...>
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
Reply to: