[v2] Make use of autofree within storage code

[libvirt] [PATCH v2 14/32] util: Fix virStorageBackendSCSINewLun error handling

Posted by John Ferlan 7 years ago

Commit a523770c3 added @retval return processing for
virStorageBackendUpdateVolInfo in order to allow a -2
to be return; however, upon successful completion
@retval = 0 and if either the virStorageBackendSCSISerial
or the virStoragePoolObjAddVol failed, the method would
return 0, but not add the @vol to the pool. So let's
just reset retval = -1 and continue processing.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/storage/storage_util.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index fa364941c5..e4913b3405 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3869,6 +3869,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
                                                  VIR_STORAGE_VOL_READ_NOERROR)) < 0)
         goto cleanup;
 
+    /* Reset retval for proper error handling */
+    retval = -1;
+
     vol->key = virStorageBackendSCSISerial(vol->target.path,
                                            (def->source.adapter.type ==
                                             VIR_STORAGE_ADAPTER_TYPE_FC_HOST));
-- 
2.20.1

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

Re: [libvirt] [PATCH v2 14/32] util: Fix virStorageBackendSCSINewLun error handling

Posted by Erik Skultety 6 years, 12 months ago

On Fri, Feb 08, 2019 at 01:37:08PM -0500, John Ferlan wrote:
> Commit a523770c3 added @retval return processing for
> virStorageBackendUpdateVolInfo in order to allow a -2
> to be return; however, upon successful completion
> @retval = 0 and if either the virStorageBackendSCSISerial
> or the virStoragePoolObjAddVol failed, the method would
> return 0, but not add the @vol to the pool. So let's
> just reset retval = -1 and continue processing.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>

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

Re: [libvirt] [PATCH v2 14/32] util: Fix virStorageBackendSCSINewLun error handling

Posted by Ján Tomko 6 years, 12 months ago

On Fri, Feb 08, 2019 at 01:37:08PM -0500, John Ferlan wrote:
>Commit a523770c3 added @retval return processing for
>virStorageBackendUpdateVolInfo in order to allow a -2
>to be return; however, upon successful completion
>@retval = 0 and if either the virStorageBackendSCSISerial
>or the virStoragePoolObjAddVol failed, the method would
>return 0, but not add the @vol to the pool. So let's
>just reset retval = -1 and continue processing.
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/storage/storage_util.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>index fa364941c5..e4913b3405 100644
>--- a/src/storage/storage_util.c
>+++ b/src/storage/storage_util.c
>@@ -3869,6 +3869,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>                                                  VIR_STORAGE_VOL_READ_NOERROR)) < 0)
>         goto cleanup;
>
>+    /* Reset retval for proper error handling */
>+    retval = -1;
>+

The neater fix is not to mess with the implied return value in
the first place:

if ((rc = UpdateVolInfo()) < 0) {
    retval = rc;
    goto cleanup;
}

But this goes away in the next patch, so:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

>     vol->key = virStorageBackendSCSISerial(vol->target.path,
>                                            (def->source.adapter.type ==
>                                             VIR_STORAGE_ADAPTER_TYPE_FC_HOST));
>-- 
>2.20.1
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list