Re: [PATCH v2] storage: Fix volStorageBackendRBDRefreshVolInfo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/29/19 12:12 AM, Yi Li wrote:
> Fix the return value status comparison checking for call to
> volStorageBackendRBDRefreshVolInfo introduced by commit id f46d137e.
> 
> we only should fail when the return is < 0. -ENOENT, -ETIMEDOUT will
> ignore according commit id f46d137e.
> 
> Signed-off-by: Yi Li <yili@xxxxxxxxxxx>
> ---
>  src/storage/storage_backend_rbd.c | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 

Sorry for the lack of response on this one. The change makes sense,
seems like that first commit was busted, not sure how it ever worked as
intended.

> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> index 7ce26ed..f7319d6 100644
> --- a/src/storage/storage_backend_rbd.c
> +++ b/src/storage/storage_backend_rbd.c
> @@ -411,6 +411,7 @@ volStorageBackendRBDGetFeatures(rbd_image_t image,
>      int r, ret = -1;
>  
>      if ((r = rbd_get_features(image, features)) < 0) {
> +        ret = r;
>          virReportSystemError(-r, _("failed to get the features of RBD image "
>                                   "%s"), volname);
>          goto cleanup;
> @@ -427,16 +428,19 @@ volStorageBackendRBDGetFlags(rbd_image_t image,
>                               const char *volname,
>                               uint64_t *flags)
>  {
> -    int rc;
> +    int r, ret = -1;
>  

Though this file uses 'int r' to hold intermediate function return
values, most libvirt code uses 'int rc'. Your change here to use 'int r'
is more consistent with the file, but inconsistent with the rest of libvirt.

Can you send a first patch that converts this whole file to use 'int rc'
in place of 'int r'? ('int ret' should be kept as is)

Then the actual changes in this patch will be a second patch on top. You
can CC me and I'll test and review those

Thanks,
Cole

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux