[Libguestfs] [PATCH v2 nbdkit 5/6] Add truncate filter for truncating or extending the size of plugins.

[Richard W.M. Jones]

On Wed, Aug 01, 2018 at 11:06:51AM -0500, Eric Blake wrote:
> On 08/01/2018 06:10 AM, Richard W.M. Jones wrote:
> >This can truncate, extend, or round up/down to a multiple.
> >---
> >  common-rules.mk                               |   3 +-
> >  configure.ac                                  |   1 +
> >  filters/offset/nbdkit-offset-filter.pod       |   7 +-
> >  filters/partition/nbdkit-partition-filter.pod |   1 +
> >  filters/truncate/Makefile.am                  |  61 ++++
> >  filters/truncate/nbdkit-truncate-filter.pod   |  88 +++++
> >  filters/truncate/truncate.c                   | 301 ++++++++++++++++++
> >  7 files changed, 459 insertions(+), 3 deletions(-)
> >
> 
> >+
> >+=item *
> >+
> >+Round the size of the plugin up or down to the next multiple of C<N>.
> >+Use either C<round-up=N> or C<round-down=N>.
> 
> Worth mentioning that N must be a power of 2.
> 
> 
> >+
> >+#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
> >+
> >+/* These are the parameters. */
> >+static int64_t truncate = -1;
> >+static unsigned round_up = 0, round_down = 0;
> >+
> >+/* The real size of the underlying plugin. */
> >+static uint64_t real_size;
> >+
> >+/* The calculated size after applying the parameters. */
> >+static uint64_t size;
> >+
> >+/* This lock protects the real_size and size fields. */
> >+static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
> 
> Do we need the lock, or can we rely on the fact that .prepare will
> get called before anything else, and treat these as always
> initialized after that point?

No, because get_size could be called at any time (not in nbdkit core
currently, but a filter is permitted to call get_size on any
read/write operation through nextops, and in future nbdkit core might
call get_size more frequently).

> >+  /* The truncate, round-up and round-down parameters are treated as
> >+   * separate operations.  It's possible to specify more than one,
> >+   * although perhaps not very useful.
> >+   */
> >+  if (truncate >= 0)
> >+    size = truncate;
> >+  if (round_up > 0)
> >+    size = (size + round_up - 1) & ~(round_up-1);
> 
> Inconsistent spacing between the two instances of 'round_up - 1'; I
> think we favor the spaces everywhere...
>
> >+  if (round_down > 0)
> >+    size &= ~(round_down-1);
> 
> ...in which case the spacing here is also suspect.

Thanks, I'll push a simple fix later.

> >+  if (count > 0) {
> >+    /* The caller must be writing zeroes, else it's an error. */
> >+    if (!is_zero (buf, count)) {
> >+      nbdkit_error ("truncate: write beyond end of underlying device");
> >+      *err = EIO;
> 
> Would ENOSPC be better here?

Yes I think so - will fix.

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