[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] fua, trim, etc



On Tue, Sep 13, 2011 at 11:49:14AM +0100, Alex Bligh wrote:
> 
> 
> --On 13 September 2011 12:36:07 +0200 Wouter Verhelst <w@...112...> wrote:
> 
> > I should probably make another release some time soon, there are quite a
> > number of new features waiting;
> 
> Not on your list: my FUA/FLUSH stuff is in, but I must admit to having
> lost track of where the ioctl() discussion got to. As there is apparently
> now agreement on the ioctl(), it would be really nice if we could either
> keep the current one or replace it in this version with one Paul et
> al are happy with.
> 
> For instance, Paul's suggestion on setting 'rotational' using /proc
> instead is definitely better than the way nbd-client does it at the
> moment.
> 
> > - Make NBD_CMD_TRIM cause nbd-server to call fallocate() with
> >   FALLOC_FL_PUNCH_HOLE.
> 
> We need to be a bit careful on these. IIRC (someone please check!)
> on Linux fallocate() is an extended call that supports various stuff,
> whereas posix_fallocate() is the normal POSIX call, and that doesn't
> support PUNCH_HOLE. On non-Linux systems, fallocate() is the normal
> POSIX call.

Oh, sure. Any fallocate() call is going into #ifdef HAVE_FALLOC_PH,
which checks for existence of the FALLOCATE_FL_PUNCH_HOLE flag. If that
doesn't exist, then the current code will be executed.

> Secondly (a) PUNCH_HOLE is not supported on all kernel versions,
> and (b) it isn't necessarily supported on all filing systems and
> (c) on some filing systems it is buggy.

Well, I'm not sure I want to check whether the file system is buggy.
Also, if you say "trim = false" (or just don't say "trim = true") in
your configuration, then we don't announce understanding of
NBD_CMD_TRIM, and we don't get to deal with the issue.

> I /think/ we can just see if FALLOC_FL_PUNCH_HOLE is defined,
> and if so, make the call, and ignore the error condition. That
> way if compiled on box X and run on box Y it will still be OK.
> However, I think we ought to have a config option to turn it off.

Definitely. And it's already there :-)

-- 
The volume of a pizza of thickness a and radius z can be described by
the following formula:

pi zz a



Reply to: