[PATCH] xen: make sysfs files behave as their names suggest

[PATCH] xen: make sysfs files behave as their names suggest

[Jeremy Fitzhardinge]

1: make "target_kb" only accept and produce a memory size in kilobytes.
2: add a second "target" file which produces output in bytes, and will accept
   memparse input (scaled bytes)

This fixes the rather irritating problem that writing the same value
read back into target_kb would end up shrinking the domain by a factor
of 1024, with generally bad results.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 drivers/xen/balloon.c |   33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

===================================================================
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -586,7 +586,7 @@
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
-	target_bytes = memparse(buf, &endchar);
+	target_bytes = simple_strtoull(buf, &endchar, 0) * 1024;
 
 	balloon_set_new_target(target_bytes >> PAGE_SHIFT);
 
@@ -596,8 +596,39 @@
 static SYSDEV_ATTR(target_kb, S_IRUGO | S_IWUSR,
 		   show_target_kb, store_target_kb);
 
+
+static ssize_t show_target(struct sys_device *dev, struct sysdev_attribute *attr,
+			      char *buf)
+{
+	return sprintf(buf, "%llu\n",
+		       (u64)balloon_stats.target_pages << PAGE_SHIFT);
+}
+
+static ssize_t store_target(struct sys_device *dev,
+			    struct sysdev_attribute *attr,
+			    const char *buf,
+			    size_t count)
+{
+	char *endchar;
+	unsigned long long target_bytes;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	target_bytes = memparse(buf, &endchar);
+
+	balloon_set_new_target(target_bytes >> PAGE_SHIFT);
+
+	return count;
+}
+
+static SYSDEV_ATTR(target, S_IRUGO | S_IWUSR,
+		   show_target, store_target);
+
+
 static struct sysdev_attribute *balloon_attrs[] = {
 	&attr_target_kb,
+	&attr_target,
 };
 
 static struct attribute *balloon_info_attrs[] = {

Re: [PATCH] xen: make sysfs files behave as their names suggest

[Simon Horman]

On Wed, Jan 28, 2009 at 04:50:20PM -0800, Jeremy Fitzhardinge wrote:
> 1: make "target_kb" only accept and produce a memory size in kilobytes.
> 2: add a second "target" file which produces output in bytes, and will accept
>   memparse input (scaled bytes)
>
> This fixes the rather irritating problem that writing the same value
> read back into target_kb would end up shrinking the domain by a factor
> of 1024, with generally bad results.

Are there any compatibility issues that we should care about
related to this change?

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en

Re: [PATCH] xen: make sysfs files behave as their names suggest

[Jeremy Fitzhardinge]

Simon Horman wrote:
> On Wed, Jan 28, 2009 at 04:50:20PM -0800, Jeremy Fitzhardinge wrote:
>   
>> 1: make "target_kb" only accept and produce a memory size in kilobytes.
>> 2: add a second "target" file which produces output in bytes, and will accept
>>   memparse input (scaled bytes)
>>
>> This fixes the rather irritating problem that writing the same value
>> read back into target_kb would end up shrinking the domain by a factor
>> of 1024, with generally bad results.
>>     
>
> Are there any compatibility issues that we should care about
> related to this change?
>   

Well, in theory, but not in practice I think.

It changes the behaviour of target_kb from accepting bytes into 
kilobytes, and it no longer parses a k/m/g suffix.  The old behaviour 
was a definite bug, given the name of the file, so I consider this to be 
pure bugfix.  The kernel introducing this interface has only been out 
for a week or two, so I don't think there's much chance anyone has 
started relying on the buggy behaviour.

    J

Re: [PATCH] xen: make sysfs files behave as their names suggest

[Simon Horman]

On Wed, Jan 28, 2009 at 08:31:25PM -0800, Jeremy Fitzhardinge wrote:
> Simon Horman wrote:
>> On Wed, Jan 28, 2009 at 04:50:20PM -0800, Jeremy Fitzhardinge wrote:
>>   
>>> 1: make "target_kb" only accept and produce a memory size in kilobytes.
>>> 2: add a second "target" file which produces output in bytes, and will accept
>>>   memparse input (scaled bytes)
>>>
>>> This fixes the rather irritating problem that writing the same value
>>> read back into target_kb would end up shrinking the domain by a factor
>>> of 1024, with generally bad results.
>>>     
>>
>> Are there any compatibility issues that we should care about
>> related to this change?
>>   
>
> Well, in theory, but not in practice I think.
>
> It changes the behaviour of target_kb from accepting bytes into  
> kilobytes, and it no longer parses a k/m/g suffix.  The old behaviour  
> was a definite bug, given the name of the file, so I consider this to be  
> pure bugfix.  The kernel introducing this interface has only been out  
> for a week or two, so I don't think there's much chance anyone has  
> started relying on the buggy behaviour.

Thanks for the clarification. I have no objections.

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en

Re: [PATCH] xen: make sysfs files behave as their names suggest

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> 1: make "target_kb" only accept and produce a memory size in kilobytes.
> 2: add a second "target" file which produces output in bytes, and will accept
>   memparse input (scaled bytes)
>
> This fixes the rather irritating problem that writing the same value
> read back into target_kb would end up shrinking the domain by a factor
> of 1024, with generally bad results.
>
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> ---
> drivers/xen/balloon.c |   33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)

applied to tip/x86/urgent, thanks Jeremy!

	Ingo