[PATCH] Convert balloon driver to xenstore

[PATCH] Convert balloon driver to xenstore

[Dan Smith]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

The attached patch converts the balloon driver and xend to use
xenstore instead of control messages.

Note: Because there is no way to set a watch on a non-existent key,
this patch includes a workaround to account for the fact that dom0's
store keys are not initialized by the tools before it boots.

Signed-off-by: Dan Smith <danms@us.ibm.com>


[-- Attachment #2: balloon.patch --]
[-- Type: text/plain, Size: 9121 bytes --]

diff -r 0db6a59abb95 -r b3ead8c9affb linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c
--- a/linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c	Mon Aug  1 10:48:24 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c	Mon Aug  1 17:03:55 2005
@@ -5,6 +5,7 @@
  *
  * Copyright (c) 2003, B Dragovic
  * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
  * 
  * This file may be distributed separately from the Linux kernel, or
  * incorporated into other software packages, subject to the following license:
@@ -42,7 +43,6 @@
 #include <linux/vmalloc.h>
 #include <asm-xen/xen_proc.h>
 #include <asm-xen/hypervisor.h>
-#include <asm-xen/ctrl_if.h>
 #include <asm-xen/balloon.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
@@ -50,6 +50,10 @@
 #include <asm/tlb.h>
 #include <linux/list.h>
 
+#include<asm-xen/xenbus.h>
+
+#define PAGES2KB(_p) ((_p)<<(PAGE_SHIFT-10))
+
 static struct proc_dir_entry *balloon_pde;
 
 static DECLARE_MUTEX(balloon_mutex);
@@ -77,11 +81,17 @@
 static DECLARE_WORK(balloon_worker, balloon_process, NULL);
 static struct timer_list balloon_timer;
 
+/* Flag for dom0 xenstore workaround */
+static int balloon_xenbus_init=0;
+
+/* Init Function */
+void balloon_init_watcher(void);
+
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
 /* Use the private and mapping fields of struct page as a list. */
 #define PAGE_TO_LIST(p) ( (struct list_head *)&p->private )
 #define LIST_TO_PAGE(l) ( list_entry( ((unsigned long *)l),   \
-				      struct page, private ) )
+                                      struct page, private ) )
 #define UNLIST_PAGE(p)  do { list_del(PAGE_TO_LIST(p));       \
                              p->mapping = NULL;               \
                              p->private = 0; } while(0)
@@ -297,25 +307,96 @@
     schedule_work(&balloon_worker);
 }
 
