On Fri, May 01, 2020 at 07:14:32PM +0200, François Revol wrote: > This allows to overlay bad sectors according to the mapfile generated by > ddrescue, to then see where sectors are used using fsck and trying to > copy files around. > > Signed-off-by: François Revol <re...@free.fr> ... > +nbdkit_ddrescue_filter_la_LDFLAGS = \ > + -module -avoid-version -shared \
We recently added $(SHARED_LDFLAGS) to the end of this line. Compare filters/ip/Makefile.am to your file. > diff --git a/filters/ddrescue/ddrescue.c b/filters/ddrescue/ddrescue.c > new file mode 100644 > index 00000000..a0e49e3c > --- /dev/null > +++ b/filters/ddrescue/ddrescue.c > @@ -0,0 +1,218 @@ > +/* nbdkit > + * Copyright (C) 2018-2020 Red Hat Inc. In several files and the man page you're assigning copyright over to Red Hat, but it's probably better to copyright them to yourself and/or your organisation. > +struct range { > + int64_t start; > + int64_t end; > + int64_t size; > + char status; > +}; > + > +struct mapfile { > + int ranges_count; > + struct range *ranges; > +}; > > +static struct mapfile map = { 0, NULL }; There are actually two generic types in common/ which might help here. There's a vector type for building lists: https://github.com/libguestfs/nbdkit/blob/master/common/utils/vector.h Example usage: https://github.com/libguestfs/nbdkit/blob/master/plugins/split/split.c And there's a regions type (built on top of vector) for building contiguous ranges covering a disk, which has a fast look-up method: https://github.com/libguestfs/nbdkit/blob/master/common/regions/regions.h Of course it's optional to use these but they will probably simplify your code. > + > + > +static void > +ddrescue_load (void) > +{ > +} Double blank line before this function, but also this function is empty so you don't need to include it (even though there is an .unload function - .unload will still be called even if .load is missing). > +/* We need this because otherwise the layer below can_write is called > + * and that might return true (eg. if the plugin has a pwrite method > + * at all), resulting in writes being passed through to the layer > + * below. This is possibly a bug in nbdkit. > + */ I think this is working as intended, not a bug. However it's good to comment on why the can_write function is needed. [...] > +=back > + > +=head1 VERSION > + > +C<nbdkit-ddrescue-filter> first appeared in nbdkit 1.21. It'll be 1.22 (next stable version after 1.20). > +=head1 SEE ALSO > + > +L<nbdkit(1)>, > +L<nbdkit-file-plugin(1)>, > +L<nbdkit-filter(3)>, > +L<ddrescue(1)>, > +L<https://www.gnu.org/software/ddrescue/manual/ddrescue_manual.html>. > + > +=head1 AUTHORS > + > +François Revol > + > +Richard W.M. Jones I guess the author is only you! > +=head1 COPYRIGHT > + > +Copyright (C) 2020 Red Hat Inc. See note about copyright above. It generally looks fine to me. 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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs