Chris Lalancette wrote:
Cole Robinson wrote:
> There's a null dereference in the storage driver when defining a pool.
> Attached patch fixes it for me.
>
> diff --git a/src/storage_driver.c b/src/storage_driver.c
> index 2432a9a..ac5e443 100644
> --- a/src/storage_driver.c
> +++ b/src/storage_driver.c
> @@ -546,7 +546,7 @@ storagePoolDefine(virConnectPtr conn,
> goto cleanup;
> def = NULL;
>
> - if (virStoragePoolObjSaveDef(conn, driver, pool, def) < 0) {
> + if (virStoragePoolObjSaveDef(conn, driver, pool, pool->def) < 0) {
> virStoragePoolObjRemove(&driver->pools, pool);
> goto cleanup;
> }
>
>
Hm, I definitely see what you are getting at, and why the crash is there, but
I'm not sure this is totally correct. virStoragePoolObjAssignDef() only assigns
this def to pool->def iff the storage pool is not running. That means that if
the pool is running, and we make this change, for running pools you would always
get the old def, not the updated one. I think we need to move the "def =
NULL"
further down, but that of course will require other changes so we still have the
unified cleanup target.
Ahh, I didn't realize the AssignDef caveat. Thanks for pointing that
out. Something like this should work then?
- Cole