[libvirt] [PATCH v3 0/8] Make use of autofree within storage code

John Ferlan posted 8 patches 5 years, 2 months ago
Failed in applying to current master (apply log)
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(-)
[libvirt] [PATCH v3 0/8] Make use of autofree within storage code
Posted by John Ferlan 5 years, 2 months ago
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.

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(-)

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/8] Make use of autofree within storage code
Posted by John Ferlan 5 years, 2 months ago

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(-)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list