[Libguestfs] [PATCH 1/2] New API: btrfs_balance_status

[Hu Tao]

On Mon, Feb 02, 2015 at 01:29:35PM +0000, Richard W.M. Jones wrote:
> On Mon, Feb 02, 2015 at 02:11:31PM +0800, Hu Tao wrote:
> [..]
> 
> So in fact there are a few problems.  I couldn't get btrfs balance to
> actually do anything at all (perhaps it doesn't work on empty
> filesystems), so this come only from code inspection.

Thou I can get btrfs-balance run, but it seems like a sync operation,
i.e. when it returns the balance operation has already completed.

I tested btrfs-balance-status by faking the "out", both normal ones and
malformed ones.

> 
> > +  lines = split_lines (out);
> > +  if (!lines)
> > +    return NULL;
> > +
> > +  ret = malloc(sizeof *ret);
> > +  if (ret == NULL) {
> > +    reply_with_perror ("malloc");
> > +    goto error;
> > +  }
> > +  memset (ret, 0, sizeof(*ret));
> > +
> > +  /* Output of `btrfs balance status' is like:
> > +   *
> > +   * running:
> > +   *
> > +   *   Balance on '/' is running
> > +   *   3 out of about 8 chunks balanced (3 considered), 62% left
> > +   *
> > +   * paused:
> > +   *
> > +   *   Balance on '/' is paused
> > +   *   3 out of about 8 chunks balanced (3 considered), 62% left
> > +   *
> > +   * no balance running:
> > +   *
> > +   *   No Balance found on '/'
> > +   *
> > +   */
> > +  if (strstr (lines[0], "No balance found on")) {
> 
> In case the output of the btrfs command changes in future, you need to
> check that the length of the lines[] array is >= 1 here, and >= 2
> below.  Otherwise this code will segfault.

Fixed. Also fixed in patch 2.

> 
> > +    ret->btrfsbalance_status = strdup("none");
> 
> Where you call strdup, you have to check that the return value is not
> NULL, and if it is call reply_with_perror ("strdup").

Fixed.

Thank you for reviewing!

Regards,
Hu