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@xxxxxxx> > --- > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list