[libvirt] [PATCH 1 of 2] Add internal cgroup manipulation functions

[Dan Smith]

BS> There is no support for permissions, is everything run as root?

The LXC driver must run as root, yes.

BS> Is 512 arbitrary? How do we know it is going to be sufficient?

This should probably be a constant, but it's more than enough for any
of the values we're setting.  Defining it specifically will help make
that clear.

BS> So every routine calls cgroup_path_of(), reads the mounts entry
BS> and find a entry for cgroup and returns it, why not do it just
BS> once and use it.

Because maintaining the (potentially stale) state isn't worth the
small amount of work necessary to parse the mount table on each domain
creation (IMHO).

BS> I see a mix of open and fopen calls.I would prefer to stick to
BS> just one, helps with readability.

You mean because I use open() and getmntent_r() wants a FILE*?  I
think I'm willing to live with that :)

BS> I don't think 512 bytes will be sufficient. THere are multi-line
BS> files like memory.stat.

We don't read memory.stat (or any other values at the moment).
However:

  % for i in $(find /cgroup | grep memory.stat); do cat $i | wc -c; done
  53
  53
  53
  98

I don't see any point in allocating a huge amount of memory on the
stack each time if we're not going to use it.  If we need to read
larger values in the future, we could move to an allocated string.

BS> Why do you need access? mkdir will do that check anyway.

Yep, I'll change it.

BS> Why 0655? Why not use meaningful names see sys/stat.h

I don't think that the meaningful names in sys/stat.h lead to better
readability, personally.  Octal permissions are used elsewhere int the
code pretty extensively, so I'm comfortable with this as it is.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms at us.ibm.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20081003/90b92ebf/attachment-0001.sig>