Re: [Libguestfs] [PATCH 1/2] New API: btrfs_balance_status
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