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

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

[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 <dtor@vmware.com>
---
 kernel/params.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index 0b30ecd..d0bb910 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -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;
 		}
 	}
 
-- 
1.7.0

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

[Dmitry Torokhov]

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 <dtor@vmware.com>
---
 drivers/misc/vmware_balloon.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/vmware_balloon.c b/drivers/misc/vmware_balloon.c
index 2a1e804..f9b2221 100644
--- a/drivers/misc/vmware_balloon.c
+++ b/drivers/misc/vmware_balloon.c
@@ -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);
-- 
1.7.0

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

[Rusty Russell]

On Wed, 11 Aug 2010 04:28:44 am Dmitry Torokhov wrote:
> 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 <dtor@vmware.com>

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.

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

[Dmitry Torokhov]

On Aug 10, 2010, at 6:39 PM, "Rusty Russell" <rusty@rustcorp.com.au>  
wrote:

> On Wed, 11 Aug 2010 04:28:44 am Dmitry Torokhov wrote:
>> 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 <dtor@vmware.com>
>
> 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.

-- 
Dmitry

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

[Rusty Russell]

On Wed, 11 Aug 2010 05:05:51 pm Dmitry Torokhov wrote:
> On Aug 10, 2010, at 6:39 PM, "Rusty Russell" <rusty@rustcorp.com.au> wrote:
> > On Wed, 11 Aug 2010 04:28:44 am Dmitry Torokhov wrote:
> >> 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.