Change in vdsm[master]: gluster: Don't fail connect server when getting volume info

nsoffer at redhat.com nsoffer at redhat.com
Mon Feb 29 20:28:11 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: gluster: Don't fail connect server when getting volume info
......................................................................


Patch Set 5:

(3 comments)

https://gerrit.ovirt.org/#/c/53785/5/vdsm/storage/storageServer.py
File vdsm/storage/storageServer.py:

Line 346:                              "automatically mount gluster storage backup "
Line 347:                              "servers")
Line 348:             return
Line 349: 
Line 350:         if self.volinfo is not None:
This will invoke get_gluster_volinfo once...
Line 351:             replicaCount = self.volinfo['replicaCount']
Line 352:             if replicaCount not in self.ALLOWED_REPLICA_COUNTS:
Line 353:                 self.log.warning("Unsupported replica count (%s) for volume "
Line 354:                                  " %r, please upgrade volume to replica 3",


Line 354:                                  " %r, please upgrade volume to replica 3",
Line 355:                                  replicaCount, self._volname)
Line 356: 
Line 357:     def _get_backup_servers_option(self):
Line 358:         if self.volinfo is None:
This will invoke _get_gluster_volinfo() again..
Line 359:             return
Line 360: 
Line 361:         servers = utils.unique(brick.split(":")[0] for brick
Line 362:                                in self.volinfo['bricks'])


Line 380:                                                        self._volfileserver)
Line 381:             return volinfo[self._volname]
Line 382:         except ge.GlusterCmdExecFailedException as e:
Line 383:             self.log.warning("Failed to get volume info. %s",
Line 384:                              e)
This should never return None  - the logic of the lazy initialization is based on this fact that this either return the info or raises, so we invoke it once.

We can solve this by returning empty volume info dict, e.g. one with empty bricks list.

Do we have a test simulating this failure? Please start by adding such tests, then we can fix the code so the test pass.
Line 385: 
Line 386: 
Line 387: class NFSConnection(object):
Line 388:     DEFAULT_OPTIONS = ["soft", "nosharecache"]


-- 
To view, visit https://gerrit.ovirt.org/53785
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e1835b7de4c5c4c5c4616d3c36f15c1f91a01dc
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list