On Thu, 2012-10-18 at 10:21 +0200, Lukáš Doktor wrote: > On 10/18/2012 09:07 AM, Satheesh Rajendran wrote: > > Thanks for the review. > > > > On Wed, 2012-10-17 at 14:27 +0200, Lukáš Doktor wrote: > >> Hi Sateesh, > >> > >> yes I guess now it's the time to create cgroup library and put all > >> related functions in one place. Would you like to do it, Satheesh, or > >> should I? > >> > > Hi Lukáš, > > > > Sure, I can start doing this and we can make modification based on the > > review. > > > > I have in my mind following things: > > > > 1) To pull out the functionality available in > > client/tests/cgroup/cgroup_common.py asis and place it in > > client/base_utils.py. > > 2) Change cgroup.py accordingly to make it work. > > > > (Or) > > 1) Move the client/tests/cgroup/cgroup_common.py to > > client/cgroup_common.py > > 2) Change cgroup.py accordingly to make it work. > > > > 3) Modify and add the below patch(function) there. > > > > Please provide your comments here. > Dear Satheesh, > > I'd go with the second proposal. Also I'd perhaps use cgroup_utils.py > instead of cgroup_common.py to suit autotest notation. What do you > think, Lucas? > Sure, that makes sense, to use cgroup_utils.py. I have created a issue tracker for tracking this. https://github.com/autotest/autotest/issues/574
Would continue to work on this. Regards, -Satheesh. > cgroup_common is used in: > client/tests/cgroup/cgroup.py > client/tests/virt/kvm/tests/cgroup.py > > The change is pretty straight forward. Anyway always keep in mind that > multiple subsystems might be monted in the same directory. Thank you for > doing this, I'm currently overwhelmed with virtio_console :-) > > Regards, > Lukáš > > > > > > > > > >> Anyway your code has one small issue, please find it bellow... > >> > >> On 10/12/2012 07:34 AM, Satheesh Rajendran wrote: > >>> On Thu, 2012-10-11 at 09:55 -0300, Lucas Meneghel Rodrigues wrote: > >>>> Sateesh, > >>>> > >>>> I wonder if you ever saw the Cgroup class that was developed by > >>>> ldoktor. Currently it is on a test module > >>>> > >>> Yes, I had gone through them, this particular functionality is not > >>> available AFIK. > >>> > >>>> https://github.com/autotest/autotest-client-tests/blob/master/cgroup/cgroup_common.py > >>>> https://github.com/autotest/autotest-client-tests/blob/master/cgroup/cgroup_client.py > >>>> > >>>> But we could move them into the client (or shared) library. Lukas' > >>>> implementation takes care of of a good deal of what you want to do > >>>> with cgroups. > >>> Agree, As you said these files have many other useful functions which > >>> will aid for any cgroup related tests, please consider moving them in to > >>> the shared library. > >>> > >>>> Please let me know what you think, > >>>> > >>> This particular patch mainly intended to resolve the controller path of > >>> a given task id, where in there is already a created controller group, > >>> which is the case for VM's created by libvirt, and this patch will aid > >>> the future virsh patchs like memtune, blkiotune etc... > >>> > >>> Let me know your view, if fine, please consider pushing it after > >>> review. > >>> > >>> Regards, > >>> -Satheesh. > >>>> Lucas > >>>> > >>>> On Thu, Oct 11, 2012 at 6:47 AM, <[email protected]> wrote: > >>>>> From: Satheesh Rajendran <[email protected]> > >>>>> > >>>>> This function would provide a support for resolving a cgroup path for a > >>>>> specific controller and a given task id. > >>>>> > >>>>> > >>>>> > >>>>> Signed-off-by: Satheesh Rajendran <[email protected]> > >>>>> --- > >>>>> client/base_utils.py | 36 ++++++++++++++++++++++++++++++++++++ > >>>>> 1 files changed, 36 insertions(+), 0 deletions(-) > >>>>> > >>>>> diff --git a/client/base_utils.py b/client/base_utils.py > >>>>> index 5e45efd..8485142 100644 > >>>>> --- a/client/base_utils.py > >>>>> +++ b/client/base_utils.py > >>>>> @@ -825,3 +825,39 @@ def suspend_to_disk(): > >>>>> Suspend the system to disk (S4) > >>>>> """ > >>>>> set_power_state('disk') > >>>>> + > >>>>> + > >>>>> +def resolve_task_cgroup_path(pid, controller): > >>>>> + """ > >>>>> + Resolving cgroup mount path of a particular task > >>>>> + > >>>>> + @params: pid : process id of a task for which the cgroup path > >>>>> required > >>>>> + @params: controller: takes one of the controller names in > >>>>> controller list > >>>>> + > >>>>> + @return: resolved path for cgroup controllers of a given pid > >>>>> + """ > >>>>> + > >>>>> + # Initialise cgroup controller list > >>>>> + controller_list = [ 'cpuacct', 'cpu', 'memory', 'cpuset', > >>>>> + 'devices', 'freezer', 'blkio', 'netcls' ] > >>>>> + > >>>>> + if controller not in controller_list: > >>>>> + raise error.TestError("Doesn't support controller <%s>" % > >>>>> controller) > >>>>> + > >>>>> + root_path = get_cgroup_mountpoint(controller) > >>>>> + > >>>>> + proc_cgroup = "/proc/%d/cgroup" % pid > >>>>> + if not os.path.isfile(proc_cgroup): > >>>>> + raise NameError('File %s does not exist\n Check whether cgroup > >>>>> \ > >>>>> + installed in the system' % > >>>>> proc_cgroup) > >>>>> + > >>>>> + f = open(proc_cgroup, 'r') > >>>>> + proc_cgroup_txt = f.read() > >>>>> + f.close > >>>>> + > >>>>> + mount_path = re.findall(r":%s:(\S*)\n" % controller, > >>>>> proc_cgroup_txt) > >> This regext won't work for multiple cgroups (fedora uses this by default > >> for cpuacct,cpu) and users are allowed to do so. One solution comes to > >> my mind: > >> re.findall(r":([^:]*,)*%s(,[^:]*)*:(\S*)\n" % controller, > >> proc_cgroup_txt)[0][2] > >> > > Thanks, I would consider this scenario, modify the patch and send next > > version accordingly. > > > > Regards, > > -Satheesh. > > > >> Than you have to pass only the third argument. I don't know how to > >> specify groups without putting them into the output. > >> > >> Tested on proc_cgroup_txt = """9:perf_event:/ > >> 8:blkio:/ > >> 7:net_cls:/ > >> 6:freezer:/ > >> 5:devices:/ > >> 4:memory:/ > >> 3:cpuacct,cpu:/system/dbus.service > >> 2:cpuset:/ > >> 1:name=systemd:/system/dbus.service > >> """ > >> > >> Kind regards, > >> Lukáš > >>>>> + logging.info(mount_path[0]) > >>>>> + > >>>>> + path = root_path + mount_path[0] > >>>>> + logging.info(path) > >>>>> + return path > >>>>> -- > >>>>> 1.7.1 > >>>>> > >>>>> _______________________________________________ > >>>>> Autotest-kernel mailing list > >>>>> [email protected] > >>>>> https://www.redhat.com/mailman/listinfo/autotest-kernel > >>>> > >>> _______________________________________________ > >>> Autotest-kernel mailing list > >>> [email protected] > >>> https://www.redhat.com/mailman/listinfo/autotest-kernel > > > _______________________________________________ Autotest-kernel mailing list [email protected] https://www.redhat.com/mailman/listinfo/autotest-kernel
