[libvirt] [PATCH] storage: prefer using newDef to save configfile

[Michal Privoznik]

On 07/10/2018 09:15 AM, Shichangkuo wrote:
> When re-defining an active storage pool, the configfile has not changed. This issue was introduced by bfcd8fc9,
> storage: Use virStoragePoolObjGetDef accessor for driver. So we prefer using newDef to save configfile.
> 
> Signed-off-by: Changkuo Shi <shi.changkuo at h3c.com>
> ---
>  src/storage/storage_driver.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

The commit message seems to have long lines. Also, next time please send
patch rebased to the latest HEAD. I had to go all the way down to v4.4.0
only to apply this patch cleanly.

> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index b0de96d..fab3c5b 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -844,13 +844,14 @@ storagePoolDefineXML(virConnectPtr conn,
> 
>      if (!(obj = virStoragePoolObjAssignDef(driver->pools, newDef)))
>          goto cleanup;
> -    newDef = NULL;
> +    newDef = virStoragePoolObjGetNewDef(obj);
>      def = virStoragePoolObjGetDef(obj);
> 
> -    if (virStoragePoolObjSaveDef(driver, obj, def) < 0) {
> +    if (virStoragePoolObjSaveDef(driver, obj, newDef ? newDef : def) < 0) {

Why wouldn't newDef be set at this point? It is always going to be a
non-NULL pointer.

Also, the rest of the function is a bit misleading now.

Long story short, I think the proper solution is to:

s/virStoragePoolObjGetDef/virStoragePoolObjGetNewDef/

actually and do nothing more.

Michal