We already have support in the file driver for punching holes during .zero with the may_trim flag (via FALLOC_FL_PUNCH_HOLE), so we should use the same mechanism to support .trim. Note that the NBD spec says that trim is advisory (we can return success even if we did nothing); but at the same time, it's nicer to avoid advertising the feature if we know for sure we can't do it, so we also need .can_trim. Note that there's still an element of runtime behavior, as FALLOC_FL_PUNCH_HOLE doesn't work on all filesystems; there we fall back on the NBD protocol allowing us to be advisory for all but a handful of errno values that we can directly report back over NBD. Signed-off-by: Eric Blake <eblake@redhat.com> --- I couldn't test --filter=log results on trim commands without at least one plugin that supports trim ;) plugins/file/file.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/plugins/file/file.c b/plugins/file/file.c index 1fe4191..081848b 100644 --- a/plugins/file/file.c +++ b/plugins/file/file.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013 Red Hat Inc. + * Copyright (C) 2013-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -175,6 +175,18 @@ file_get_size (void *handle) return statbuf.st_size; } +static int +file_can_trim (void *handle) +{ + /* Trim is advisory, but we prefer to advertise it only when we can + * actually (attempt to) punch holes. */ +#ifdef FALLOC_FL_PUNCH_HOLE + return 1; +#else + return 0; +#endif +} + /* Read data from the file. */ static int file_pread (void *handle, void *buf, uint32_t count, uint64_t offset) @@ -219,7 +231,7 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset) return 0; } -/* Write data to the file. */ +/* Write zeroes to the file. */ static int file_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) { @@ -268,6 +280,33 @@ file_flush (void *handle) return 0; } +/* Punch a hole in the file. */ +static int +file_trim (void *handle, uint32_t count, uint64_t offset) +{ + int r = -1; +#ifdef FALLOC_FL_PUNCH_HOLE + struct handle *h = handle; + + /* Trim is advisory; we don't care if it fails for anything other + * than EIO or EPERM. */ + r = fallocate (h->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, + offset, count); + if (r < 0) { + if (errno != EPERM && errno != EIO) { + nbdkit_debug ("ignoring failed fallocate during trim: %m"); + r = 0; + } + else + nbdkit_error ("fallocate: %m"); + } +#else + /* Based on .can_trim, this should not be reached. */ + errno = EOPNOTSUPP; +#endif + return r; +} + static struct nbdkit_plugin plugin = { .name = "file", .longname = "nbdkit file plugin", @@ -279,10 +318,12 @@ static struct nbdkit_plugin plugin = { .open = file_open, .close = file_close, .get_size = file_get_size, + .can_trim = file_can_trim, .pread = file_pread, .pwrite = file_pwrite, - .zero = file_zero, .flush = file_flush, + .trim = file_trim, + .zero = file_zero, .errno_is_preserved = 1, }; -- 2.14.3
Richard W.M. Jones
2018-Jan-31 16:23 UTC
Re: [Libguestfs] [nbdkit PATCH] file: Add trim support
On Tue, Jan 30, 2018 at 08:14:05PM -0600, Eric Blake wrote:> We already have support in the file driver for punching holes > during .zero with the may_trim flag (via FALLOC_FL_PUNCH_HOLE), > so we should use the same mechanism to support .trim. Note that > the NBD spec says that trim is advisory (we can return success > even if we did nothing); but at the same time, it's nicer to > avoid advertising the feature if we know for sure we can't do > it, so we also need .can_trim. Note that there's still an > element of runtime behavior, as FALLOC_FL_PUNCH_HOLE doesn't > work on all filesystems; there we fall back on the NBD > protocol allowing us to be advisory for all but a handful of > errno values that we can directly report back over NBD. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > > I couldn't test --filter=log results on trim commands without > at least one plugin that supports trim ;) > > plugins/file/file.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 44 insertions(+), 3 deletions(-) > > diff --git a/plugins/file/file.c b/plugins/file/file.c > index 1fe4191..081848b 100644 > --- a/plugins/file/file.c > +++ b/plugins/file/file.c > @@ -1,5 +1,5 @@ > /* nbdkit > - * Copyright (C) 2013 Red Hat Inc. > + * Copyright (C) 2013-2018 Red Hat Inc. > * All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > @@ -175,6 +175,18 @@ file_get_size (void *handle) > return statbuf.st_size; > } > > +static int > +file_can_trim (void *handle) > +{ > + /* Trim is advisory, but we prefer to advertise it only when we can > + * actually (attempt to) punch holes. */ > +#ifdef FALLOC_FL_PUNCH_HOLE > + return 1; > +#else > + return 0; > +#endif > +} > + > /* Read data from the file. */ > static int > file_pread (void *handle, void *buf, uint32_t count, uint64_t offset) > @@ -219,7 +231,7 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset) > return 0; > } > > -/* Write data to the file. */ > +/* Write zeroes to the file. */ > static int > file_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) > { > @@ -268,6 +280,33 @@ file_flush (void *handle) > return 0; > } > > +/* Punch a hole in the file. */ > +static int > +file_trim (void *handle, uint32_t count, uint64_t offset) > +{ > + int r = -1; > +#ifdef FALLOC_FL_PUNCH_HOLE > + struct handle *h = handle; > + > + /* Trim is advisory; we don't care if it fails for anything other > + * than EIO or EPERM. */ > + r = fallocate (h->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, > + offset, count); > + if (r < 0) { > + if (errno != EPERM && errno != EIO) { > + nbdkit_debug ("ignoring failed fallocate during trim: %m"); > + r = 0; > + } > + else > + nbdkit_error ("fallocate: %m"); > + } > +#else > + /* Based on .can_trim, this should not be reached. */ > + errno = EOPNOTSUPP; > +#endif > + return r; > +} > + > static struct nbdkit_plugin plugin = { > .name = "file", > .longname = "nbdkit file plugin", > @@ -279,10 +318,12 @@ static struct nbdkit_plugin plugin = { > .open = file_open, > .close = file_close, > .get_size = file_get_size, > + .can_trim = file_can_trim, > .pread = file_pread, > .pwrite = file_pwrite, > - .zero = file_zero, > .flush = file_flush, > + .trim = file_trim, > + .zero = file_zero, > .errno_is_preserved = 1, > };ACK Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Seemingly Similar Threads
- [PATCH nbdkit v2 07/11] file: Implement NBDKIT_API_VERSION 2.
- [PATCH nbdkit 0/7] server: Implement NBD_FLAG_CAN_MULTI_CONN.
- [PATCH nbdkit v2 01/11] server: Implement NBD_FLAG_CAN_MULTI_CONN.
- Re: [PATCH 1/3] file: Avoid unsupported fallocate() calls
- [PATCH 1/3] file: Avoid unsupported fallocate() calls