[Libguestfs] [PATCH v3 2/2] New API: btfs_scrub_status
Chen, Hanxiao
chenhanxiao at cn.fujitsu.com
Fri Feb 13 09:22:54 UTC 2015
> -----Original Message-----
> From: Richard W.M. Jones [mailto:rjones at redhat.com]
> Sent: Wednesday, February 11, 2015 10:04 PM
> To: Chen, Hanxiao/陈 晗霄
> Cc: libguestfs at redhat.com
> Subject: Re: [Libguestfs] [PATCH v3 2/2] New API: btfs_scrub_status
>
> On Wed, Feb 11, 2015 at 06:41:20PM +0800, Chen Hanxiao wrote:
> > + /* Output of `btrfs scrub -R status' is like:
> > + *
> > + * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
> > + * scrub started at Mon Feb 2 17:39:38 2015, running for 93 seconds
> > + * data_extents_scrubbed: 136670
> > + * tree_extents_scrubbed: 30023
> > + * data_bytes_scrubbed: 4474441728
> > + * tree_bytes_scrubbed: 491896832
> > + * read_errors: 0
> > + * csum_errors: 0
> > + * verify_errors: 0
> > + * no_csum: 17760
> > + * csum_discards: 197622
> > + * super_errors: 0
> > + * malloc_errors: 0
> > + * uncorrectable_errors: 0
> > + * unverified_errors: 0
> > + * corrected_errors: 0
> > + * last_physical: 10301341696
> > + *
> > + * or:
> > + *
> > + * scrub status for 346121d1-1847-40f8-9b7b-2bf3d539c68f
> > + * no stats available
> > + */
> > + for (i = 1; lines[i] != NULL; ++i) {
> > + if ((i == 1) && STREQ (lines[i], "\tno stats available"))
> > + return ret;
> > + else if ((i == 2) && sscanf (lines[i], "\tdata_extents_scrubbed: %" SCNu64,
> > + &ret->btrfsscrub_data_extents_scrubbed) != 1)
> > + goto error;
> > + else if ((i == 3) && sscanf (lines[i], "\ttree_extents_scrubbed: %" SCNu64,
> > + &ret->btrfsscrub_tree_extents_scrubbed) != 1)
> > + goto error;
> [etc]
>
> In my previous comment[1], I said it's better to turn this into a
> loop, and that's what you've done.
>
> [1] https://www.redhat.com/archives/libguestfs/2015-February/msg00008.html
>
> Unfortunately testing `i == 2' etc means the loop isn't useful. The
> code is effectively the same as the old code. It's still fragile if
> the output of scrub status changes slightly.
>
> I suggest removing the `i == X' tests.
>
Will do.
Thanks,
- Chen
> Rest of the patch looks OK.
>
> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> Fedora Windows cross-compiler. Compile Windows programs, test, and
> build Windows installers. Over 100 libraries supported.
> http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list