Re: about remus and xl patches

[Wen Congyang <wency@cn.fujitsu.com>]

At 12/16/2013 03:57 PM, Shriram Rajagopalan Wrote:
> On Mon, Dec 16, 2013 at 1:04 AM, wencongyang <wencongyang@gmail.com> wrote:
> 
>> hi, shriram
>>
>> i don't use this mail to subscribe to the list. so i don't reply the
>> thread.
>>
>> 1. about the script, you check some modules, but the user can build it
>> into the kernel....
>>
> 
> At the moment, I am only concerned about users who just want to run Remus
> on their stock distribution.
> (Over the last two years, a majority of Remus help requests I have received
> were from people who didnt have the expertise
> or did not want to mess with custom kernels on their test boxes :) ). So I
> figured if someone is smart enough to compile
> a module into the kernel itself, he/she would be easily able to fix the
> script to make Remus work in their setup :).

sch_plug sch_ingress act_mirred cls_u32 will be loaded automatically by tc commands.
So I think there is no need to check them. If these modules doesn't exist
or are built into kernel, tc will fail, and we can report error later.

Thanks
Wen Congyang

> 
> 
> 
>> 2. in the function init_qdisc(), you use nl_socket_alloc() to alloc new
>> netlink socket, but i don't find nl_socket_free()
>>
> 
> Good catch. Thanks!
> 
> 
>> .  You don't cleanup when something fails?
>>
> 
> I do. Check the *_teardown functions. It currently uses nl_close. But I
> should be using nl_socket_free as it seems to invoke
> nl_close too.
> 
> 
>> 3. in the function get_guest_vif_list(), you call get_vifname() in LOG(),
>> you don't free the memory.
>>
>>
>>
> No need to, AFAIK.  get_vifname allocates the string in gc context.
> Besides, get_vif_list (and subsequently get_vifname) is a
> one time call during net buffer setup, i.e., before Remus starts.
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel