param: allow omitting set() methods for truly read-only params

PATCH In review Dmitry Torokhov 2 participants 5 messages Aug 10, 2010 – Aug 11, 2010 Char and misc drivers

Share & tools

Dmitry Torokhov

author

2010-08-10 18:59 UTC #0 lore raw

PATCH 1/2 param: allow omitting set() methods for truly read-only params

Certain "parameters", such as acpica version and upcoming VMware Balloon version, are need to be purely read-only. They are exported as parameters so that they are visible in sysfs even in cases when the module is built directly into the kernel, but their values should be immutable. Specifying S_IRUGO takes care of sysfs interface, but it has no effect on kernel command line or modprobe configuration files and so these "parameters" attempt to omit set() method. Unfortunately kernel expects set() to be always present and crashes if it is not there, so let's add appropriate check.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 kernel/params.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

kernel/params.c +4 -4

index 0b30ecd..d0bb910 100644
@@ -49,7 +49,7 @@ static inline int parameq(const char *input, const char *paramname)
 
 static int parse_one(char *param,
 		     char *val,
-		     struct kernel_param *params, 
+		     struct kernel_param *params,
 		     unsigned num_params,
 		     int (*handle_unknown)(char *param, char *val))
 {

@@ -58,9 +58,9 @@ static int parse_one(char *param,
 	/* Find parameter */
 	for (i = 0; i < num_params; i++) {
 		if (parameq(param, params[i].name)) {
-			DEBUGP("They are equal!  Calling %p\n",
-			       params[i].set);
-			return params[i].set(val, &params[i]);
+			DEBUGP("They are equal! set() is %p\n", params[i].set);
+			return params[i].set ?
+				params[i].set(val, &params[i]) : -EPERM;
 		}
 	}

Signature · 1 line

1.7.0

Dmitry Torokhov

author

2010-08-10 18:59 UTC #1 lore raw

PATCH 2/2 VMware balloon: export module version as module parameter

To allow external tools detect presence of the driver in the kernel when the driver is built in (as opposed to be compiled as a module) export driver version as a read-only module parameter.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/misc/vmware_balloon.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

drivers/misc/vmware_balloon.c +13 -1

index 2a1e804..f9b2221 100644
@@ -43,9 +43,11 @@
 #include <linux/seq_file.h>
 #include <asm/hypervisor.h>
 
+#define VMW_BALLOON_VERSION	"1.2.1.2-k"
+
 MODULE_AUTHOR("VMware, Inc.");
 MODULE_DESCRIPTION("VMware Memory Control (Balloon) Driver");
-MODULE_VERSION("1.2.1.1-k");
+MODULE_VERSION(VMW_BALLOON_VERSION);
 MODULE_ALIAS("dmi:*:svnVMware*:*");
 MODULE_ALIAS("vmware_vmmemctl");
 MODULE_LICENSE("GPL");

@@ -842,3 +844,13 @@ static void __exit vmballoon_exit(void)
 	vmballoon_pop(&balloon);
 }
 module_exit(vmballoon_exit);
+
+/*
+ * Make the driver version exported as module parameter so that the
+ * driver is still visible if it is compiled into kernel
+ */
+static int param_get_vmballoon_version(char *buffer, struct kernel_param *kp)
+{
+	return sprintf(buffer, "%s", VMW_BALLOON_VERSION);
+}
+module_param_call(version, NULL, param_get_vmballoon_version, NULL, S_IRUGO);

Signature · 1 line

1.7.0

Rusty Russell

2010-08-11 01:39 UTC #2 lore raw

In reply to Dmitry Torokhov

Quoted 11 lines · Dmitry Torokhov

Certain "parameters", such as acpica version and upcoming VMware Balloon version, are need to be purely read-only. They are exported as parameters so that they are visible in sysfs even in cases when the module is built directly into the kernel, but their values should be immutable. Specifying S_IRUGO takes care of sysfs interface, but it has no effect on kernel command line or modprobe configuration files and so these "parameters" attempt to omit set() method. Unfortunately kernel expects set() to be always present and crashes if it is not there, so let's add appropriate check.
Signed-off-by Dmitry Torokhov <[email protected]>

That almost makes sense, but not quite.

Your followup patch uses this for version, but the version is already in /sys/module/vmware_balloon/version as is standard with modules, no?

Confused,
Rusty.

Dmitry Torokhov

author

2010-08-11 07:35 UTC #3 lore raw

Quoted 13 lines · Rusty Russell

Quoted 12 lines · Dmitry Torokhov
Certain "parameters", such as acpica version and upcoming VMware Balloon
version, are need to be purely read-only. They are exported as parameters so that they are visible in sysfs even in cases when the module is built directly into the kernel, but their values should be immutable. Specifying S_IRUGO takes care of sysfs interface, but it has no effect on kernel command line or modprobe configuration files and
so these "parameters" attempt to omit set() method. Unfortunately kernel expects set() to be always present and crashes if it is not there, so let's add appropriate check.

Quoted 7 lines

Quoted 1 line
Signed-off-by Dmitry Torokhov <[email protected]>
That almost makes sense, but not quite.
Your followup patch uses this for version, but the version is already in /sys/module/vmware_balloon/version as is standard with modules, no?

That version attribute is not present if the driver is built-in, only parameters are visible in both cases.

Signature · 1 line

Dmitry

Rusty Russell

2010-08-11 10:49 UTC #4 lore raw

Quoted 10 lines · Dmitry Torokhov

Quoted 6 lines · Rusty Russell
Quoted 2 lines · Dmitry Torokhov
Certain "parameters", such as acpica version and upcoming VMware Balloon version, are need to be purely read-only.
Your followup patch uses this for version, but the version is already in /sys/module/vmware_balloon/version as is standard with modules, no?
That version attribute is not present if the driver is built-in, only parameters are visible in both cases.

You know what I'm going to say now then, don't you? :)

Thanks,
Rusty.