On 3/13/15 8:43 AM, Brian Foster wrote:
> On Thu, Mar 05, 2015 at 03:26:32PM -0600, Eric Sandeen wrote:
>> Being able to write corrupt data is handy if we wish to test
>> repair against specific types of corruptions.
>>
>> Add a "-c" option to the write command which allows this.
>>
>> Note that this also skips CRC updates; it's not currently possible
>> to write invalid data with a valid CRC; CRC recalculation is
>> intertwined with validation.
>>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> ---
>>
>> diff --git a/db/crc.c b/db/crc.c
>> index ad46c3f..861086a 100644
>> --- a/db/crc.c
>> +++ b/db/crc.c
>
> This depends on the previous crc command which isn't merged yet. It
> might be worth it to include that again here so it hopefully gets picked
> up.
>
>> @@ -58,13 +58,6 @@ crc_help(void)
>>
>> }
>>
>> -void
>> -xfs_dummy_verify(
>> - struct xfs_buf *bp)
>> -{
>> - return;
>> -}
>> -
>> static int
>> crc_f(
>> int argc,
>> diff --git a/db/crc.h b/db/crc.h
>> index 80ecec3..9f44615 100644
>> --- a/db/crc.h
>> +++ b/db/crc.h
>> @@ -20,4 +20,3 @@ struct field;
>>
>> extern void crc_init(void);
>> extern void crc_struct(const field_t *fields, int argc, char **argv);
>> -extern void xfs_dummy_verify(struct xfs_buf *bp);
>> diff --git a/db/io.c b/db/io.c
>> index eb3daa1..2d18d56 100644
>> --- a/db/io.c
>> +++ b/db/io.c
>> @@ -458,6 +458,13 @@ write_cur_bbs(void)
>> }
>>
>> void
>> +xfs_dummy_verify(
>> + struct xfs_buf *bp)
>> +{
>> + return;
>> +}
>> +
>> +void
>> write_cur(void)
>> {
>> int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
>> diff --git a/db/io.h b/db/io.h
>> index 71082e6..31d96b4 100644
>> --- a/db/io.h
>> +++ b/db/io.h
>> @@ -63,6 +63,7 @@ extern void set_cur(const struct typ *t, __int64_t
>> d, int c, int ring_add,
>> bbmap_t *bbmap);
>> extern void ring_add(void);
>> extern void set_iocur_type(const struct typ *t);
>> +extern void xfs_dummy_verify(struct xfs_buf *bp);
>>
>> /*
>> * returns -1 for unchecked, 0 for bad and 1 for good
>> diff --git a/db/write.c b/db/write.c
>> index a0f14f4..a01c25a 100644
>> --- a/db/write.c
>> +++ b/db/write.c
>> @@ -38,7 +38,7 @@ static int write_f(int argc, char **argv);
>> static void write_help(void);
>>
>> static const cmdinfo_t write_cmd =
>> - { "write", NULL, write_f, 0, -1, 0, N_("[field or value]..."),
>> + { "write", NULL, write_f, 0, -1, 0, N_("[-c] [field or value]..."),
>> N_("write value to disk"), write_help };
>>
>> void
>> @@ -79,6 +79,7 @@ write_help(void)
>> " String mode: 'write \"This_is_a_filename\" - write null terminated
>> string.\n"
>> "\n"
>> " In data mode type 'write' by itself for a list of specific commands.\n\n"
>> +" Specifying the -c option will allow writes of invalid (corrupt) data.\n\n"
>> ));
>>
>> }
>> @@ -90,6 +91,9 @@ write_f(
>> {
>> pfunc_t pf;
>> extern char *progname;
>> + int c;
>> + int corrupt = 0; /* Allow write of corrupt data; skip
>> verification */
>> + const struct xfs_buf_ops *stashed_ops = NULL;
>
> Trailing space here.
>
>>
>> if (x.isreadonly & LIBXFS_ISREADONLY) {
>> dbprintf(_("%s started in read only mode, writing disabled\n"),
>> @@ -109,12 +113,36 @@ write_f(
>> return 0;
>> }
>>
>> - /* move past the "write" command */
>> - argc--;
>> - argv++;
>> + if (argc) while ((c = getopt(argc, argv, "c")) != EOF) {
>
> Isn't argc always going to be non-zero at this point (e.g., argv[0] ==
> "write")?
Hm, yeah, not sure if that was a thinko or a typo, thanks.
(hm, db's bmap_f does the same thing, I might have just blindly copied that)
>> + if (corrupt) {
>> + struct xfs_buf_ops nowrite_ops;
>> +
>
> This may not be an explicit failure, but I'd rather not define this on
> the stack in the local scope of this branch and then implicitly use it
> outside of that scope. Could we move the definition and possibility the
> initialization to the top of the function?
*nod* I'll fix that too.
I'll send the crc patch & this in another 2-patch series (though I am wondering
if the crc patch is needed, if we're able to write bad data; it kind of achieves
the same goal - thoughts?)
-Eric
|