On 2/12/19 9:18 AM, John Ferlan wrote:
v2: https://www.redhat.com/archives/libvir-list/2019-February/msg00477.html
Changes since v2:
* Pushed what was already agreed upon w/ R-by's, so in order to apply this you'll need to get all those changes.
* Repost extractions of code that were requested in code review, but not formally R-by'd (patches 1-5). NB: Although Erik did agree in principle to what was changed for patch5, I figured posting would be better than assuming.
* (NEW) patches 6 & 7 are a result of comment from v2 patch1 where it was noted that @authdef could have been overwritten
* Patch8 is R-by'd from Jano; however, Erik has noted a failure for src/util/virstoragefile.c to build on MinGW. Neither of us have any idea what the failure is, so I'll leave this in the series to at least get through patches 1-7.
"A" thought to resolve this issue is to remove the 'inline' from the VIR_DEFINE_AUTOPTR_FUNC definition. I have seen this before from a review of LXC code:
https://www.redhat.com/archives/libvir-list/2019-January/msg00975.html
where the "fix" for my environment was to remove the "inline". While I assume that'd work here, it's still not clear why src/conf changes are fine, but src/util changes are not.
Other than src/util/virstoragefile.c, the rest of this series is now pushed. Maybe someone else understands the build system well enough in the MinGW world to help figure out why other modules seem to be fine, but the VIR_AUTOPTR(virStorageSource) fails. Maybe it's just virStorageFileMetadataNew? Thanks for the reviews - John
John Ferlan (8): storage: Cleanup virStorageFileBackendGlusterReadlinkCallback storage: Use VIR_AUTOFREE for storage backends storage: Rework ret logic in storageBackendUpdateVolTargetInfo storage: Use VIR_AUTOCLOSE tests: Fix memory leak in testCompareXMLToArgvFiles conf: Check for duplicate authdef during hostdev iSCSI processing util: Check for duplicated id in virStorageSourceParseRBDColonString util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource
src/conf/domain_conf.c | 9 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 9 +- src/qemu/qemu_migration.c | 3 +- src/storage/storage_backend.c | 9 +- src/storage/storage_backend_disk.c | 62 +++---- src/storage/storage_backend_fs.c | 17 +- src/storage/storage_backend_gluster.c | 33 ++-- src/storage/storage_backend_iscsi.c | 72 +++----- src/storage/storage_backend_iscsi_direct.c | 36 ++-- src/storage/storage_backend_logical.c | 35 ++-- src/storage/storage_backend_mpath.c | 17 +- src/storage/storage_backend_rbd.c | 35 ++-- src/storage/storage_backend_scsi.c | 77 +++------ src/storage/storage_backend_sheepdog.c | 27 +-- src/storage/storage_backend_vstorage.c | 25 +-- src/storage/storage_backend_zfs.c | 15 +- src/storage/storage_file_fs.c | 15 +- src/storage/storage_file_gluster.c | 16 +- src/storage/storage_util.c | 182 ++++++++------------ src/util/virstoragefile.c | 185 +++++++++------------ src/util/virstoragefile.h | 1 + tests/qemublocktest.c | 6 +- tests/storagepoolxml2argvtest.c | 13 +- tests/virstoragetest.c | 50 +++--- 25 files changed, 349 insertions(+), 603 deletions(-)