Re: [Libguestfs] [PATCH v3 2/2] New API: btfs_scrub_status
-----Original Message-----
From: Richard W.M. Jones [mailto:rjones@redhat.com]
Sent: Wednesday, February 11, 2015 10:04 PM
To: Chen, Hanxiao/陈 晗霄
Cc: libguestfs(a)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