Bug 1298024 - Incorrectly set variable 'r' which is reported as errno for rbd_snap_is_protected and rbd_snap_unprotect
Incorrectly set variable 'r' which is reported as errno for rbd_snap_is_prote...
Status: CLOSED NEXTRELEASE
Product: Virtualization Tools
Classification: Community
Component: libvirt (Show other bugs)
unspecified
x86_64 Linux
unspecified Severity unspecified
: ---
: ---
Assigned To: Libvirt Maintainers
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2016-01-12 21:49 EST by yangyang
Modified: 2016-01-18 08:08 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-01-18 08:08:26 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description yangyang 2016-01-12 21:49:18 EST
Description of problem:
In virStorageBackendRBDCleanupSnapshots function, '-r' is
reported as an errno for rbd_snap_is_protected and rbd_snap_unprotect
error, but it was set only on the rbd_open call, so it doesn't correspond
to rbd_snap_is_protected and rbd_snap_unprotect error. It causes
incorrect error

src/storage/storage_backend_rbd.c

static int virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx,
virStoragePoolSourcePtr source,
virStorageVolDefPtr vol)

int r = 0;
int max_snaps = 128;
int snap_count, protected;
size_t i;
rbd_snap_info_t *snaps = NULL;
rbd_image_t image = NULL;

r = rbd_open(ioctx, vol->name, &image, NULL);
if (r < 0) {
virReportSystemError(-r, _("failed to open the RBD image '%s'"),
vol->name);
goto cleanup;
}

if (rbd_snap_is_protected(image, snaps[i].name, &protected)) {
virReportSystemError(-r, _("failed to verify if snapshot '%s/%s@%s' is protected"),
source->name, vol->name,
snaps[i].name);
goto cleanup;
}

if (protected == 1) {
VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be "
"unprotected", source->name, vol->name,
snaps[i].name);

if (rbd_snap_unprotect(image, snaps[i].name) < 0) {
virReportSystemError(-r, _("failed to unprotect snapshot '%s/%s@%s'"),
source->name, vol->name,
snaps[i].name);
goto cleanup;
}
}

Version-Release number of selected component (if applicable):
libvirt-1.3.1-1.el7.x86_64
# git describe
v1.3.1-rc1

How reproducible:
100%

Steps to Reproduce:
1. prepare a rbd pool with source 'yy'
# virsh pool-dumpxml rbd
<pool type='rbd'>
  <name>rbd</name>
  <uuid>ebda974a-4fb7-4af2-b1a0-7a94e5cdda98</uuid>
  <capacity unit='bytes'>152820314112</capacity>
  <allocation unit='bytes'>16957992</allocation>
  <available unit='bytes'>131244736512</available>
  <source>
    <host name='10.66.111.149'/>
    <name>yy</name>
  </source>
</pool>

create a vol
# virsh vol-create-as rbd vol1 1G
Vol vol1 created

2. clone a rbd snapshot
# rbd snap create yy/vol1@sn1
# rbd snap protect yy/vol1@sn1
# rbd clone yy/vol1@sn1 rbd/vol1-clone
# rbd children yy/vol1@sn1
rbd/vol1-clone

3. prepare another rbd pool with source rbd
# virsh pool-dumpxml rbdautotest
<pool type='rbd'>
  <name>rbdautotest</name>
  <uuid>942dd90f-8258-4ffc-8563-df989406a9f5</uuid>
  <capacity unit='bytes'>152820314112</capacity>
  <allocation unit='bytes'>40</allocation>
  <available unit='bytes'>131283779584</available>
  <source>
    <host name='10.66.110.36'/>
    <name>rbd</name>
  </source>
</pool>

# virsh vol-list rbdautotest
 Name                 Path                                    
------------------------------------------------------------------------------
 vol1-clone           rbd/vol1-clone                          
 vol100               rbd/vol100

# virsh vol-delete vol1 rbd --delete-snapshots
error: Failed to delete vol vol1
error: failed to unprotect snapshot 'yy/vol1@sn1': Success

Actual result

Expected result
Set 'r' correctly

Actual results:


Expected results:


Additional info:
Comment 1 Ján Tomko 2016-01-18 08:08:26 EST
Fixed upstream by:
commit a5a383adc17f708204fdaeb3dead2a4f3cebf4fd
Author:     Wido den Hollander <wido@widodh.nl>
CommitDate: 2016-01-18 14:06:24 +0100

    rbd: Set r variable so it can be returned should an error occur
    
    This was reported in bug #1298024 where r would be filled with the
    return code of rbd_open().
    
    Should rbd_snap_unprotect() fail for any reason the virReportSystemError
    call would return 'Success' since rbd_open() succeeded.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1298024
    Signed-off-by: Wido den Hollander <wido@widodh.nl>

git describe: v1.3.1-6-ga5a383a

Note You need to log in before you can comment on or make changes to this bug.