[libvirt] [PATCH v2 12/32] storage: Use VIR_AUTOFREE for storage backends

[Ján Tomko]

On Mon, Feb 11, 2019 at 10:14:56PM -0500, John Ferlan wrote:
>
>
>On 2/11/19 7:52 AM, Ján Tomko wrote:
>> On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote:
>>> Let's make use of the auto __cleanup capabilities. This also allows
>>> for the cleanup of some goto paths.
>>>
>>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>>> ---
>>> 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      | 30 +++-----
>>> src/storage/storage_backend_iscsi.c        | 73 +++++++-------------
>>> src/storage/storage_backend_iscsi_direct.c | 36 ++++------
>>> src/storage/storage_backend_logical.c      | 35 +++-------
>>> src/storage/storage_backend_mpath.c        | 18 ++---
>>> src/storage/storage_backend_rbd.c          | 35 +++-------
>>> src/storage/storage_backend_scsi.c         | 79 ++++++++--------------
>>> 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_gluster.c         | 16 ++---
>>> 14 files changed, 158 insertions(+), 319 deletions(-)
>>>
>
>[...]
>
>>> diff --git a/src/storage/storage_backend_scsi.c
>>> b/src/storage/storage_backend_scsi.c
>>> index 14f01f9ec0..7460349c81 100644
>>> --- a/src/storage/storage_backend_scsi.c
>>> +++ b/src/storage/storage_backend_scsi.c
>>> @@ -56,16 +56,14 @@ static int
>>> virStorageBackendSCSITriggerRescan(uint32_t host)
>>> {
>>>     int fd = -1;
>>> -    int retval = 0;
>>> -    char *path;
>>> +    int retval = -1;
>>
>> This inverts the logic of the function
>>
>>> +    VIR_AUTOFREE(char *) path = NULL;
>>>
>>>     VIR_DEBUG("Triggering rescan of host %d", host);
>>>
>>>     if (virAsprintf(&path, "%s/host%u/scan",
>>> -                    LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
>>> -        retval = -1;
>>> -        goto out;
>>> -    }
>>> +                    LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
>>> +        return -1;
>>>
>>>     VIR_DEBUG("Scan trigger path is '%s'", path);
>>>
>>> @@ -75,8 +73,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>>>         virReportSystemError(errno,
>>>                              _("Could not open '%s' to trigger host
>>> scan"),
>>>                              path);
>>
>>> -        retval = -1;
>>> -        goto free_path;
>>> +        goto cleanup;
>>
>> Unrelated rename. (There's no jump to 'cleanup' with fd != -1)
>>
>>>     }
>>>
>>>     if (safewrite(fd,
>>> @@ -86,13 +83,12 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>>>         virReportSystemError(errno,
>>>                              _("Write to '%s' to trigger host scan
>>> failed"),
>>>                              path);
>>> -        retval = -1;
>>
>> Before, this returned -1, now it will return 0.
>>
>>>     }
>>>
>>> +    retval = 0;
>>> +
>>> + cleanup:
>>>     VIR_FORCE_CLOSE(fd);
>>> - free_path:
>>> -    VIR_FREE(path);
>>> - out:
>>>     VIR_DEBUG("Rescan of host %d complete", host);
>>>     return retval;
>>> }
>>
>
>So two pre-patches are attached with any luck...
>

git send-email, please.

>John
>

>From 46e3b6fe94d68220c52e23e359d5b43a3f5cfbba Mon Sep 17 00:00:00 2001
>From: John Ferlan <jferlan at redhat.com>
>Date: Mon, 11 Feb 2019 21:48:53 -0500
>Subject: [PATCH 2/2] storage: Invert retval logic in
> virStorageBackendSCSITriggerRescan
>
>Rather than initialize to 0 and change to -1 on error, let's do the
>normal operation of initializing to -1 and set to 0 on success.
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
> src/storage/storage_backend_scsi.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko at redhat.com>

>diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
>index 85a177865f..591bcb04e2 100644
>--- a/src/storage/storage_backend_scsi.c
>+++ b/src/storage/storage_backend_scsi.c
>@@ -56,16 +56,14 @@ static int
> virStorageBackendSCSITriggerRescan(uint32_t host)
> {
>     int fd = -1;
>-    int retval = 0;
>+    int retval = -1;
>     char *path = NULL;
> 
>     VIR_DEBUG("Triggering rescan of host %d", host);
> 
>     if (virAsprintf(&path, "%s/host%u/scan",
>-                    LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
>-        retval = -1;
>+                    LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
>         goto cleanup;
>-    }
> 
>     VIR_DEBUG("Scan trigger path is '%s'", path);
> 
>@@ -75,7 +73,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>         virReportSystemError(errno,
>                              _("Could not open '%s' to trigger host scan"),
>                              path);
>-        retval = -1;
>         goto cleanup;
>     }
> 
>@@ -85,9 +82,11 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>         virReportSystemError(errno,
>                              _("Write to '%s' to trigger host scan failed"),
>                              path);
>-        retval = -1;
>+        goto cleanup;
>     }
> 
>+    retval = 0;
>+
>  cleanup:
>     VIR_FORCE_CLOSE(fd);
>     VIR_FREE(path);
>-- 
>2.20.1
>

>From e7e2aa6a7fb00a1207e9a5a52596fb4ec3ffce8b Mon Sep 17 00:00:00 2001
>From: John Ferlan <jferlan at redhat.com>
>Date: Mon, 11 Feb 2019 21:46:28 -0500
>Subject: [PATCH 1/2] src: Fix label logic in
> virStorageBackendSCSITriggerRescan
>
>Let's initialize @path to NULL, then rather than use two labels
>free_path and out labels, let's use the cleanup: label to call
>VIR_FREE(path); and VIR_FORCE_CLOSE(fd);
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
> src/storage/storage_backend_scsi.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190212/134721cd/attachment-0001.sig>