Change in vdsm[master]: gluster: Set mount path based on gluster volume info

nsoffer at redhat.com nsoffer at redhat.com
Tue Nov 10 15:53:07 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: gluster: Set mount path based on gluster volume info
......................................................................


Patch Set 5: Code-Review-1

(2 comments)

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

Line 336:             raise se.UnsupportedGlusterVolumeReplicaCountError(replicaCount)
Line 337: 
Line 338:     def _get_backup_servers_option(self):
Line 339:         servers = [brick.split(":")[0] for brick in self.volinfo['bricks']]
Line 340:         self.log.info("backup servers returned from volinfo are %s", servers)
This log is falsy, these are not the backup server, but the bricks. Also, keep log messages much shorter.

This could be nice if we use debug level, something like this:

    self.log.debug("Using bricks: %s", servers)
Line 341:         if self._volfileserver in servers:
Line 342:             servers.remove(self._volfileserver)
Line 343:         else:
Line 344:             self.log.warn("gluster server %r not in bricks, possibly mounting"


Line 340:         self.log.info("backup servers returned from volinfo are %s", servers)
Line 341:         if self._volfileserver in servers:
Line 342:             servers.remove(self._volfileserver)
Line 343:         else:
Line 344:             self.log.warn("gluster server %r not in bricks, possibly mounting"
I would like to see the list of bricks in this log message. When I see a warning in the log, I want to see all the relevant information in the warning, instead of hunting for it in the log.

The message about the bricks returned from gluster is a debug level message. This message should not be in the log in normal condition (we should use info level by default). So when I log a warning, the warning must include all the info I have.
Line 345:                           "duplicate servers", self._volfileserver)
Line 346: 
Line 347:         if not servers:
Line 348:             return ""


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3386b37cd407c52e1b8f38d54c236bffc143e2f
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: Sahina Bose <sabose at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list