Hello community, here is the log from the commit of package yast2-nfs-client for openSUSE:Factory checked in at 2018-04-07 20:52:07 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/yast2-nfs-client (Old) and /work/SRC/openSUSE:Factory/.yast2-nfs-client.new (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "yast2-nfs-client" Sat Apr 7 20:52:07 2018 rev:72 rq:594013 version:4.0.5 Changes: -------- --- /work/SRC/openSUSE:Factory/yast2-nfs-client/yast2-nfs-client.changes 2018-03-04 11:49:32.236787605 +0100 +++ /work/SRC/openSUSE:Factory/.yast2-nfs-client.new/yast2-nfs-client.changes 2018-04-07 20:52:11.444958410 +0200 @@ -1,0 +2,17 @@ +Fri Apr 6 08:51:02 UTC 2018 - an...@suse.com + +- Use only nfsvers (or its alias) to specify the version of the + NFS protocol, instead of the legacy nfs4 (vfstype) and + minorversion (bsc#1088426). +- Detect legacy entries and warn the user. +- 4.0.5 + +------------------------------------------------------------------- +Tue Apr 3 08:41:56 UTC 2018 - an...@suse.com + +- Command line interface: display correct content in the 'Options' + column (bsc#1087826) when listing. +- Command line interface: updated help about the 'type' option. +- 4.0.4 + +------------------------------------------------------------------- Old: ---- yast2-nfs-client-4.0.3.tar.bz2 New: ---- yast2-nfs-client-4.0.5.tar.bz2 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ yast2-nfs-client.spec ++++++ --- /var/tmp/diff_new_pack.ZfPgR6/_old 2018-04-07 20:52:12.116934089 +0200 +++ /var/tmp/diff_new_pack.ZfPgR6/_new 2018-04-07 20:52:12.116934089 +0200 @@ -17,7 +17,7 @@ Name: yast2-nfs-client -Version: 4.0.3 +Version: 4.0.5 Release: 0 Url: https://github.com/yast/yast-nfs-client @@ -89,6 +89,7 @@ %{yast_moduledir}/Nfs.rb %{yast_moduledir}/NfsOptions.rb %{yast_dir}/lib/fstab +%{yast_dir}/lib/y2nfs_client %dir %{yast_desktopdir} %{yast_desktopdir}/nfs.desktop %doc %{yast_docdir} ++++++ yast2-nfs-client-4.0.3.tar.bz2 -> yast2-nfs-client-4.0.5.tar.bz2 ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/package/yast2-nfs-client.changes new/yast2-nfs-client-4.0.5/package/yast2-nfs-client.changes --- old/yast2-nfs-client-4.0.3/package/yast2-nfs-client.changes 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/package/yast2-nfs-client.changes 2018-04-06 16:21:39.000000000 +0200 @@ -1,4 +1,21 @@ ------------------------------------------------------------------- +Fri Apr 6 08:51:02 UTC 2018 - an...@suse.com + +- Use only nfsvers (or its alias) to specify the version of the + NFS protocol, instead of the legacy nfs4 (vfstype) and + minorversion (bsc#1088426). +- Detect legacy entries and warn the user. +- 4.0.5 + +------------------------------------------------------------------- +Tue Apr 3 08:41:56 UTC 2018 - an...@suse.com + +- Command line interface: display correct content in the 'Options' + column (bsc#1087826) when listing. +- Command line interface: updated help about the 'type' option. +- 4.0.4 + +------------------------------------------------------------------- Tue Feb 27 13:36:26 UTC 2018 - knut.anders...@suse.com - Fix a missing definition of firewalld method (bsc#1082919) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/package/yast2-nfs-client.spec new/yast2-nfs-client-4.0.5/package/yast2-nfs-client.spec --- old/yast2-nfs-client-4.0.3/package/yast2-nfs-client.spec 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/package/yast2-nfs-client.spec 2018-04-06 16:21:39.000000000 +0200 @@ -17,7 +17,7 @@ Name: yast2-nfs-client -Version: 4.0.3 +Version: 4.0.5 Release: 0 Url: https://github.com/yast/yast-nfs-client @@ -89,6 +89,7 @@ %{yast_moduledir}/Nfs.rb %{yast_moduledir}/NfsOptions.rb %{yast_dir}/lib/fstab +%{yast_dir}/lib/y2nfs_client %dir %{yast_desktopdir} %{yast_desktopdir}/nfs.desktop %doc %{yast_docdir} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/src/Makefile.am new/yast2-nfs-client-4.0.5/src/Makefile.am --- old/yast2-nfs-client-4.0.3/src/Makefile.am 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/src/Makefile.am 2018-04-06 16:21:39.000000000 +0200 @@ -27,6 +27,11 @@ ylib_DATA = \ lib/fstab/tsort.rb -EXTRA_DIST = $(module_DATA) $(client_DATA) $(ynclude_DATA) $(schemafiles_DATA) $(desktop_DATA) $(ylib_DATA) +y2nfslibdir = @ylibdir@/y2nfs_client +y2nfslib_DATA = \ + lib/y2nfs_client/nfs_version.rb + +EXTRA_DIST = \ + $(module_DATA) $(client_DATA) $(ynclude_DATA) $(schemafiles_DATA) $(desktop_DATA) $(ylib_DATA) $(y2nfslib_DATA) include $(top_srcdir)/Makefile.am.common diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/src/clients/nfs.rb new/yast2-nfs-client-4.0.5/src/clients/nfs.rb --- old/yast2-nfs-client-4.0.3/src/clients/nfs.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/src/clients/nfs.rb 2018-04-06 16:21:39.000000000 +0200 @@ -96,7 +96,7 @@ # command line option help # fstab(5): fs_type "help" => _( - "File system ID, supported nfs and nfs4. Default value is nfs." + "File system ID. Legacy. Only default value (nfs) makes sense." ) } }, @@ -148,7 +148,7 @@ Ops.get_string(i, 1, ""), Ops.get_string(i, 2, ""), Ops.get_string(i, 3, ""), - Ops.get_string(i, 4, "") + Ops.get_string(i, 5, "") ] ) end @@ -262,7 +262,7 @@ Ops.get_string(i2, 1, ""), Ops.get_string(i2, 2, ""), Ops.get_string(i2, 3, ""), - Ops.get_string(i2, 4, "") + Ops.get_string(i2, 5, "") ] ) end diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/src/include/nfs/routines.rb new/yast2-nfs-client-4.0.5/src/include/nfs/routines.rb --- old/yast2-nfs-client-4.0.3/src/include/nfs/routines.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/src/include/nfs/routines.rb 2018-04-06 16:21:39.000000000 +0200 @@ -1,5 +1,7 @@ # encoding: utf-8 +require "y2nfs_client/nfs_version" + # YaST namespace module Yast # Miscellaneous @@ -12,6 +14,7 @@ Yast.import "IP" Yast.import "Hostname" Yast.import "String" + Yast.import "NfsOptions" end # @param [String] spec "server:/path/specification" @@ -49,13 +52,14 @@ count = 0 Builtins.maplist(fstab) do |entry| sp = SpecToServPath(Ops.get_string(entry, "spec", "")) + mntops = entry["mntops"] || "" it = Item( Id(count), Ops.add(Ops.get_string(sp, 0, ""), " "), Ops.add(Ops.get_string(sp, 1, ""), " "), Ops.add(Ops.get_string(entry, "file", ""), " "), - Ops.get_string(entry, "vfstype", " "), - Ops.add(Ops.get_string(entry, "mntops", ""), " ") + "#{nfs_version_for_table(entry)} ", + "#{mntops} " ) count = Ops.add(count, 1) deep_copy(it) @@ -170,5 +174,33 @@ def IsPortmapperInstalled(portmapper) Package.Install(portmapper) end + + # Whether the fstab entry uses old ways of configuring the NFS version that + # do not longer work in the way they used to. + # + # @param entry [Hash] + # @return [Boolean] + def legacy_entry?(entry) + entry["vfstype"] == "nfs4" || NfsOptions.legacy?(entry["mntops"] || "") + end + + private + + # @see #FstabTableItems + # + # @param entry [Hash] + # @return [String] + def nfs_version_for_table(entry) + mntops = entry["mntops"] || "" + version = NfsOptions.nfs_version(mntops) + + if legacy_entry?(entry) + # TRANSLATORS: %s is a string representing the NFS version used, but + # maybe it's not the one the user wanted. + _("%s (Please Check)") % version.label + else + version.label + end + end end end diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/src/include/nfs/ui.rb new/yast2-nfs-client-4.0.5/src/include/nfs/ui.rb --- old/yast2-nfs-client-4.0.3/src/include/nfs/ui.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/src/include/nfs/ui.rb 2018-04-06 16:21:39.000000000 +0200 @@ -1,6 +1,8 @@ # encoding: utf-8 require "y2firewall/firewalld" require "yast2/feedback" +require "yast2/popup" +require "y2nfs_client/nfs_version" # YaST namespace module Yast @@ -39,19 +41,29 @@ @modify_line = {} - # Help, part 1 of 3 + # Help, part 1 of 4 @help_text1 = _( "<p>The table contains all directories \n" \ "exported from remote servers and mounted locally via NFS (NFS shares).</p>" ) + - # Help, part 2 of 3 + # Help, part 2 of 4 _( "<p>Each NFS share is identified by remote NFS server address and\n" \ "exported directory, local directory where the remote directory is mounted, \n" \ - "NFS type (either plain nfs or nfsv4) and mount options. For further information \n" \ + "version of the NFS protocol and mount options. For further information \n" \ "about mounting NFS and mount options, refer to <tt>man nfs.</tt></p>" ) + - # Help, part 3 of 3 + # Help, part 3 of 4 + _( + "<p>It may happen that some NFS share is mounted using an old method\n" \ + "to specify the version of the NFS protocol, like the usage of 'nfs4'\n" \ + "as file system type or the usage of 'minorversion' in the mount options.\n" \ + "Those methods do not longer work as they used to, so if such\n" \ + "circumstance is detected, the real current version is displayed in the\n" \ + "list followed by a warning message. Those entries can be edited to\n" \ + "make sure they use more current ways of specifying the version.</p>" + ) + + # Help, part 4 of 4 _( "<p>To mount a new NFS share, click <B>Add</B>. To change the configuration of\n" \ "a currently mounted share, click <B>Edit</B>. Remove and unmount a selected\n" \ @@ -61,8 +73,8 @@ @help_text2 = Ops.add( _( "<p>If you need to access NFSv4 shares (NFSv4 is a newer version of the NFS\n" \ - "protocol), check the <b>Enable NFSv4</b> option. In that case, you might need\n" \ - "to supply specific a <b>NFSv4 Domain Name</b> required for the correct setting\n" \ + "protocol), check the <b>NFS version</b> option. In that case, you might need\n" \ + "to supply an specific <b>NFSv4 Domain Name</b> required for the correct setting\n" \ "of file/directory access rights.</p>\n" ), Ops.get_string(@fw_cwm_widget, "help", "") @@ -202,8 +214,6 @@ server = "" pth = "" mount = "" - nfs4 = false - nfs41 = false options = "defaults" servers = [] old = "" @@ -214,15 +224,14 @@ server = Ops.get_string(couple, 0, "") pth = Ops.get_string(couple, 1, "") mount = Ops.get_string(fstab_ent, "file", "") - nfs4 = Ops.get_string(fstab_ent, "vfstype", "") == "nfs4" options = Ops.get_string(fstab_ent, "mntops", "") - nfs41 = nfs4 && NfsOptions.get_nfs41(options) servers = [server] old = Ops.get_string(fstab_ent, "spec", "") else proposed_server = ProposeHostname() servers = [proposed_server] if HostnameExists(proposed_server) end + version = NfsOptions.nfs_version(options) # append already defined servers - bug #547983 Builtins.foreach(@nfs_entries) do |nfs_entry| @@ -273,12 +282,7 @@ ) ), Left( - HBox( - CheckBox(Id(:nfs4), _("NFS&v4 Share"), nfs4), - HSpacing(2), - # parallel NFS, protocol version 4.1 - CheckBox(Id(:nfs41), _("pNFS (v4.1)"), nfs41) - ) + version_widget(version) ), Left( TextAndButton( @@ -350,7 +354,7 @@ next end - v4 = UI.QueryWidget(Id(:nfs4), :Value) + v4 = version_from_widget.browse_with_v4? scan_exports(server2, v4) elsif ret == :browse dir = Convert.to_string(UI.QueryWidget(Id(:mountent), :Value)) @@ -372,13 +376,11 @@ mount = StripExtraSlash( Convert.to_string(UI.QueryWidget(Id(:mountent), :Value)) ) - nfs4 = Convert.to_boolean(UI.QueryWidget(Id(:nfs4), :Value)) - nfs41 = Convert.to_boolean(UI.QueryWidget(Id(:nfs41), :Value)) options = Builtins.deletechars( Convert.to_string(UI.QueryWidget(Id(:optionsent), :Value)), " " ) - options = NfsOptions.set_nfs41(options, nfs41) + options = NfsOptions.set_nfs_version(options, version_from_widget) ret = nil options_error = NfsOptions.validate(options) @@ -395,7 +397,7 @@ fstab_ent = { "spec" => Ops.add(Ops.add(server, ":"), pth), "file" => mount, - "vfstype" => nfs4 ? "nfs4" : "nfs", + "vfstype" => "nfs", "mntops" => options } if old != Ops.add(Ops.add(server, ":"), pth) @@ -472,7 +474,7 @@ # table header _("Mount Point") + " ", # table header - _("NFS Type"), + _("NFS Version"), # table header _("Options") + " " ), @@ -598,28 +600,9 @@ UI.ChangeWidget(Id(:fstable), :Items, FstabTableItems(@nfs_entries)) elsif widget == :editbut - entry = GetFstabEntry( - Ops.get(@nfs_entries, entryno, {}), - Convert.convert( - Builtins.union( - Nfs.non_nfs_entries, - Builtins.remove(@nfs_entries, entryno) - ), - from: "list", - to: "list <map>" - ) # Default values - ) - if entry - count2 = 0 - @nfs_entries = Builtins.maplist(@nfs_entries) do |ent| - count2 = Ops.add(count2, 1) - next deep_copy(ent) if Ops.subtract(count2, 1) != entryno - deep_copy(entry) - end - - @modify_line = deep_copy(entry) - UI.ChangeWidget(Id(:fstable), :Items, FstabTableItems(@nfs_entries)) - Nfs.SetModified + source_entry = @nfs_entries[entryno] || {} + if !legacy_entry?(source_entry) || edit_legacy? + edit_entry(source_entry, entryno) end elsif widget == :delbut && Ops.greater_than(Builtins.size(@nfs_entries), 0) @@ -723,5 +706,63 @@ Report.Error(_("The NFS scan failed.")) end end + + private + + # Widget to select the version of the NFS protocol to use in a mount that is + # being created or edited. + def version_widget(current_version) + items = Y2NfsClient::NfsVersion.all.map do |vers| + Item(Id(vers.widget_id), vers.widget_text, current_version == vers) + end + ComboBox(Id(:nfs_version), _("NFS &Version"), items) + end + + # Version of the NFS protocol selected in the corresponding widget. + # + # @return [Y2NfsClient::NfsVersion] + def version_from_widget + id = UI.QueryWidget(Id(:nfs_version), :Value).to_sym + Y2NfsClient::NfsVersion.all.find { |v| v.widget_id == id } + end + + # @see #HandleEvent + def edit_entry(source_entry, entryno) + entry = GetFstabEntry( + source_entry, + Convert.convert( + Builtins.union( + Nfs.non_nfs_entries, + Builtins.remove(@nfs_entries, entryno) + ), + from: "list", + to: "list <map>" + ) # Default values + ) + if entry + count2 = 0 + @nfs_entries = Builtins.maplist(@nfs_entries) do |ent| + count2 = Ops.add(count2, 1) + next deep_copy(ent) if Ops.subtract(count2, 1) != entryno + deep_copy(entry) + end + + @modify_line = deep_copy(entry) + UI.ChangeWidget(Id(:fstable), :Items, FstabTableItems(@nfs_entries)) + Nfs.SetModified + end + end + + def edit_legacy? + msg = _( + "This entry uses old ways of specifying the NFS protocol version that\n" \ + "do not longer work as they used to do it (like the usage of 'nfs4' as\n" \ + "file system type or the usage of 'minorversion' in the mount options).\n\n" \ + "Editing the entry will change how the version is specified, with no\n" \ + "possibility to use old outdated method again.\n\n" \ + "Proceed and edit?" + ) + Yast2::Popup.show(msg, buttons: :continue_cancel) == :continue + end end end diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/src/lib/y2nfs_client/nfs_version.rb new/yast2-nfs-client-4.0.5/src/lib/y2nfs_client/nfs_version.rb --- old/yast2-nfs-client-4.0.3/src/lib/y2nfs_client/nfs_version.rb 1970-01-01 01:00:00.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/src/lib/y2nfs_client/nfs_version.rb 2018-04-06 16:21:39.000000000 +0200 @@ -0,0 +1,120 @@ +# encoding: utf-8 +# +# Copyright (c) [2018] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "yast" + +module Y2NfsClient + # Version of the NFS protocol configured for an NFS mount + # + # The main goal of the class if to map how the version is configured in the + # /etc/fstab entry with how that is presented to the user. + # + # Each instance is an immutable object that can represent the configuration + # to enforce a concrete version or a more generic configuration (i.e. "use + # any available version"). + class NfsVersion + include Yast::I18n + extend Yast::I18n + textdomain "nfs" + + # Constructor + def initialize(mntops_value, label, widget_id, widget_text) + @mntops_value = mntops_value + @label = label + @widget_id = widget_id + @widget_text = widget_text + end + + ALL = [ + new(nil, N_("Any"), :vers_any, N_("Any (Highest Available)")), + new("3", N_("NFSv3"), :vers_3, N_("Force NFSv3")), + new("4", N_("NFSv4"), :vers_4, N_("Force NFSv4")), + new("4.1", N_("NFSv4.1"), :vers_4_1, N_("Force pNFS (v4.1)")) + ].freeze + private_constant :ALL + + # Sorted list of all possible settings + def self.all + ALL.dup + end + + # An instance corresponding to a given value in the mount options + # + # @see #mntops_value + # + # @param value [String] + # @return [NfsVersion] + def self.for_mntops_value(value) + all.find { |version| version.mntops_value == value } + end + + # Value used in the corresponding mount option (nfsvers or vers) + # + # @return [String] + attr_reader :mntops_value + + # Id for a widget representing the instance + # + # @return [Symbol] + attr_reader :widget_id + + # Short localized label to represent the instance in listings + # + # @return [String] very likely, a frozen string + def label + _(@label) + end + + # Localized text to use in a widget representing the instance + # + # @return [String] very likely, a frozen string + def widget_text + _(@widget_text) + end + + def ==(other) + other.class == self.class && other.mntops_value == mntops_value + end + + alias_method :eql?, :== + + # Whether the system infrastructure associated to NFSv4 (e.g. enabled + # NFS4_SUPPORT in sysconfig/nfs) is needed in order to use this version of + # the protocol. + # + # @return [Boolean] + def requires_v4? + return false if mntops_value.nil? + mntops_value.start_with?("4") + end + + # Whether is necessary to use the browsing mechanisms associated to NFSv4 in + # order to find shares of this type in the network or in a given server. + # + # Scanning a network or server to find NFS shares is completely different + # depending on the version of the protocol (version 3 vs version 4+). + # + # @return [Boolean] + def browse_with_v4? + requires_v4? + end + end +end diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/src/modules/Nfs.rb new/yast2-nfs-client-4.0.5/src/modules/Nfs.rb --- old/yast2-nfs-client-4.0.3/src/modules/Nfs.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/src/modules/Nfs.rb 2018-04-06 16:21:39.000000000 +0200 @@ -201,7 +201,8 @@ # vfstype can override a missing enable_nfs4 @nfs4_enabled = true if Builtins.find(entries) do |entry| - Ops.get_string(entry, "vfstype", "") == "nfs4" + version = NfsOptions.nfs_version(entry["nfs_options"] || "") + version && version.requires_v4? end @nfs_entries = Builtins.maplist(entries) do |entry| diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/src/modules/NfsOptions.rb new/yast2-nfs-client-4.0.5/src/modules/NfsOptions.rb --- old/yast2-nfs-client-4.0.3/src/modules/NfsOptions.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/src/modules/NfsOptions.rb 2018-04-06 16:21:39.000000000 +0200 @@ -1,6 +1,7 @@ # encoding: utf-8 require "yast" +require "y2nfs_client/nfs_version" # YaST namespace module Yast @@ -84,7 +85,9 @@ "context", "defcontext", "fscontext", - "minorversion", + # Minorversion is basically undocumented and obsolete (nfsvers supports + # strings line "4.1"). Commented out so it gets deleted. + # "minorversion", "mounthost", "mountport", "mountprog", @@ -169,40 +172,136 @@ error_message end - # FIXME: factor out get_nfs4(vfstype, options) (depending on n::o)! - # @param options [String] fstab option string - # @return [Boolean] is version >= 4.1 enabled - def get_nfs41(options) + # Version of the NFS protocol specified in a given set of mount options. + # + # This method can handle situations in which 'nfsvers' and 'vers' (the two + # equivalent options to specify the protocol) are used more than once (which + # is wrong but recoverable), but if will not work if the mount options + # string is malformed. + # + # @param options [String] mount options in the comma-separated format used + # by mount and /etc/fstab + # @return [Y2NfsClient::NfsVersion] + def nfs_version(options) option_list = from_string(options) - - enabled = "minorversion=1" - Builtins.contains(option_list, enabled) + Y2NfsClient::NfsVersion.for_mntops_value(relevant_version_value(option_list)) end - # Add or remove minorversion=1 according to nfs41. - # FIXME: vfstype=nfs4 is deprecated in favor of nfsvers=4 (aka vers=4) - # @param [String] options fstab option string - # @param [Boolean] nfs41 is version >= 4.1 enabled - # @return [String] new fstab option string - def set_nfs41(options, nfs41) - # don't mutate the string unnecessarily - return options if get_nfs41(options) == nfs41 + # Returns a copy of the mount options with the changes needed to ensure the + # given NFS protocol version. + # + # This method modifies or deletes the existing 'nfsvers' or 'vers' option + # (deleting always the surplus options). If no option is present and one + # must be added, 'nfsvers' is used. + # + # Although it can handle several wrong or legacy configurations, this method + # will not work if the mount options string is malformed. + # + # @param options [String] mount options in the comma-separated format used + # by mount and /etc/fstab + # @param version [Y2NfsClient::NfsVersion] + def set_nfs_version(options, version) + option_list = from_string(options) + without_option = version.mntops_value.nil? - enabled = "minorversion=1" - disabled = "minorversion=0" + # Cleanup minorversion, it should never be used + option_list.delete_if { |opt| opt.start_with?("minorversion=") } - option_list = from_string(options) - option_list = Builtins.filter(option_list) { |opt| opt != enabled } - option_list = Builtins.filter(option_list) { |opt| opt != disabled } + # Cleanup surplus options + option_to_keep = without_option ? nil : relevant_version_option(option_list) + option_list.delete_if { |opt| version_option?(opt) && !opt.equal?(option_to_keep) } - option_list = Builtins.add(option_list, enabled) if nfs41 + return to_string(option_list) if without_option + + if option_to_keep + option_to_keep.gsub!(/=.*$/, "=#{version.mntops_value}") + else + option_list << "nfsvers=#{version.mntops_value}" + end to_string(option_list) end + # Whether the given mount options correspond to a NFSv4.1 mount + # + # This checks the usage of 'nfsvers' and 'vers'. 'minorversion' is ignored + # since it should not be used nowadays. + # + # @param options [String] mount options in the comma-separated format used + # by mount and /etc/fstab + # @return [Boolean] is version >= 4.1 enabled + def get_nfs41(options) + nfs_version(options).mntops_value == "4.1" + end + + # Modifies the mount options to make sure NFSv4.1 is used (or to make sure + # it stops being used). + # + # This uses 'nfsvers' (or 'vers' if it was already present). Any + # 'minorversion' option will be deleted, since they should not be longer + # used. + # + # @param options [String] mount options in the comma-separated format used + # by mount and /etc/fstab + # @param nfs41 [Boolean] whether to enable version >= 4.1. If false the + # options will be configured to use any NFS version. + # @return [String] new fstab option string + def set_nfs41(options, nfs41) + version_string = nfs41 ? "4.1" : nil + version = Y2NfsClient::NfsVersion.for_mntops_value(version_string) + set_nfs_version(options, version) + end + + # Checks whether some of the old options that used to work to configure + # the NFS version (but do not longer work now) is used. + # + # Basically, this checks for the presence of minorversion + # + # @param options [String] mount options in the comma-separated format used + # by mount and /etc/fstab + # @return [Boolean] + def legacy?(options) + option_list = from_string(options) + option_list.any? { |opt| opt.start_with?("minorversion=") } + end + publish function: :validate, type: "string (string)" publish function: :get_nfs41, type: "boolean (string)" publish function: :set_nfs41, type: "string (string, boolean)" + + private + + # Option used to set the NFS protocol version + # + # @param option_list [Array<String>] + # @return [String, nil] contains the whole 'option=value' string + def relevant_version_option(option_list) + # According to manual tests and documentation, none of the forms has higher precedence. + # Use #reverse_each because in case of conflicting options, the latest one is used by mount + option_list.reverse_each.find do |opt| + version_option?(opt) + end + end + + # Value part for {#relevant_version_option} + # + # @param option_list [Array<String>] + # @return [String, nil] + def relevant_version_value(option_list) + relevant_option = relevant_version_option(option_list) + return nil unless relevant_option + parts = relevant_option.split("=") + return nil if parts.size != 2 + parts.last + end + + # Checks if a given option is used to configure the NFS protocol version + # + # @param [String] + # @return [Boolean] + def version_option?(option) + option.start_with?("nfsvers=", "vers=") + end end NfsOptions = NfsOptionsClass.new diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/test/data/nfs_entries.yaml new/yast2-nfs-client-4.0.5/test/data/nfs_entries.yaml --- old/yast2-nfs-client-4.0.3/test/data/nfs_entries.yaml 1970-01-01 01:00:00.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/test/data/nfs_entries.yaml 2018-04-06 16:21:39.000000000 +0200 @@ -0,0 +1,24 @@ +- spec: nfs.example.com:/one + file: /one + vfstype: nfs + mntops: defaults + +- spec: nfs.example.com:/two + file: /two + vfstype: nfs4 + mntops: defaults + +- spec: nfs.example.com:/three + file: /three + vfstype: nfs + mntops: nfsvers=4 + +- spec: nfs.example.com:/four + file: /four + vfstype: nfs4 + mntops: rw,minorversion=1 + +- spec: nfs.example.com:/five + file: /five + vfstype: nfs + mntops: vers=4,minorversion=1,ro diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/test/nfs_options_test.rb new/yast2-nfs-client-4.0.5/test/nfs_options_test.rb --- old/yast2-nfs-client-4.0.3/test/nfs_options_test.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/test/nfs_options_test.rb 2018-04-06 16:21:39.000000000 +0200 @@ -88,4 +88,87 @@ end end end + + describe "#nfs_version" do + it "returns the generic version if none of vers or nfsvers is used" do + [ + "defaults", + "nolock,bg", + "nolock,minorversion=1", + "nolock,rsize=8192", + "defaults,ro,noatime,minorversion=1,users,exec" + ].each do |options| + returned = Yast::NfsOptions.nfs_version(options) + expect(returned.mntops_value).to be_nil + end + end + + it "returns the version specified by nfsvers if it's present" do + { + "nfsvers=4" => "4", + "nfsvers=4,minorversion=1" => "4", + "defaults,nfsvers=3" => "3", + "nfsvers=4.1,nolock" => "4.1" + }.each_pair do |opts, version| + returned = Yast::NfsOptions.nfs_version(opts) + expect(returned.mntops_value).to eq version + end + end + + it "returns the version specified by vers if it's present" do + { + "minorversion=1,vers=4" => "4", + "vers=3,ro" => "3", + "vers=4.1" => "4.1" + }.each_pair do |opts, version| + returned = Yast::NfsOptions.nfs_version(opts) + expect(returned.mntops_value).to eq version + end + end + + it "returns the correct version if nfsvers and vers appear several time" do + { + "nfsvers=4,minorversion=1,vers=3" => "3", + "vers=3,ro,vers=4" => "4", + "vers=4.1,rw,nfsvers=3,nfsvers=4,nolock" => "4" + }.each_pair do |opts, version| + returned = Yast::NfsOptions.nfs_version(opts) + expect(returned.mntops_value).to eq version + end + end + end + + describe "#set_nfs_version" do + def set_version(opts, version_value) + version = Y2NfsClient::NfsVersion.for_mntops_value(version_value) + Yast::NfsOptions.set_nfs_version(opts, version) + end + + it "removes existing minorversion options" do + expect(set_version("minorversion=1", nil)).to eq "defaults" + expect(set_version("minorversion=1,ro,minorversion=1", "4")). to eq "ro,nfsvers=4" + end + + it "removes nfsvers and vers when enforcing no particular version" do + expect(set_version("nfsvers=4", nil)).to eq "defaults" + expect(set_version("vers=3,ro", nil)). to eq "ro" + expect(set_version("nolock,vers=4.1,rw,nfsvers=4", nil)). to eq "nolock,rw" + end + + it "modifies the existing nfsvers or vers option if needed" do + expect(set_version("nfsvers=4", "3")).to eq "nfsvers=3" + expect(set_version("vers=3,ro", "4")). to eq "vers=4,ro" + expect(set_version("nolock,nfsvers=4.1,rw,vers=4", "4.1")). to eq "nolock,rw,vers=4.1" + end + + it "deletes surplus useless nfsvers and vers options" do + expect(set_version("vers=4,nolock,nfsvers=4.1,rw,vers=4", "4.1")). to eq "nolock,rw,vers=4.1" + expect(set_version("nfsvers=4,vers=4.1,rw,nfsvers=4", "3")). to eq "rw,nfsvers=3" + end + + it "adds a nfsvers if a new option is needed" do + expect(set_version("defaults", "4.1")). to eq "nfsvers=4.1" + expect(set_version("rw,nolock", "3")). to eq "rw,nolock,nfsvers=3" + end + end end diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/test/nfs_test.rb new/yast2-nfs-client-4.0.5/test/nfs_test.rb --- old/yast2-nfs-client-4.0.3/test/nfs_test.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/test/nfs_test.rb 2018-04-06 16:21:39.000000000 +0200 @@ -103,4 +103,24 @@ subject.WriteOnly end end + + describe ".legacy_entry?" do + let(:all_entries) { YAML.load_file(File.join(DATA_PATH, "nfs_entries.yaml")) } + let(:entries) { all_entries.map { |e| [e["file"], e] }.to_h } + + it "returns true for entries using nfs4 as vfstype" do + expect(subject.legacy_entry?(entries["/two"])).to eq true + expect(subject.legacy_entry?(entries["/four"])).to eq true + end + + it "returns true for entries using minorversion in the mount options" do + expect(subject.legacy_entry?(entries["/four"])).to eq true + expect(subject.legacy_entry?(entries["/five"])).to eq true + end + + it "returns false for entries without nfs4 or minorversion" do + expect(subject.legacy_entry?(entries["/one"])).to eq false + expect(subject.legacy_entry?(entries["/three"])).to eq false + end + end end diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/testsuite/tests/autoyast.out new/yast2-nfs-client-4.0.5/testsuite/tests/autoyast.out --- old/yast2-nfs-client-4.0.3/testsuite/tests/autoyast.out 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/testsuite/tests/autoyast.out 2018-04-06 16:21:39.000000000 +0200 @@ -23,6 +23,11 @@ Read .sysconfig.nfs.NFS_SECURITY_GSS "no" Return true Dump -- and Export +Dump - NFSv4 via mntops +Read .sysconfig.nfs.NFS4_SUPPORT "no" +Read .sysconfig.nfs.NFS_SECURITY_GSS "no" +Return true +Dump -- and Export Dump - with GSS Return true Dump -- and Export diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/testsuite/tests/autoyast.rb new/yast2-nfs-client-4.0.5/testsuite/tests/autoyast.rb --- old/yast2-nfs-client-4.0.3/testsuite/tests/autoyast.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/testsuite/tests/autoyast.rb 2018-04-06 16:21:39.000000000 +0200 @@ -101,7 +101,7 @@ TEST(->() { Nfs.ImportAny([@global_options2, @entry2]) }, [@READ, {}, {}], nil) - Assert.Equal(true, Nfs.nfs4_enabled) + Assert.Equal(false, Nfs.nfs4_enabled) Assert.Equal("example.com", Nfs.idmapd_domain) Assert.Equal(1, Builtins.size(Nfs.nfs_entries)) Assert.Equal( @@ -113,7 +113,7 @@ @ex = Nfs.Export @e = Ops.get_list(@ex, "nfs_entries", []) Assert.Equal(1, Builtins.size(@e)) - Assert.Equal(true, Ops.get_boolean(@ex, "enable_nfs4", false)) + Assert.Equal(false, Ops.get_boolean(@ex, "enable_nfs4", false)) Assert.Equal("example.com", Ops.get_string(@ex, "idmapd_domain", "")) Assert.Equal( "data.example.com:/mirror", @@ -123,6 +123,27 @@ Assert.Equal("defaults", Ops.get_string(@e, [0, "nfs_options"], "")) # --------- + DUMP("- NFSv4 via mntops") + @entry3 = { + "server_path" => "data.example.com:/mirror", + "mount_point" => "/mirror", + "nfs_options" => "nfsvers=4", + "vfstype" => "nfs" + } + + TEST(->() { Nfs.ImportAny([@global_options2, @entry3]) }, [@READ, {}, {}], nil) + + Assert.Equal(true, Nfs.nfs4_enabled) + Assert.Equal(1, Nfs.nfs_entries.size) + + DUMP("-- and Export") + @ex = Nfs.Export + @e = @ex["nfs_entries"] + Assert.Equal(1, @e.size) + Assert.Equal(true, @ex["enable_nfs4"]) + Assert.Equal("nfsvers=4", @e.first["nfs_options"]) + + # --------- DUMP("- with GSS") @global_options = { "enable_nfs4" => true, diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/testsuite/tests/nfs_options.rb new/yast2-nfs-client-4.0.5/testsuite/tests/nfs_options.rb --- old/yast2-nfs-client-4.0.3/testsuite/tests/nfs_options.rb 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/testsuite/tests/nfs_options.rb 2018-04-06 16:21:39.000000000 +0200 @@ -25,57 +25,51 @@ Assert.Equal(false, NfsOptions.get_nfs41("")) Assert.Equal(false, NfsOptions.get_nfs41("defaults")) Assert.Equal(false, NfsOptions.get_nfs41("ro,sync")) + # minorversion is irrelevant nowadays, no matter its value Assert.Equal(false, NfsOptions.get_nfs41("minorversion=0")) - Assert.Equal(true, NfsOptions.get_nfs41("minorversion=1")) - # "minorversion=2" does not exist yet, YAGNI + Assert.Equal(false, NfsOptions.get_nfs41("minorversion=1")) Assert.Equal(false, NfsOptions.get_nfs41("subminorversion=1")) # substring must not match # Assert::Equal(?, NfsOptions::get_nfs41("minorversion=1,minorversion=0")); // don't care Assert.Equal(false, NfsOptions.get_nfs41("ro,minorversion=0,sync")) - Assert.Equal(true, NfsOptions.get_nfs41("ro,minorversion=1,sync")) + Assert.Equal(false, NfsOptions.get_nfs41("ro,minorversion=1,sync")) DUMP("NfsOptions::set_nfs41") - Assert.Equal("", NfsOptions.set_nfs41("", false)) - Assert.Equal("minorversion=1", NfsOptions.set_nfs41("", true)) + Assert.Equal("defaults", NfsOptions.set_nfs41("", false)) + Assert.Equal("nfsvers=4.1", NfsOptions.set_nfs41("", true)) Assert.Equal("defaults", NfsOptions.set_nfs41("defaults", false)) - Assert.Equal("minorversion=1", NfsOptions.set_nfs41("defaults", true)) + Assert.Equal("nfsvers=4.1", NfsOptions.set_nfs41("defaults", true)) Assert.Equal("ro,sync", NfsOptions.set_nfs41("ro,sync", false)) Assert.Equal( - "ro,sync,minorversion=1", + "ro,sync,nfsvers=4.1", NfsOptions.set_nfs41("ro,sync", true) ) Assert.Equal( - "minorversion=0", + "defaults", NfsOptions.set_nfs41("minorversion=0", false) ) Assert.Equal( - "minorversion=1", + "nfsvers=4.1", NfsOptions.set_nfs41("minorversion=0", true) ) - Assert.Equal("defaults", NfsOptions.set_nfs41("minorversion=1", false)) - Assert.Equal( - "minorversion=1", - NfsOptions.set_nfs41("minorversion=1", true) - ) - Assert.Equal( "subminorversion=1", NfsOptions.set_nfs41("subminorversion=1", false) ) Assert.Equal( - "subminorversion=1,minorversion=1", + "subminorversion=1,nfsvers=4.1", NfsOptions.set_nfs41("subminorversion=1", true) ) Assert.Equal( - "ro,minorversion=0,sync", + "ro,sync", NfsOptions.set_nfs41("ro,minorversion=0,sync", false) ) Assert.Equal( - "ro,sync,minorversion=1", + "ro,sync,nfsvers=4.1", NfsOptions.set_nfs41("ro,minorversion=0,sync", true) ) @@ -84,7 +78,7 @@ NfsOptions.set_nfs41("ro,minorversion=1,sync", false) ) Assert.Equal( - "ro,minorversion=1,sync", + "ro,sync,nfsvers=4.1", NfsOptions.set_nfs41("ro,minorversion=1,sync", true) ) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/yast2-nfs-client-4.0.3/testsuite/tests/r_fstab.out new/yast2-nfs-client-4.0.5/testsuite/tests/r_fstab.out --- old/yast2-nfs-client-4.0.3/testsuite/tests/r_fstab.out 2018-02-27 15:19:17.000000000 +0100 +++ new/yast2-nfs-client-4.0.5/testsuite/tests/r_fstab.out 2018-04-06 16:21:39.000000000 +0200 @@ -1,5 +1,5 @@ Dump FstabTableItems -Return [`item (`id (0), "foo.bar.com ", "/home ", "/home ", "nfs4", "defaults "), `item (`id (1), "foo.bar.com ", "/var/spool/mail ", "/var/spool/mail ", "nfs", "defaults "), `item (`id (2), "foo.bar.com.tw ", "/local/install ", "/install ", "nfs", "hard,intr ")] +Return [`item (`id (0), "foo.bar.com ", "/home ", "/home ", "Any (Please Check) ", "defaults "), `item (`id (1), "foo.bar.com ", "/var/spool/mail ", "/var/spool/mail ", "Any ", "defaults "), `item (`id (2), "foo.bar.com.tw ", "/local/install ", "/install ", "Any ", "hard,intr ")] Dump IsMpInFstab Log fstab already contains an entry Return true