[pykickstart][rhel7-branch/master][PATCH 1/2] Validate network interface name when parsing the kickstart (#1081982)
Martin Kolman
mkolman at redhat.com
Mon Sep 8 17:10:11 UTC 2014
On Mon, 2014-09-08 at 12:06 -0400, Chris Lumens wrote:
> > Resolves: rhbz#1081982
>
> If this patch resolves this bug, the bug should be reassigned to avoid
> process nightmares.
Done!
>
> > + # if it contains '.', vlan id should follow (eg 'ens7.171', 'mydev.171')
> > + split_name = retval.interfacename.split(".", 1)
> > + if len(split_name) > 1:
> > + # 'vlan' can't be followed by a '.'
> > + if split_name[0] == "vlan":
> > + raise KickstartValueError(formatErrorMsg(self.lineno,
> > + msg=_("for network --interfacename 'vlan' can't be followed by a '.'")))
>
> I don't really understand this error message. Could it be made more
> clear?
After studying the related NetworkManager source code, there are two
ways of how to include a vlan id in a network interface name:
* use the vlan prefix, followed by the vlan number:
"vlan123"
* use a prefix followed by a dot ('.') and then the vlan number, with
the condition that the prefix can't equal to "vlan"
So what about:
"When using the <prefix>.<vlan id> interface name notation <prefix>
can't be equal to 'vlan'."
Suggestions for an even more understandable error message are
welcome! :)
>
> > + try:
> > + vlan_id = int(split_name[1])
> > + except ValueError:
> > + raise KickstartValueError(formatErrorMsg(self.lineno,
> > + msg=_("if network --interfacename contains a '.', valid vlan id should follow")))
> > +
> > + # if it starts with 'vlan', vlan id should follow ('vlan171')
> > + if retval.interfacename.startswith("vlan"):
> > + try:
> > + vlan_id = int(retval.interfacename.strip("vlan"))
> > + except ValueError:
> > + raise KickstartValueError(formatErrorMsg(self.lineno,
> > + msg=_("if network --interfacename starts with 'vlan', valid vlan id should follow")))
>
> I'd like these two error messages to look more like sentences -
> capitalize the If, put a period at the end.
OK
>
> > + # check if the vlan id is in range
> > + if vlan_id is not None:
> > + if not(min_vlan_id <= vlan_id <= max_vlan_id):
> > + raise KickstartValueError(formatErrorMsg(self.lineno,
> > + msg=_("vlan id out of range")))
> > +
> > + return retval
>
> It'd help the user if you said what the valid range is.
Sure! :)
>
> Also this should probably all be in its own method. That way if some
> later version of the network command needs to override the parse method,
> it doesn't also need to duplicate all the work you've done here.
Doing just that.
>
> - Chris
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
More information about the anaconda-patches
mailing list