-static void balloon_ctrlif_rx(ctrl_msg_t *msg, unsigned long id)
-{
-    switch ( msg->subtype )
-    {
-    case CMSG_MEM_REQUEST_SET:
-    {
-        mem_request_t *req = (mem_request_t *)&msg->msg[0];
-        set_new_target(req->target);
-        req->status = 0;
-    }
-    break;        
-
-    default:
-        msg->length = 0;
-        break;
-    }
-
-    ctrl_if_send_response(msg);
-}
+static struct xenbus_watch xb_watch =
+{
+    .node = "memory"
+};
+
+/* FIXME: This is part of a dom0 sequencing workaround */
+static struct xenbus_watch root_watch =
+{
+    .node = "/"
+};
+
+/* React to a change in the target key */
+static void watch_target(struct xenbus_watch *watch, const char *node)
+{
+    unsigned long new_target;
+    int err;
+
+    if(watch == &root_watch)
+    {
+        /* FIXME: This is part of a dom0 sequencing workaround */
+        if(register_xenbus_watch(&xb_watch) == 0)
+        {
+            /* 
+               We successfully set a watch on memory/target:
+               now we can stop watching root 
+            */
+            unregister_xenbus_watch(&root_watch);
+            balloon_xenbus_init=1;
+        } 
+        else 
+        {
+            return;
+        }
+    }
+
+    err = xenbus_scanf("memory", "target", "%lu", &new_target);
+        
+    if(err != 1) 
+    {
+        IPRINTK("Unable to read memory/target\n");
+        return;
+    } 
+        
+    set_new_target(new_target >> PAGE_SHIFT);
+    
+}
+
+/* 
+   Try to set up our watcher, if not already set
+   
+*/
+void balloon_init_watcher(void) 
+{
+    int err;
+
+    if(!xen_start_info.store_evtchn)
+    {
+        IPRINTK("Delaying watcher init until xenstore is available\n");
+        return;
+    }
+
+    down(&xenbus_lock);
+
+    if(! balloon_xenbus_init) 
+    {
+        err = register_xenbus_watch(&xb_watch);
+        if(err) 
+        {
+            /* BIG FAT FIXME: dom0 sequencing workaround
+             * dom0 can't set a watch on memory/target until
+             * after the tools create it.  So, we have to watch
+             * the whole store until that happens.
+             *
+             * This will go away when we have the ability to watch
+             * non-existant keys
+             */
+            register_xenbus_watch(&root_watch);
+        } 
+        else
+        {
+            IPRINTK("Balloon xenbus watcher initialized\n");
+            balloon_xenbus_init = 1;
+        }
+    }
+
+    up(&xenbus_lock);
+
+}
+
+EXPORT_SYMBOL(balloon_init_watcher);
 
 static int balloon_write(struct file *file, const char __user *buffer,
                          unsigned long count, void *data)
@@ -346,7 +427,6 @@
 {
     int len;
 
-#define K(_p) ((_p)<<(PAGE_SHIFT-10))
     len = sprintf(
         page,
         "Current allocation: %8lu kB\n"
@@ -354,13 +434,14 @@
         "Low-mem balloon:    %8lu kB\n"
         "High-mem balloon:   %8lu kB\n"
         "Xen hard limit:     ",
-        K(current_pages), K(target_pages), K(balloon_low), K(balloon_high));
+        PAGES2KB(current_pages), PAGES2KB(target_pages), 
+        PAGES2KB(balloon_low), PAGES2KB(balloon_high));
 
     if ( hard_limit != ~0UL )
         len += sprintf(
             page + len, 
             "%8lu kB (inc. %8lu kB driver headroom)\n",
-            K(hard_limit), K(driver_pages));
+            PAGES2KB(hard_limit), PAGES2KB(driver_pages));
     else
         len += sprintf(
             page + len,
@@ -396,9 +477,7 @@
 
     balloon_pde->read_proc  = balloon_read;
     balloon_pde->write_proc = balloon_write;
-
-    (void)ctrl_if_register_receiver(CMSG_MEM_REQUEST, balloon_ctrlif_rx, 0);
-
+    
     /* Initialise the balloon with excess memory space. */
     for ( pfn = xen_start_info.nr_pages; pfn < max_pfn; pfn++ )
     {
@@ -406,6 +485,11 @@
         if ( !PageReserved(page) )
             balloon_append(page);
     }
+
+    xb_watch.callback = watch_target;
+    root_watch.callback = watch_target;
+
+    balloon_init_watcher();
 
     return 0;
 }
diff -r 0db6a59abb95 -r b3ead8c9affb linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Mon Aug  1 10:48:24 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Mon Aug  1 17:03:55 2005
@@ -309,6 +309,9 @@
 		return err;
 	}
 
+	/* Initialize non-xenbus drivers */
+	balloon_init_watcher();
+
 	down(&xenbus_lock);
 	/* Enumerate devices in xenstore. */
 	xenbus_probe_devices("device");
diff -r 0db6a59abb95 -r b3ead8c9affb tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Aug  1 10:48:24 2005
+++ b/tools/python/xen/xend/XendDomainInfo.py	Mon Aug  1 17:03:55 2005
@@ -152,6 +152,9 @@
         vm = cls(db)
         vm.construct(config)
         vm.saveToDB(sync=True)
