[libvirt] [PATCH] storage: Fix volStorageBackendRBDRefreshVolInfo
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@winhong.com> --- src/storage/storage_backend_rbd.c | 40 ++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 7ce26ed..1551cf4 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; - if ((rc = rbd_get_flags(image, flags)) < 0) { - virReportSystemError(-rc, + if ((r = rbd_get_flags(image, flags)) < 0) { + ret = r; + virReportSystemError(-r, _("failed to get the flags of RBD image %s"), volname); - return -1; + goto cleanup; } + ret = 0; - return 0; + cleanup: + return ret; } static bool @@ -470,6 +474,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol, if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1, &virStorageBackendRBDRefreshVolInfoCb, &allocation)) < 0) { + ret = r; virReportSystemError(-r, _("failed to iterate RBD image '%s'"), vol->name); goto cleanup; @@ -525,24 +530,28 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, uint64_t flags; if ((r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { - ret = -r; + ret = r; virReportSystemError(-r, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { - ret = -r; + ret = r; virReportSystemError(-r, _("failed to stat the RBD image '%s'"), vol->name); goto cleanup; } - if (volStorageBackendRBDGetFeatures(image, vol->name, &features) < 0) + if ((r = volStorageBackendRBDGetFeatures(image, vol->name, &features)) < 0) { + ret = r; goto cleanup; + } - if (volStorageBackendRBDGetFlags(image, vol->name, &flags) < 0) + if ((r = volStorageBackendRBDGetFlags(image, vol->name, &flags)) < 0) { + ret = r; goto cleanup; + } vol->target.capacity = info.size; vol->type = VIR_STORAGE_VOL_NETWORK; @@ -555,8 +564,11 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, "Querying for actual allocation", def->source.name, vol->name); - if (virStorageBackendRBDSetAllocation(vol, image, &info) < 0) + if ((r = virStorageBackendRBDSetAllocation(vol, image, &info) < 0)) { + ret = r; goto cleanup; + } + } else { vol->target.allocation = info.obj_size * info.num_objs; } @@ -734,12 +746,10 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) * * Do not error out and simply ignore the volume */ - if (r < 0) { - if (r == -ENOENT || r == -ETIMEDOUT) - continue; - + if (r == -ENOENT || r == -ETIMEDOUT) + continue; + else if (r < 0) goto cleanup; - } if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; -- 2.7.5
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@winhong.com> --- src/storage/storage_backend_rbd.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) 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; - if ((rc = rbd_get_flags(image, flags)) < 0) { - virReportSystemError(-rc, + if ((r = rbd_get_flags(image, flags)) < 0) { + ret = r; + virReportSystemError(-r, _("failed to get the flags of RBD image %s"), volname); - return -1; + goto cleanup; } + ret = 0; - return 0; + cleanup: + return ret; } static bool @@ -470,6 +474,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol, if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1, &virStorageBackendRBDRefreshVolInfoCb, &allocation)) < 0) { + ret = r; virReportSystemError(-r, _("failed to iterate RBD image '%s'"), vol->name); goto cleanup; @@ -525,24 +530,28 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, uint64_t flags; if ((r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { - ret = -r; + ret = r; virReportSystemError(-r, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { - ret = -r; + ret = r; virReportSystemError(-r, _("failed to stat the RBD image '%s'"), vol->name); goto cleanup; } - if (volStorageBackendRBDGetFeatures(image, vol->name, &features) < 0) + if ((r = volStorageBackendRBDGetFeatures(image, vol->name, &features)) < 0) { + ret = r; goto cleanup; + } - if (volStorageBackendRBDGetFlags(image, vol->name, &flags) < 0) + if ((r = volStorageBackendRBDGetFlags(image, vol->name, &flags)) < 0) { + ret = r; goto cleanup; + } vol->target.capacity = info.size; vol->type = VIR_STORAGE_VOL_NETWORK; @@ -555,8 +564,11 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, "Querying for actual allocation", def->source.name, vol->name); - if (virStorageBackendRBDSetAllocation(vol, image, &info) < 0) + if ((r = virStorageBackendRBDSetAllocation(vol, image, &info) < 0)) { + ret = r; goto cleanup; + } + } else { vol->target.allocation = info.obj_size * info.num_objs; } -- 2.7.5
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@winhong.com> --- 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
most libvirt code uses 'int rc' to hold intermediate function return values. consistent with the rest of libvirt. Signed-off-by: Yi Li <yili@winhong.com> --- src/storage/storage_backend_rbd.c | 202 ++++++++++++++++++-------------------- 1 file changed, 96 insertions(+), 106 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 34088f7..81f7cd5 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -184,8 +184,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, virStoragePoolDefPtr def) { - int ret = -1; - int r = 0; + int rc, ret = -1; virStoragePoolSourcePtr source = &def->source; virStorageAuthDefPtr authdef = source->auth; unsigned char *secret_value = NULL; @@ -203,8 +202,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, if (authdef) { VIR_DEBUG("Using cephx authorization, username: %s", authdef->username); - if ((r = rados_create(&ptr->cluster, authdef->username)) < 0) { - virReportSystemError(-r, "%s", _("failed to initialize RADOS")); + if ((rc = rados_create(&ptr->cluster, authdef->username)) < 0) { + virReportSystemError(-rc, "%s", _("failed to initialize RADOS")); goto cleanup; } @@ -319,8 +318,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, } ptr->starttime = time(0); - if ((r = rados_connect(ptr->cluster)) < 0) { - virReportSystemError(-r, _("failed to connect to the RADOS monitor on: %s"), + if ((rc = rados_connect(ptr->cluster)) < 0) { + virReportSystemError(-rc, _("failed to connect to the RADOS monitor on: %s"), mon_buff); goto cleanup; } @@ -340,12 +339,12 @@ virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - int r = rados_ioctx_create(ptr->cluster, def->source.name, &ptr->ioctx); - if (r < 0) { - virReportSystemError(-r, _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), + int rc = rados_ioctx_create(ptr->cluster, def->source.name, &ptr->ioctx); + if (rc < 0) { + virReportSystemError(-rc, _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), def->source.name); } - return r; + return rc; } static void @@ -408,10 +407,10 @@ volStorageBackendRBDGetFeatures(rbd_image_t image, const char *volname, uint64_t *features) { - int r; + int rc; - if ((r = rbd_get_features(image, features)) < 0) { - virReportSystemError(-r, _("failed to get the features of RBD image " + if ((rc = rbd_get_features(image, features)) < 0) { + virReportSystemError(-rc, _("failed to get the features of RBD image " "%s"), volname); return -1; } @@ -462,13 +461,13 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol, rbd_image_t *image, rbd_image_info_t *info) { - int r; + int rc; size_t allocation = 0; - if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1, + if ((rc = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1, &virStorageBackendRBDRefreshVolInfoCb, &allocation)) < 0) { - virReportSystemError(-r, _("failed to iterate RBD image '%s'"), + virReportSystemError(-rc, _("failed to iterate RBD image '%s'"), vol->name); return -1; } @@ -512,24 +511,23 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStoragePoolObjPtr pool, virStorageBackendRBDStatePtr ptr) { - int ret = -1; + int rc, ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - int r = 0; rbd_image_t image = NULL; rbd_image_info_t info; uint64_t features; uint64_t flags; - if ((r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { - ret = -r; - virReportSystemError(-r, _("failed to open the RBD image '%s'"), + if ((rc = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { + ret = -rc; + virReportSystemError(-rc, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } - if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { - ret = -r; - virReportSystemError(-r, _("failed to stat the RBD image '%s'"), + if ((rc = rbd_stat(image, &info, sizeof(info))) < 0) { + ret = -rc; + virReportSystemError(-rc, _("failed to stat the RBD image '%s'"), vol->name); goto cleanup; } @@ -672,8 +670,7 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr) static int virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) { - int ret = -1; - int r = 0; + int rc, ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; struct rados_cluster_stat_t clusterstat; @@ -684,13 +681,13 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; - if ((r = rados_cluster_stat(ptr->cluster, &clusterstat)) < 0) { - virReportSystemError(-r, "%s", _("failed to stat the RADOS cluster")); + if ((rc = rados_cluster_stat(ptr->cluster, &clusterstat)) < 0) { + virReportSystemError(-rc, "%s", _("failed to stat the RADOS cluster")); goto cleanup; } - if ((r = rados_ioctx_pool_stat(ptr->ioctx, &poolstat)) < 0) { - virReportSystemError(-r, _("failed to stat the RADOS pool '%s'"), + if ((rc = rados_ioctx_pool_stat(ptr->ioctx, &poolstat)) < 0) { + virReportSystemError(-rc, _("failed to stat the RADOS pool '%s'"), def->source.name); goto cleanup; } @@ -715,7 +712,7 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) vol->name = g_steal_pointer(&names[i]); - r = volStorageBackendRBDRefreshVolInfo(vol, pool, ptr); + rc = volStorageBackendRBDRefreshVolInfo(vol, pool, ptr); /* It could be that a volume has been deleted through a different route * then libvirt and that will cause a -ENOENT to be returned. @@ -726,8 +723,8 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) * * Do not error out and simply ignore the volume */ - if (r < 0) { - if (r == -ENOENT || r == -ETIMEDOUT) + if (rc < 0) { + if (rc == -ENOENT || rc == -ETIMEDOUT) continue; goto cleanup; @@ -754,16 +751,15 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, virStoragePoolSourcePtr source, virStorageVolDefPtr vol) { - int ret = -1; - int r = 0; + int rc, ret = -1; int max_snaps = 128; int snap_count, protected; size_t i; rbd_image_t image = NULL; g_autofree rbd_snap_info_t *snaps = NULL; - if ((r = rbd_open(ioctx, vol->name, &image, NULL)) < 0) { - virReportSystemError(-r, _("failed to open the RBD image '%s'"), + if ((rc = rbd_open(ioctx, vol->name, &image, NULL)) < 0) { + virReportSystemError(-rc, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } @@ -782,8 +778,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, source->name, vol->name); for (i = 0; i < snap_count; i++) { - if ((r = rbd_snap_is_protected(image, snaps[i].name, &protected)) < 0) { - virReportSystemError(-r, _("failed to verify if snapshot '%s/%s@%s' is protected"), + if ((rc = rbd_snap_is_protected(image, snaps[i].name, &protected)) < 0) { + virReportSystemError(-rc, _("failed to verify if snapshot '%s/%s@%s' is protected"), source->name, vol->name, snaps[i].name); goto cleanup; @@ -794,8 +790,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, "unprotected", source->name, vol->name, snaps[i].name); - if ((r = rbd_snap_unprotect(image, snaps[i].name)) < 0) { - virReportSystemError(-r, _("failed to unprotect snapshot '%s/%s@%s'"), + if ((rc = rbd_snap_unprotect(image, snaps[i].name)) < 0) { + virReportSystemError(-rc, _("failed to unprotect snapshot '%s/%s@%s'"), source->name, vol->name, snaps[i].name); goto cleanup; @@ -805,8 +801,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, VIR_DEBUG("Removing snapshot %s/%s@%s", source->name, vol->name, snaps[i].name); - if ((r = rbd_snap_remove(image, snaps[i].name)) < 0) { - virReportSystemError(-r, _("failed to remove snapshot '%s/%s@%s'"), + if ((rc = rbd_snap_remove(image, snaps[i].name)) < 0) { + virReportSystemError(-rc, _("failed to remove snapshot '%s/%s@%s'"), source->name, vol->name, snaps[i].name); goto cleanup; @@ -830,8 +826,7 @@ virStorageBackendRBDDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { - int ret = -1; - int r = 0; + int rc, ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; @@ -854,9 +849,9 @@ virStorageBackendRBDDeleteVol(virStoragePoolObjPtr pool, VIR_DEBUG("Removing volume %s/%s", def->source.name, vol->name); - r = rbd_remove(ptr->ioctx, vol->name); - if (r < 0 && (-r) != ENOENT) { - virReportSystemError(-r, _("failed to remove volume '%s/%s'"), + rc = rbd_remove(ptr->ioctx, vol->name); + if (rc < 0 && (-rc) != ENOENT) { + virReportSystemError(-rc, _("failed to remove volume '%s/%s'"), def->source.name, vol->name); goto cleanup; } @@ -906,8 +901,7 @@ virStorageBackendRBDBuildVol(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; - int ret = -1; - int r = 0; + int rc, ret = -1; VIR_DEBUG("Creating RBD image %s/%s with size %llu", def->source.name, vol->name, vol->target.capacity); @@ -935,9 +929,9 @@ virStorageBackendRBDBuildVol(virStoragePoolObjPtr pool, if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; - if ((r = virStorageBackendRBDCreateImage(ptr->ioctx, vol->name, + if ((rc = virStorageBackendRBDCreateImage(ptr->ioctx, vol->name, vol->target.capacity)) < 0) { - virReportSystemError(-r, _("failed to create volume '%s/%s'"), + virReportSystemError(-rc, _("failed to create volume '%s/%s'"), def->source.name, vol->name); goto cleanup; } @@ -956,11 +950,11 @@ virStorageBackendRBDImageInfo(rbd_image_t image, uint64_t *stripe_unit, uint64_t *stripe_count) { - int r = 0; + int rc = 0; uint8_t oldformat; - if ((r = rbd_get_old_format(image, &oldformat)) < 0) { - virReportSystemError(-r, _("failed to get the format of RBD image %s"), + if ((rc = rbd_get_old_format(image, &oldformat)) < 0) { + virReportSystemError(-rc, _("failed to get the format of RBD image %s"), volname); return -1; } @@ -976,14 +970,14 @@ virStorageBackendRBDImageInfo(rbd_image_t image, if (volStorageBackendRBDGetFeatures(image, volname, features) < 0) return -1; - if ((r = rbd_get_stripe_unit(image, stripe_unit)) < 0) { - virReportSystemError(-r, _("failed to get the stripe unit of RBD image %s"), + if ((rc = rbd_get_stripe_unit(image, stripe_unit)) < 0) { + virReportSystemError(-rc, _("failed to get the stripe unit of RBD image %s"), volname); return -1; } - if ((r = rbd_get_stripe_count(image, stripe_count)) < 0) { - virReportSystemError(-r, _("failed to get the stripe count of RBD image %s"), + if ((rc = rbd_get_stripe_count(image, stripe_count)) < 0) { + virReportSystemError(-rc, _("failed to get the stripe count of RBD image %s"), volname); return -1; } @@ -1015,8 +1009,7 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, char *imgname, virBufferPtr snapname) { - int r = -1; - int ret = -1; + int rc, ret = -1; int snap_count; int max_snaps = 128; size_t i; @@ -1024,8 +1017,8 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, rbd_image_info_t info; g_autofree rbd_snap_info_t *snaps = NULL; - if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { - virReportSystemError(-r, _("failed to stat the RBD image %s"), + if ((rc = rbd_stat(image, &info, sizeof(info))) < 0) { + virReportSystemError(-rc, _("failed to stat the RBD image %s"), imgname); goto cleanup; } @@ -1063,15 +1056,15 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, * is available */ #if LIBRBD_VERSION_CODE > 265 - r = rbd_diff_iterate2(image, snaps[i].name, 0, info.size, 0, 1, + rc = rbd_diff_iterate2(image, snaps[i].name, 0, info.size, 0, 1, virStorageBackendRBDIterateCb, (void *)&diff); #else - r = rbd_diff_iterate(image, snaps[i].name, 0, info.size, + rc = rbd_diff_iterate(image, snaps[i].name, 0, info.size, virStorageBackendRBDIterateCb, (void *)&diff); #endif - if (r < 0) { - virReportSystemError(-r, _("failed to iterate RBD snapshot %s@%s"), + if (rc < 0) { + virReportSystemError(-rc, _("failed to iterate RBD snapshot %s@%s"), imgname, snaps[i].name); goto cleanup; } @@ -1103,12 +1096,12 @@ virStorageBackendRBDSnapshotCreate(rbd_image_t image, char *imgname, char *snapname) { - int r = -1; + int rc = -1; VIR_DEBUG("Creating RBD snapshot %s@%s", imgname, snapname); - if ((r = rbd_snap_create(image, snapname)) < 0) { - virReportSystemError(-r, _("failed to create RBD snapshot %s@%s"), + if ((rc = rbd_snap_create(image, snapname)) < 0) { + virReportSystemError(-rc, _("failed to create RBD snapshot %s@%s"), imgname, snapname); return -1; } @@ -1121,13 +1114,13 @@ virStorageBackendRBDSnapshotProtect(rbd_image_t image, char *imgname, char *snapname) { - int r = -1; + int rc = -1; int protected; VIR_DEBUG("Querying if RBD snapshot %s@%s is protected", imgname, snapname); - if ((r = rbd_snap_is_protected(image, snapname, &protected)) < 0) { - virReportSystemError(-r, _("failed to verify if RBD snapshot %s@%s " + if ((rc = rbd_snap_is_protected(image, snapname, &protected)) < 0) { + virReportSystemError(-rc, _("failed to verify if RBD snapshot %s@%s " "is protected"), imgname, snapname); return -1; } @@ -1136,8 +1129,8 @@ virStorageBackendRBDSnapshotProtect(rbd_image_t image, VIR_DEBUG("RBD Snapshot %s@%s is not protected, protecting", imgname, snapname); - if ((r = rbd_snap_protect(image, snapname)) < 0) { - virReportSystemError(-r, _("failed to protect RBD snapshot %s@%s"), + if ((rc = rbd_snap_protect(image, snapname)) < 0) { + virReportSystemError(-rc, _("failed to protect RBD snapshot %s@%s"), imgname, snapname); return -1; } @@ -1153,8 +1146,7 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, char *origvol, char *newvol) { - int r = -1; - int ret = -1; + int rc, ret = -1; int order = 0; uint64_t features; uint64_t stripe_count; @@ -1163,8 +1155,8 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, rbd_image_t image = NULL; g_autofree char *snapname_buff = NULL; - if ((r = rbd_open(io, origvol, &image, NULL)) < 0) { - virReportSystemError(-r, _("failed to open the RBD image %s"), + if ((rc = rbd_open(io, origvol, &image, NULL)) < 0) { + virReportSystemError(-rc, _("failed to open the RBD image %s"), origvol); goto cleanup; } @@ -1211,14 +1203,14 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, * RBD snapshots have to be 'protected' before they can be used * as a parent snapshot for a child image */ - if ((r = virStorageBackendRBDSnapshotProtect(image, origvol, snapname_buff)) < 0) + if ((rc = virStorageBackendRBDSnapshotProtect(image, origvol, snapname_buff)) < 0) goto cleanup; VIR_DEBUG("Performing RBD clone from %s to %s", origvol, newvol); - if ((r = rbd_clone2(io, origvol, snapname_buff, io, newvol, features, + if ((rc = rbd_clone2(io, origvol, snapname_buff, io, newvol, features, &order, stripe_unit, stripe_count)) < 0) { - virReportSystemError(-r, _("failed to clone RBD volume %s to %s"), + virReportSystemError(-rc, _("failed to clone RBD volume %s to %s"), origvol, newvol); goto cleanup; } @@ -1293,22 +1285,21 @@ virStorageBackendRBDResizeVol(virStoragePoolObjPtr pool, { virStorageBackendRBDStatePtr ptr = NULL; rbd_image_t image = NULL; - int ret = -1; - int r = 0; + int rc, ret = -1; virCheckFlags(0, -1); if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; - if ((r = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { - virReportSystemError(-r, _("failed to open the RBD image '%s'"), + if ((rc = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { + virReportSystemError(-rc, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } - if ((r = rbd_resize(image, capacity)) < 0) { - virReportSystemError(-r, _("failed to resize the RBD image '%s'"), + if ((rc = rbd_resize(image, capacity)) < 0) { + virReportSystemError(-rc, _("failed to resize the RBD image '%s'"), vol->name); goto cleanup; } @@ -1328,7 +1319,7 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, rbd_image_info_t *info, uint64_t stripe_count) { - int r = -1; + int rc; unsigned long long offset = 0; unsigned long long length; g_autofree char *writebuf = NULL; @@ -1339,8 +1330,8 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, while (offset < info->size) { length = MIN((info->size - offset), (info->obj_size * stripe_count)); - if ((r = rbd_write(image, offset, length, writebuf)) < 0) { - virReportSystemError(-r, _("writing %llu bytes failed on " + if ((rc = rbd_write(image, offset, length, writebuf)) < 0) { + virReportSystemError(-rc, _("writing %llu bytes failed on " "RBD image %s at offset %llu"), length, imgname, offset); return -1; @@ -1361,7 +1352,7 @@ virStorageBackendRBDVolWipeDiscard(rbd_image_t image, rbd_image_info_t *info, uint64_t stripe_count) { - int r = -1; + int rc = -1; unsigned long long offset = 0; unsigned long long length; @@ -1370,8 +1361,8 @@ virStorageBackendRBDVolWipeDiscard(rbd_image_t image, while (offset < info->size) { length = MIN((info->size - offset), (info->obj_size * stripe_count)); - if ((r = rbd_discard(image, offset, length)) < 0) { - virReportSystemError(-r, _("discarding %llu bytes failed on " + if ((rc = rbd_discard(image, offset, length)) < 0) { + virReportSystemError(-rc, _("discarding %llu bytes failed on " "RBD image %s at offset %llu"), length, imgname, offset); return -1; @@ -1397,8 +1388,7 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, rbd_image_t image = NULL; rbd_image_info_t info; uint64_t stripe_count; - int r = -1; - int ret = -1; + int rc, ret = -1; virCheckFlags(0, -1); @@ -1411,20 +1401,20 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, if (!ptr) goto cleanup; - if ((r = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { - virReportSystemError(-r, _("failed to open the RBD image %s"), + if ((rc = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { + virReportSystemError(-rc, _("failed to open the RBD image %s"), vol->name); goto cleanup; } - if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { - virReportSystemError(-r, _("failed to stat the RBD image %s"), + if ((rc = rbd_stat(image, &info, sizeof(info))) < 0) { + virReportSystemError(-rc, _("failed to stat the RBD image %s"), vol->name); goto cleanup; } - if ((r = rbd_get_stripe_count(image, &stripe_count)) < 0) { - virReportSystemError(-r, _("failed to get stripe count of RBD image %s"), + if ((rc = rbd_get_stripe_count(image, &stripe_count)) < 0) { + virReportSystemError(-rc, _("failed to get stripe count of RBD image %s"), vol->name); goto cleanup; } @@ -1434,11 +1424,11 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, switch ((virStorageVolWipeAlgorithm) algorithm) { case VIR_STORAGE_VOL_WIPE_ALG_ZERO: - r = virStorageBackendRBDVolWipeZero(image, vol->name, + rc = virStorageBackendRBDVolWipeZero(image, vol->name, &info, stripe_count); break; case VIR_STORAGE_VOL_WIPE_ALG_TRIM: - r = virStorageBackendRBDVolWipeDiscard(image, vol->name, + rc = virStorageBackendRBDVolWipeDiscard(image, vol->name, &info, stripe_count); break; case VIR_STORAGE_VOL_WIPE_ALG_NNSA: @@ -1455,8 +1445,8 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, goto cleanup; } - if (r < 0) { - virReportSystemError(-r, _("failed to wipe RBD image %s"), + if (rc < 0) { + virReportSystemError(-rc, _("failed to wipe RBD image %s"), vol->name); goto cleanup; } -- 2.7.5
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@winhong.com> --- src/storage/storage_backend_rbd.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 81f7cd5..5f14156 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -412,7 +412,7 @@ volStorageBackendRBDGetFeatures(rbd_image_t image, if ((rc = rbd_get_features(image, features)) < 0) { virReportSystemError(-rc, _("failed to get the features of RBD image " "%s"), volname); - return -1; + return rc; } return 0; @@ -430,7 +430,7 @@ volStorageBackendRBDGetFlags(rbd_image_t image, virReportSystemError(-rc, _("failed to get the flags of RBD image %s"), volname); - return -1; + return rc; } return 0; @@ -469,7 +469,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol, &allocation)) < 0) { virReportSystemError(-rc, _("failed to iterate RBD image '%s'"), vol->name); - return -1; + return rc; } VIR_DEBUG("Found %zu bytes allocated for RBD image %s", @@ -519,24 +519,28 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, uint64_t flags; if ((rc = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { - ret = -rc; + ret = rc; virReportSystemError(-rc, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } if ((rc = rbd_stat(image, &info, sizeof(info))) < 0) { - ret = -rc; + ret = rc; virReportSystemError(-rc, _("failed to stat the RBD image '%s'"), vol->name); goto cleanup; } - if (volStorageBackendRBDGetFeatures(image, vol->name, &features) < 0) + if ((rc = volStorageBackendRBDGetFeatures(image, vol->name, &features)) < 0) { + ret = rc; goto cleanup; + } - if (volStorageBackendRBDGetFlags(image, vol->name, &flags) < 0) + if ((rc = volStorageBackendRBDGetFlags(image, vol->name, &flags)) < 0) { + ret = rc; goto cleanup; + } vol->target.capacity = info.size; vol->type = VIR_STORAGE_VOL_NETWORK; @@ -549,8 +553,10 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, "Querying for actual allocation", def->source.name, vol->name); - if (virStorageBackendRBDSetAllocation(vol, image, &info) < 0) + if ((rc = virStorageBackendRBDSetAllocation(vol, image, &info)) < 0) { + ret = rc; goto cleanup; + } } else { vol->target.allocation = info.obj_size * info.num_objs; } -- 2.7.5
Use errno parameter in virReportSystemError. Remove hold function return values if don't need. Signed-off-by: Yi Li <yili@winhong.com> --- src/storage/storage_backend_rbd.c | 159 ++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 83 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5f14156..78a8e95 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -184,7 +184,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, virStoragePoolDefPtr def) { - int rc, ret = -1; + int ret = -1; virStoragePoolSourcePtr source = &def->source; virStorageAuthDefPtr authdef = source->auth; unsigned char *secret_value = NULL; @@ -202,8 +202,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, if (authdef) { VIR_DEBUG("Using cephx authorization, username: %s", authdef->username); - if ((rc = rados_create(&ptr->cluster, authdef->username)) < 0) { - virReportSystemError(-rc, "%s", _("failed to initialize RADOS")); + if (rados_create(&ptr->cluster, authdef->username) < 0) { + virReportSystemError(errno, "%s", _("failed to initialize RADOS")); goto cleanup; } @@ -318,8 +318,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, } ptr->starttime = time(0); - if ((rc = rados_connect(ptr->cluster)) < 0) { - virReportSystemError(-rc, _("failed to connect to the RADOS monitor on: %s"), + if (rados_connect(ptr->cluster) < 0) { + virReportSystemError(errno, _("failed to connect to the RADOS monitor on: %s"), mon_buff); goto cleanup; } @@ -341,7 +341,7 @@ virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int rc = rados_ioctx_create(ptr->cluster, def->source.name, &ptr->ioctx); if (rc < 0) { - virReportSystemError(-rc, _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), + virReportSystemError(errno, _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), def->source.name); } return rc; @@ -410,7 +410,7 @@ volStorageBackendRBDGetFeatures(rbd_image_t image, int rc; if ((rc = rbd_get_features(image, features)) < 0) { - virReportSystemError(-rc, _("failed to get the features of RBD image " + virReportSystemError(errno, _("failed to get the features of RBD image " "%s"), volname); return rc; } @@ -427,7 +427,7 @@ volStorageBackendRBDGetFlags(rbd_image_t image, int rc; if ((rc = rbd_get_flags(image, flags)) < 0) { - virReportSystemError(-rc, + virReportSystemError(errno, _("failed to get the flags of RBD image %s"), volname); return rc; @@ -467,7 +467,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol, if ((rc = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1, &virStorageBackendRBDRefreshVolInfoCb, &allocation)) < 0) { - virReportSystemError(-rc, _("failed to iterate RBD image '%s'"), + virReportSystemError(errno, _("failed to iterate RBD image '%s'"), vol->name); return rc; } @@ -520,14 +520,14 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, if ((rc = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { ret = rc; - virReportSystemError(-rc, _("failed to open the RBD image '%s'"), + virReportSystemError(errno, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } if ((rc = rbd_stat(image, &info, sizeof(info))) < 0) { ret = rc; - virReportSystemError(-rc, _("failed to stat the RBD image '%s'"), + virReportSystemError(errno, _("failed to stat the RBD image '%s'"), vol->name); goto cleanup; } @@ -600,7 +600,7 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr) if (rc >= 0) break; if (rc != -ERANGE) { - virReportSystemError(-rc, "%s", _("Unable to list RBD images")); + virReportSystemError(errno, "%s", _("Unable to list RBD images")); goto error; } } @@ -641,7 +641,7 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr) if (rc >= 0) break; if (rc != -ERANGE) { - virReportSystemError(-rc, "%s", _("Unable to list RBD images")); + virReportSystemError(errno, "%s", _("Unable to list RBD images")); goto error; } VIR_FREE(namebuf); @@ -687,13 +687,13 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; - if ((rc = rados_cluster_stat(ptr->cluster, &clusterstat)) < 0) { - virReportSystemError(-rc, "%s", _("failed to stat the RADOS cluster")); + if (rados_cluster_stat(ptr->cluster, &clusterstat) < 0) { + virReportSystemError(errno, "%s", _("failed to stat the RADOS cluster")); goto cleanup; } - if ((rc = rados_ioctx_pool_stat(ptr->ioctx, &poolstat)) < 0) { - virReportSystemError(-rc, _("failed to stat the RADOS pool '%s'"), + if (rados_ioctx_pool_stat(ptr->ioctx, &poolstat) < 0) { + virReportSystemError(errno, _("failed to stat the RADOS pool '%s'"), def->source.name); goto cleanup; } @@ -757,15 +757,15 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, virStoragePoolSourcePtr source, virStorageVolDefPtr vol) { - int rc, ret = -1; + int ret = -1; int max_snaps = 128; int snap_count, protected; size_t i; rbd_image_t image = NULL; g_autofree rbd_snap_info_t *snaps = NULL; - if ((rc = rbd_open(ioctx, vol->name, &image, NULL)) < 0) { - virReportSystemError(-rc, _("failed to open the RBD image '%s'"), + if (rbd_open(ioctx, vol->name, &image, NULL) < 0) { + virReportSystemError(errno, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } @@ -784,8 +784,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, source->name, vol->name); for (i = 0; i < snap_count; i++) { - if ((rc = rbd_snap_is_protected(image, snaps[i].name, &protected)) < 0) { - virReportSystemError(-rc, _("failed to verify if snapshot '%s/%s@%s' is protected"), + if (rbd_snap_is_protected(image, snaps[i].name, &protected) < 0) { + virReportSystemError(errno, _("failed to verify if snapshot '%s/%s@%s' is protected"), source->name, vol->name, snaps[i].name); goto cleanup; @@ -796,8 +796,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, "unprotected", source->name, vol->name, snaps[i].name); - if ((rc = rbd_snap_unprotect(image, snaps[i].name)) < 0) { - virReportSystemError(-rc, _("failed to unprotect snapshot '%s/%s@%s'"), + if (rbd_snap_unprotect(image, snaps[i].name) < 0) { + virReportSystemError(errno, _("failed to unprotect snapshot '%s/%s@%s'"), source->name, vol->name, snaps[i].name); goto cleanup; @@ -807,8 +807,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, VIR_DEBUG("Removing snapshot %s/%s@%s", source->name, vol->name, snaps[i].name); - if ((rc = rbd_snap_remove(image, snaps[i].name)) < 0) { - virReportSystemError(-rc, _("failed to remove snapshot '%s/%s@%s'"), + if (rbd_snap_remove(image, snaps[i].name) < 0) { + virReportSystemError(errno, _("failed to remove snapshot '%s/%s@%s'"), source->name, vol->name, snaps[i].name); goto cleanup; @@ -857,7 +857,7 @@ virStorageBackendRBDDeleteVol(virStoragePoolObjPtr pool, rc = rbd_remove(ptr->ioctx, vol->name); if (rc < 0 && (-rc) != ENOENT) { - virReportSystemError(-rc, _("failed to remove volume '%s/%s'"), + virReportSystemError(errno, _("failed to remove volume '%s/%s'"), def->source.name, vol->name); goto cleanup; } @@ -907,7 +907,7 @@ virStorageBackendRBDBuildVol(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; - int rc, ret = -1; + int ret = -1; VIR_DEBUG("Creating RBD image %s/%s with size %llu", def->source.name, vol->name, vol->target.capacity); @@ -935,9 +935,9 @@ virStorageBackendRBDBuildVol(virStoragePoolObjPtr pool, if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; - if ((rc = virStorageBackendRBDCreateImage(ptr->ioctx, vol->name, - vol->target.capacity)) < 0) { - virReportSystemError(-rc, _("failed to create volume '%s/%s'"), + if (virStorageBackendRBDCreateImage(ptr->ioctx, vol->name, + vol->target.capacity) < 0) { + virReportSystemError(errno, _("failed to create volume '%s/%s'"), def->source.name, vol->name); goto cleanup; } @@ -956,11 +956,10 @@ virStorageBackendRBDImageInfo(rbd_image_t image, uint64_t *stripe_unit, uint64_t *stripe_count) { - int rc = 0; uint8_t oldformat; - if ((rc = rbd_get_old_format(image, &oldformat)) < 0) { - virReportSystemError(-rc, _("failed to get the format of RBD image %s"), + if (rbd_get_old_format(image, &oldformat) < 0) { + virReportSystemError(errno, _("failed to get the format of RBD image %s"), volname); return -1; } @@ -976,14 +975,14 @@ virStorageBackendRBDImageInfo(rbd_image_t image, if (volStorageBackendRBDGetFeatures(image, volname, features) < 0) return -1; - if ((rc = rbd_get_stripe_unit(image, stripe_unit)) < 0) { - virReportSystemError(-rc, _("failed to get the stripe unit of RBD image %s"), + if (rbd_get_stripe_unit(image, stripe_unit) < 0) { + virReportSystemError(errno, _("failed to get the stripe unit of RBD image %s"), volname); return -1; } - if ((rc = rbd_get_stripe_count(image, stripe_count)) < 0) { - virReportSystemError(-rc, _("failed to get the stripe count of RBD image %s"), + if (rbd_get_stripe_count(image, stripe_count) < 0) { + virReportSystemError(errno, _("failed to get the stripe count of RBD image %s"), volname); return -1; } @@ -1015,7 +1014,7 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, char *imgname, virBufferPtr snapname) { - int rc, ret = -1; + int ret = -1; int snap_count; int max_snaps = 128; size_t i; @@ -1023,8 +1022,8 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, rbd_image_info_t info; g_autofree rbd_snap_info_t *snaps = NULL; - if ((rc = rbd_stat(image, &info, sizeof(info))) < 0) { - virReportSystemError(-rc, _("failed to stat the RBD image %s"), + if (rbd_stat(image, &info, sizeof(info)) < 0) { + virReportSystemError(errno, _("failed to stat the RBD image %s"), imgname); goto cleanup; } @@ -1062,15 +1061,13 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, * is available */ #if LIBRBD_VERSION_CODE > 265 - rc = rbd_diff_iterate2(image, snaps[i].name, 0, info.size, 0, 1, - virStorageBackendRBDIterateCb, (void *)&diff); + if (rbd_diff_iterate2(image, snaps[i].name, 0, info.size, 0, 1, + virStorageBackendRBDIterateCb, (void *)&diff) < 0) { #else - rc = rbd_diff_iterate(image, snaps[i].name, 0, info.size, - virStorageBackendRBDIterateCb, (void *)&diff); + if (rbd_diff_iterate(image, snaps[i].name, 0, info.size, + virStorageBackendRBDIterateCb, (void *)&diff) < 0) { #endif - - if (rc < 0) { - virReportSystemError(-rc, _("failed to iterate RBD snapshot %s@%s"), + virReportSystemError(errno, _("failed to iterate RBD snapshot %s@%s"), imgname, snaps[i].name); goto cleanup; } @@ -1102,12 +1099,10 @@ virStorageBackendRBDSnapshotCreate(rbd_image_t image, char *imgname, char *snapname) { - int rc = -1; - VIR_DEBUG("Creating RBD snapshot %s@%s", imgname, snapname); - if ((rc = rbd_snap_create(image, snapname)) < 0) { - virReportSystemError(-rc, _("failed to create RBD snapshot %s@%s"), + if (rbd_snap_create(image, snapname) < 0) { + virReportSystemError(errno, _("failed to create RBD snapshot %s@%s"), imgname, snapname); return -1; } @@ -1120,13 +1115,12 @@ virStorageBackendRBDSnapshotProtect(rbd_image_t image, char *imgname, char *snapname) { - int rc = -1; int protected; VIR_DEBUG("Querying if RBD snapshot %s@%s is protected", imgname, snapname); - if ((rc = rbd_snap_is_protected(image, snapname, &protected)) < 0) { - virReportSystemError(-rc, _("failed to verify if RBD snapshot %s@%s " + if (rbd_snap_is_protected(image, snapname, &protected) < 0) { + virReportSystemError(errno, _("failed to verify if RBD snapshot %s@%s " "is protected"), imgname, snapname); return -1; } @@ -1135,8 +1129,8 @@ virStorageBackendRBDSnapshotProtect(rbd_image_t image, VIR_DEBUG("RBD Snapshot %s@%s is not protected, protecting", imgname, snapname); - if ((rc = rbd_snap_protect(image, snapname)) < 0) { - virReportSystemError(-rc, _("failed to protect RBD snapshot %s@%s"), + if (rbd_snap_protect(image, snapname) < 0) { + virReportSystemError(errno, _("failed to protect RBD snapshot %s@%s"), imgname, snapname); return -1; } @@ -1152,7 +1146,7 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, char *origvol, char *newvol) { - int rc, ret = -1; + int ret = -1; int order = 0; uint64_t features; uint64_t stripe_count; @@ -1161,8 +1155,8 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, rbd_image_t image = NULL; g_autofree char *snapname_buff = NULL; - if ((rc = rbd_open(io, origvol, &image, NULL)) < 0) { - virReportSystemError(-rc, _("failed to open the RBD image %s"), + if (rbd_open(io, origvol, &image, NULL) < 0) { + virReportSystemError(errno, _("failed to open the RBD image %s"), origvol); goto cleanup; } @@ -1209,14 +1203,14 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, * RBD snapshots have to be 'protected' before they can be used * as a parent snapshot for a child image */ - if ((rc = virStorageBackendRBDSnapshotProtect(image, origvol, snapname_buff)) < 0) + if (virStorageBackendRBDSnapshotProtect(image, origvol, snapname_buff) < 0) goto cleanup; VIR_DEBUG("Performing RBD clone from %s to %s", origvol, newvol); - if ((rc = rbd_clone2(io, origvol, snapname_buff, io, newvol, features, - &order, stripe_unit, stripe_count)) < 0) { - virReportSystemError(-rc, _("failed to clone RBD volume %s to %s"), + if (rbd_clone2(io, origvol, snapname_buff, io, newvol, features, + &order, stripe_unit, stripe_count) < 0) { + virReportSystemError(errno, _("failed to clone RBD volume %s to %s"), origvol, newvol); goto cleanup; } @@ -1291,21 +1285,21 @@ virStorageBackendRBDResizeVol(virStoragePoolObjPtr pool, { virStorageBackendRBDStatePtr ptr = NULL; rbd_image_t image = NULL; - int rc, ret = -1; + int ret = -1; virCheckFlags(0, -1); if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; - if ((rc = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { - virReportSystemError(-rc, _("failed to open the RBD image '%s'"), + if (rbd_open(ptr->ioctx, vol->name, &image, NULL) < 0) { + virReportSystemError(errno, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } - if ((rc = rbd_resize(image, capacity)) < 0) { - virReportSystemError(-rc, _("failed to resize the RBD image '%s'"), + if (rbd_resize(image, capacity) < 0) { + virReportSystemError(errno, _("failed to resize the RBD image '%s'"), vol->name); goto cleanup; } @@ -1325,7 +1319,6 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, rbd_image_info_t *info, uint64_t stripe_count) { - int rc; unsigned long long offset = 0; unsigned long long length; g_autofree char *writebuf = NULL; @@ -1336,8 +1329,8 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, while (offset < info->size) { length = MIN((info->size - offset), (info->obj_size * stripe_count)); - if ((rc = rbd_write(image, offset, length, writebuf)) < 0) { - virReportSystemError(-rc, _("writing %llu bytes failed on " + if (rbd_write(image, offset, length, writebuf) < 0) { + virReportSystemError(errno, _("writing %llu bytes failed on " "RBD image %s at offset %llu"), length, imgname, offset); return -1; @@ -1358,7 +1351,6 @@ virStorageBackendRBDVolWipeDiscard(rbd_image_t image, rbd_image_info_t *info, uint64_t stripe_count) { - int rc = -1; unsigned long long offset = 0; unsigned long long length; @@ -1367,8 +1359,8 @@ virStorageBackendRBDVolWipeDiscard(rbd_image_t image, while (offset < info->size) { length = MIN((info->size - offset), (info->obj_size * stripe_count)); - if ((rc = rbd_discard(image, offset, length)) < 0) { - virReportSystemError(-rc, _("discarding %llu bytes failed on " + if (rbd_discard(image, offset, length) < 0) { + virReportSystemError(errno, _("discarding %llu bytes failed on " "RBD image %s at offset %llu"), length, imgname, offset); return -1; @@ -1394,7 +1386,8 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, rbd_image_t image = NULL; rbd_image_info_t info; uint64_t stripe_count; - int rc, ret = -1; + int rc = 0; + int ret = -1; virCheckFlags(0, -1); @@ -1407,20 +1400,20 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, if (!ptr) goto cleanup; - if ((rc = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { - virReportSystemError(-rc, _("failed to open the RBD image %s"), + if (rbd_open(ptr->ioctx, vol->name, &image, NULL) < 0) { + virReportSystemError(errno, _("failed to open the RBD image %s"), vol->name); goto cleanup; } - if ((rc = rbd_stat(image, &info, sizeof(info))) < 0) { - virReportSystemError(-rc, _("failed to stat the RBD image %s"), + if (rbd_stat(image, &info, sizeof(info)) < 0) { + virReportSystemError(errno, _("failed to stat the RBD image %s"), vol->name); goto cleanup; } - if ((rc = rbd_get_stripe_count(image, &stripe_count)) < 0) { - virReportSystemError(-rc, _("failed to get stripe count of RBD image %s"), + if (rbd_get_stripe_count(image, &stripe_count) < 0) { + virReportSystemError(errno, _("failed to get stripe count of RBD image %s"), vol->name); goto cleanup; } @@ -1452,7 +1445,7 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, } if (rc < 0) { - virReportSystemError(-rc, _("failed to wipe RBD image %s"), + virReportSystemError(errno, _("failed to wipe RBD image %s"), vol->name); goto cleanup; } -- 2.7.5
On 12/22/19 8:15 PM, Yi Li wrote:
most libvirt code uses 'int rc' to hold intermediate function return values. consistent with the rest of libvirt.
Signed-off-by: Yi Li <yili@winhong.com> --- src/storage/storage_backend_rbd.c | 202 ++++++++++++++++++-------------------- 1 file changed, 96 insertions(+), 106 deletions(-)
patch #2 had a hard tab in it, which made syntax-check fail. I fixed it and pushed this series. Please remember to check 'make check' and 'make syntax-check' pass on every patch before sending Thanks, Cole
On Tue, Dec 24, 2019 at 3:32 AM Cole Robinson <crobinso@redhat.com> wrote:
On 12/22/19 8:15 PM, Yi Li wrote:
most libvirt code uses 'int rc' to hold intermediate function return values. consistent with the rest of libvirt.
Signed-off-by: Yi Li <yili@winhong.com> --- src/storage/storage_backend_rbd.c | 202 ++++++++++++++++++-------------------- 1 file changed, 96 insertions(+), 106 deletions(-)
patch #2 had a hard tab in it, which made syntax-check fail. I fixed it and pushed this series. Please remember to check 'make check' and 'make syntax-check' pass on every patch before sending
Got it. Thanks . Thanks, YiLi
participants (3)
-
Cole Robinson -
Yi Li -
李义