Xen devel - Aug 2005 - [PATCH] Convert balloon driver to xenstore

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>






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

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

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

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

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


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

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


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

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>


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

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

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

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
>
>  
>

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

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

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

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


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