+        # Flush info to xenstore immediately
+        vm.exportToDB()
+
         return vm
 
     create = classmethod(create)
@@ -172,6 +175,7 @@
         log.debug('config=' + prettyprintstring(config))
 
         vm.memory = info['mem_kb']/1024
+        vm.target = info['mem_kb'] * 1024
 
         if config:
             try:
@@ -222,6 +226,7 @@
         DBVar('restart_state', ty='str'),
         DBVar('restart_time',  ty='float'),
         DBVar('restart_count', ty='int'),
+        DBVar('target',        ty='long', path="memory/target"),
         ]
     
     def __init__(self, db):
@@ -239,6 +244,8 @@
         self.memory = None
         self.ssidref = None
         self.image = None
+
+        self.target = None
 
         self.channel = None
         self.store_channel = None
@@ -315,6 +322,7 @@
         self.info = info
         self.memory = self.info['mem_kb'] / 1024
         self.ssidref = self.info['ssidref']
+        self.target = self.info['mem_kb'] * 1024
 
     def state_set(self, state):
         self.state_updated.acquire()
@@ -399,7 +407,8 @@
                 ['id', self.id],
                 ['name', self.name],
                 ['memory', self.memory],
-                ['ssidref', self.ssidref] ]
+                ['ssidref', self.ssidref],
+                ['target', self.target] ]
         if self.uuid:
             sxpr.append(['uuid', self.uuid])
         if self.info:
@@ -536,6 +545,7 @@
         self.memory = int(sxp.child_value(config, 'memory'))
         if self.memory is None:
             raise VmError('missing memory size')
+        self.target = self.memory * (1 << 20)
         self.ssidref = int(sxp.child_value(config, 'ssidref'))
         cpu = sxp.child_value(config, 'cpu')
         if self.recreate and self.id and cpu is not None and int(cpu) >= 0:
@@ -947,11 +957,12 @@
             index[field_name] = field_index + 1
 
     def mem_target_set(self, target):
