Change in vdsm[master]: sp: fix spm start when failing to produce domain

laravot at redhat.com laravot at redhat.com
Sun Mar 23 08:49:11 UTC 2014


Liron Ar has posted comments on this change.

Change subject: sp: fix spm start when failing to produce domain
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.ovirt.org/#/c/25424/2/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 207:                 if domain.getDomainRole() == sd.REGULAR_DOMAIN:
Line 208:                     continue
Line 209: 
Line 210:                 self._backend.setDomainRegularRole(domain)
Line 211:             except Exception:
> SPM start should fail if one of the domains is unreacheable by the host des
Regardless of changes in the engine that you think that should be done (I don't agree with the comment, but that's not the scope for discussing it),
This code currently breaks BC with older engine versions, so we need to keep the behavior that was before, therefore it's irrelevant - the two options are either removing the code or to ignore the expcetion.
Line 212:                 # log any exception, but keep going
Line 213:                 self.log.error("Error trying to check/update domain %s role",
Line 214:                                sdUUID, exc_info=True)
Line 215: 


Line 209: 
Line 210:                 self._backend.setDomainRegularRole(domain)
Line 211:             except Exception:
Line 212:                 # log any exception, but keep going
Line 213:                 self.log.error("Error trying to check/update domain %s role",
> Comparing the masterVersion is the way to tell which is your most-recent ma
Dan,
Let's separate the issues here -
1. there's a current situation that breaks spm start, the current terminology of starting SPM doesn't require access to all the storage domain (previously to that code) - why is it needed? because that's excatly how the user can continue to work with the system while having monitoring thread on the domain - but that's irrelevant to this patch (that's the current way the system operates).

Previous to this change, vdsm never required the spm to access all domains - therfore it breaks BC with older engines regardless of what will be done in the engine.

let's first get this solved by either removing the method or having this fix.

2. About that master version - i'm not sure that it's always good enough (because you might have attached a domain that was master in other dc for example..etc), I don't see harm in trying to set the correct role for the domains that we can reach  - but again, i'm not totally against removing it. Fede?
Line 214:                                sdUUID, exc_info=True)
Line 215: 
Line 216:     @unsecured
Line 217:     def startSpm(self, prevID, prevLVER, maxHostID, expectedDomVersion=None):


-- 
To view, visit http://gerrit.ovirt.org/25424
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4f34360770ca8c8741a50956d0cab92bcd9a810
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list