On Thursday, December 1, 2016 at 11:53:43 AM UTC, Iustin Pop wrote:
> On Thu, Dec 01, 2016 at 11:38:35AM +0000, Ganeti Development List wrote:
> > Following issue #1194, this patch allows Ganeti to correctly
> > parse drbd versions that also include a dash in their k-fix
> > version component.
> This means 8.4.8-1 and 220.127.116.11 will be treated the same. Is this the
> correct behaviour?
Yes, you are correct. I'm not sure to be honest. I've been looking around
the drbd and
also the debian packaging site but I'm not 100% sure what logic follows
numbering scheme. In drbd, from what I can see, they always use x.y.z-k
in one particular (old) version where they do x.y.z.k.
Honestly, though, as somebody correctly pointed out on the ganeti mailing
list, does it
really matter? Shouldn't we mostly only care about major and minor version
should be what might break compatibility? All in all I'd say we can treat
interchangeable but if somebody disagrees or knows more about this, I'd be
> > ---
> > lib/storage/drbd_info.py | 17 +++++++++++++++--
> > test/py/ganeti.storage.drbd_unittest.py | 10 ++++++++++
> > 2 files changed, 25 insertions(+), 2 deletions(-)
> > diff --git a/lib/storage/drbd_info.py b/lib/storage/drbd_info.py
> > index 99605f1..469ed7f 100644
> > --- a/lib/storage/drbd_info.py
> > +++ b/lib/storage/drbd_info.py
> > @@ -164,13 +164,15 @@ class DRBD8Info(object):
> > """
> > - _VERSION_RE = re.compile(r"^version: (\d+)\.(\d+)\.(\d+)(?:\.(\d+))?"
> > + _VERSION_RE = re.compile(r"^version:
> > r" \(api:(\d+)/proto:(\d+)(?:-(\d+))?\)")
> > _VALID_LINE_RE = re.compile("^ *([0-9]+): cs:([^ ]+).*$")
> > + _K_FIX_DASH_SEPARATOR_RE = re.compile(r"^version:
> > + def _GetKFixSeparator(self, lines):
> > + """Check, in case of a K-fix version, if the separator is a dash or
> > +
> > + first_line = lines.strip()
> > + match = self._K_FIX_DASH_SEPARATOR_RE.match(first_line)
> > + if match is None:
> > + return "."
> > + else:
> > + return "-"
> This seems to be done in two steps. Would it be simpler to have
> K_FIX_DASH_SEPARATOR itself extract the separator, instead of it having
> to match - and if not, return . vs -?
> You could even get rid of _K_FIX_DASH_SEPARATOR_RE, and extract the
> separator from the _VERSION_RE, after changing the RE to have the
> separator a capturing group.
I was trying to get as little regex pain as possible but what you're saying
makes more sense, I'll see what I can do with capture groups and fix the