[Libguestfs] [PATCH] WIP: ddrescue mapfile filter
François Revol
revol at free.fr
Fri May 1 19:15:21 UTC 2020
Hi,
Le 01/05/2020 à 19:40, Richard W.M. Jones a écrit :
> 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 <revol at 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.
Ah, ok.
Btw, -shared is not always the correct flag. We had to change it from
-nostart in Haiku because of compatibility, but BeOS used the later.
>> 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.
Ok, I wasn't sure about that, not that I really mind for such a small patch.
>
>> +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.
Ah, I looked at sparse stuff only…
I guess if someone ever wants to add write support it would help, but it
works well enough as is for me.
>> +
>> +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).
Ok, wasn't sure.
> It generally looks fine to me.
I'll send a v2 then.
François.
More information about the Libguestfs
mailing list