-        """Set domain memory target in pages.
-        """
-        if self.channel:
-            msg = messages.packMsg('mem_request_t', { 'target' : target * (1 << 8)} )
-            self.channel.writeRequest(msg)
+        """Set domain memory target in bytes.
+        """
+        if target:
+            self.target = target * (1 << 20)
+            # Commit to XenStore immediately
+            self.exportToDB()
 
     def vcpu_hotplug(self, vcpu, state):
         """Disable or enable VCPU in domain.

[-- Attachment #3: Type: text/plain, Size: 88 bytes --]


-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Convert balloon driver to xenstore

[aq]

On 8/2/05, Dan Smith <danms@us.ibm.com> wrote:
> The attached patch converts the balloon driver and xend to use
> xenstore instead of control messages.
> 

great. but is there a better way to initialize watcher for balloon?
will we always have to patch xenbus code each time we add a driver
into the kernel?


regards,
aq

Re: [PATCH] Convert balloon driver to xenstore

[Dan Smith]

aq> great. but is there a better way to initialize watcher for
aq> balloon?  

Not at the moment, as I understand it.  Rusty recommended calling the
init method directly from the xenbus probe, since the balloon driver
doesn't (and shouldn't) register itself as a normal xenbus driver
(like blk{front,back}).

aq> will we always have to patch xenbus code each time we add a driver
aq> into the kernel?

I think we need a standardized way to initialize miscellaneous drivers
such as the balloon.  It's definitely a temporary solution to a
problem that will require more thought as we get more drivers
converted to xenbus.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com

Re: [PATCH] Convert balloon driver to xenstore

[Rusty Russell]

On Tue, 2005-08-02 at 06:51 -0700, Dan Smith wrote:

> aq> will we always have to patch xenbus code each time we add a driver
> aq> into the kernel?
> 
> I think we need a standardized way to initialize miscellaneous drivers
> such as the balloon.  It's definitely a temporary solution to a
> problem that will require more thought as we get more drivers
> converted to xenbus.

Generally, you will want to do a module_init() and initialize your
driver.  If it's in domain 0, it needs to be initialized later (the
store isn't up yet): the standard way of doing this is a notifier chain.

Something like the following:

static struct notifier_block store_notify =
{
	.notifier_call = balloon_setup,
};

static int balloon_init()
{
	if (!xen_start_info.store_evtchn)
		register_xenstore_notifier(&store_notify);
	else
		balloon_setup(&store_notify, 0, NULL);
	return 0;
}
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: [PATCH] Convert balloon driver to xenstore

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

Rusty Russell wrote:

>On Tue, 2005-08-02 at 06:51 -0700, Dan Smith wrote:
>  
>
>Generally, you will want to do a module_init() and initialize your
>driver.  If it's in domain 0, it needs to be initialized later (the
>store isn't up yet): the standard way of doing this is a notifier chain.
>
>Something like the following:
>
>static struct notifier_block store_notify =
>{
>	.notifier_call = balloon_setup,
>};
>
>static int balloon_init()
>{
>	if (!xen_start_info.store_evtchn)
>		register_xenstore_notifier(&store_notify);
>	else
>		balloon_setup(&store_notify, 0, NULL);
>	return 0;
>}
>  
>
How does this look?

Regards,

Anthony Liguori

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

[-- Attachment #2: xenbus_notifier.diff --]
[-- Type: text/x-patch, Size: 2385 bytes --]

diff -r dcdcec634c2d linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Tue Aug  2 16:11:31 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Tue Aug  2 21:09:03 2005
@@ -35,9 +35,15 @@
 #include <linux/ctype.h>
 #include <linux/fcntl.h>
 #include <stdarg.h>
+#include <linux/notifier.h>
 #include "xenbus_comms.h"
 
 #define streq(a, b) (strcmp((a), (b)) == 0)
+
+/* Protects notifier chain */
+DECLARE_MUTEX(xenstore_control);
+
+static struct notifier_block *xenstore_chain;
 
 /* If something in array of ids matches this device, return it. */
 static const struct xenbus_device_id *
@@ -295,6 +301,26 @@
 	.callback = dev_changed,
 };
 
+int register_xenstore_notifier(struct notifier_block *nb)
+{
+	int ret;
+
+	if ((ret = down_interruptible(&xenstore_control)) != 0) 
+		return ret;
+	ret = notifier_chain_register(&xenstore_chain, nb);
+	up(&xenstore_control);
+	return ret;
+}
+EXPORT_SYMBOL(register_xenstore_notifier);
+
+void unregister_xenstore_notifier(struct notifier_block *nb)
+{
+	down(&xenstore_control);
+	notifier_chain_unregister(&xenstore_chain, nb);
+	up(&xenstore_control);
+}
+EXPORT_SYMBOL(unregister_xenstore_notifier);
+
 /* called from a thread in privcmd/privcmd.c */
 int do_xenbus_probe(void *unused)
 {
@@ -309,6 +335,15 @@
 		return err;
 	}
 
+	err = notifier_call_chain(&xenstore_chain, 0, 0);
+	if (err == NOTIFY_BAD) {
+		printk("%s: calling xenstore notify chain failed\n",
+		       __FUNCTION__);
+		return -EINVAL;
+	}
+
+	err = 0;
+
 	/* Initialize non-xenbus drivers */
 	balloon_init_watcher();
 
diff -r dcdcec634c2d linux-2.6-xen-sparse/include/asm-xen/xenbus.h
--- a/linux-2.6-xen-sparse/include/asm-xen/xenbus.h	Tue Aug  2 16:11:31 2005
+++ b/linux-2.6-xen-sparse/include/asm-xen/xenbus.h	Tue Aug  2 21:09:03 2005
@@ -29,6 +29,7 @@
  * IN THE SOFTWARE.
  */
 #include <linux/device.h>
+#include <linux/notifier.h>
 #include <asm/semaphore.h>
 
 /* A xenbus device. */
@@ -112,6 +113,10 @@
 	void (*callback)(struct xenbus_watch *, const char *node);
 };
 
+/* notifer routines for when the xenstore comes up */
+int register_xenstore_notifier(struct notifier_block *nb);
+void unregister_xenstore_notifier(struct notifier_block *nb);
+
 int register_xenbus_watch(struct xenbus_watch *watch);
 void unregister_xenbus_watch(struct xenbus_watch *watch);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Convert balloon driver to xenstore

[aq]

On 8/3/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Rusty Russell wrote:
> 
> >On Tue, 2005-08-02 at 06:51 -0700, Dan Smith wrote:
> >
> >
> >Generally, you will want to do a module_init() and initialize your
> >driver.  If it's in domain 0, it needs to be initialized later (the
> >store isn't up yet): the standard way of doing this is a notifier chain.
> >
> >Something like the following:
> >
> >static struct notifier_block store_notify =
> >{
> >       .notifier_call = balloon_setup,
> >};
> >
> >static int balloon_init()
> >{
> >       if (!xen_start_info.store_evtchn)
> >               register_xenstore_notifier(&store_notify);
> >       else
> >               balloon_setup(&store_notify, 0, NULL);
> >       return 0;
> >}
> >
> >
> How does this look?
> 

looks nice to me. why didnt you provice also the patch for balloon ;-)
 
regards,
aq

Re: [PATCH] Convert balloon driver to xenstore

[Anthony Liguori]

aq wrote:

>looks nice to me. why didnt you provice also the patch for balloon ;-)
>
Can't make things too easy for Dan Smith :-)

He's already updating the balloon driver code to use the non-existent 
store watches.  Easier to make the changes there than to try to remerge 
against another code base.

Regards,

Anthony Liguori

> 
>regards,
>aq
>
>  
>

Re: [PATCH] Convert balloon driver to xenstore

[aq]

On 8/2/05, Dan Smith <danms@us.ibm.com> wrote:
> The attached patch converts the balloon driver and xend to use
> xenstore instead of control messages.
> 
<snip>
> +static void watch_target(struct xenbus_watch *watch, const char *node)
> +{
> +    unsigned long new_target;
> +    int err;
> +
> +    if(watch == &root_watch)
> +    {
> +        /* FIXME: This is part of a dom0 sequencing workaround */
> +        if(register_xenbus_watch(&xb_watch) == 0)
> +        {
> +            /*
> +               We successfully set a watch on memory/target:
> +               now we can stop watching root
> +            */
> +            unregister_xenbus_watch(&root_watch);
> +            balloon_xenbus_init=1;
> +        }
> +        else
> +        {
> +            return;
> +        }
> +    }
> +
> +    err = xenbus_scanf("memory", "target", "%lu", &new_target);
> +
> +    if(err != 1)
> +    {
> +        IPRINTK("Unable to read memory/target\n");
> +        return;
> +    }
> +
> +    set_new_target(new_target >> PAGE_SHIFT);

as under "memory"node, there probably are several nodes besides
"target" (that means what triggered this watch may be not balloon
request), i guess it is better to do some checking before calling
set_new_target, like this:


if (new_target != target_pages)
    set_new_target(new_target >> PAGE_SHIFT);


regards,
aq

Re: [PATCH] Convert balloon driver to xenstore

[Dan Smith]

aq> as under "memory"node, there probably are several nodes besides
aq> "target" 

There probably /will/ be several nodes besides target, but there
aren't /yet/ :)

aq> (that means what triggered this watch may be not balloon
aq> request), i guess it is better to do some checking before calling
aq> set_new_target, like this:

If the desired behavior is really to only perform the watch duties
when a specific key is changed, I think it would be better to check
the node parameter of the watch handler.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com