[lxc-devel] [PATCH 5/6] Destroy bdevs using new bdev_destroy() from bdev.h

[Serge Hallyn]

Quoting Christian Brauner (christianvanbrauner at gmail.com):
> Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> 
>  100.0% src/lxc/
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 2103437..9f22fdc 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2213,21 +2213,6 @@ static int lxc_rmdir_onedev_wrapper(void *data)
>  	return lxc_rmdir_onedev(arg, "snaps");
>  }
>  
> -static int do_bdev_destroy(struct lxc_conf *conf)
> -{
> -	struct bdev *r;
> -	int ret = 0;
> -
> -	r = bdev_init(conf, conf->rootfs.path, conf->rootfs.mount, NULL);
> -	if (!r)
> -		return -1;
> -
> -	if (r->ops->destroy(r) < 0)
> -		ret = -1;
> -	bdev_put(r);
> -	return ret;
> -}
> -
>  static int bdev_destroy_wrapper(void *data)
>  {
>  	struct lxc_conf *conf = data;
> @@ -2242,13 +2227,16 @@ static int bdev_destroy_wrapper(void *data)
>  		ERROR("Failed to setuid to 0");
>  		return -1;
>  	}
> -	return do_bdev_destroy(conf);
> +	if (!bdev_destroy(conf))
> +		return -1;
> +	else
> +		return 0;
>  }
>  
>  static bool container_destroy(struct lxc_container *c)
>  {
>  	bool bret = false;
> -	int ret;
> +	int ret = 0;
>  	struct lxc_conf *conf;
>  
>  	if (!c || !do_lxcapi_is_defined(c))
> @@ -2304,8 +2292,8 @@ static bool container_destroy(struct lxc_container *c)
>  		if (am_unpriv())
>  			ret = userns_exec_1(conf, bdev_destroy_wrapper, conf);
>  		else
> -			ret = do_bdev_destroy(conf);
> -		if (ret < 0) {
> +			bret = bdev_destroy(conf);
> +		if (ret < 0 || !bret) {

I don't see how this can work - if the container is unprivileged, bret
will remain false as initialized, so an error will always be raised.

I think it's better to have a single return value from this if/else
block.  You could just do

	bret = userns_exec_1(conf, bdev_destroy_wrapper, conf) ? 0 : -1;

but it's probably nicer to just add a short static wrapper function
returning bool, something like

static bool do_destroy_container(struct lxc_container *conf) {
	if (am_unpriv()) {
		if (userns_exec_1(conf, bdev_destroy_wrapper, conf) < 0)
			return false;
		return true;
	}
	return bdev_destroy(conf);
}

>  			ERROR("Error destroying rootfs for %s", c->name);
>  			goto out;
>  		}
> -- 
> 2.5.1
>