"Daniel P. Berrange" <berrange@xxxxxxxxxx> writes:
> On Wed, Aug 12, 2015 at 11:09:12AM +0530, Nikunj A Dadhania wrote:
>>
>> Hi Daniel,
>>
>> "Daniel P. Berrange" <berrange@xxxxxxxxxx> writes:
>> > On Tue, Aug 11, 2015 at 04:57:15PM +0530, Nikunj A Dadhania wrote:
>> >> The default resource partition is created in the domain start path if it
>> >> is not existing. Even when libvirtd is stopped after shutting down all
>> >> domains, the resource partition still exists.
>> >>
>> >> The patch adds code to removes the default resource partition in the
>> >> cgroup removal path of the domain. If the default resource partition is
>> >> found to have no child cgroup, the default resource partition will be
>> >> removed.
>> >>
>> >> Moreover, the code does not remove the user provided resource
>> >> partitions.
>> >>
>> >> Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxxxxxxxxxxxxx>
>> >
>> > I don't think we want to be doing this. In non-systemd hosts this will
>> > be deleting the heirarchy that the sysadmin manually pre-created for
>> > their VMs. In a systemd host it will also end up deleting slices that
>> > were created by systemd.
>>
>> AFAIU, there are three cases here:
>>
>> 1) User created resource partition, for example /production/foo
>> As this is created by user, we should not touch them. And my patch
>> does not remove them
>>
>> 2) systemd created /machine.slice
>> If not libvirt, should systemd clean this up when the libvirtd
>> service is stopped ?
>>
>> Currently, my patch does remove this when its found empty
>
> It isn't libvirtd's job to delete /machine.slice - systemd will
> periodically prune empty slices itself.
Before runnning machine.slice
.
├── blkio
│ ├── system.slice
│ └── user.slice
├── cpu -> cpu,cpuacct
├── cpuacct -> cpu,cpuacct
├── cpu,cpuacct
│ ├── system.slice
│ └── user.slice
├── cpuset
├── devices
│ ├── system.slice
│ └── user.slice
├── freezer
├── hugetlb
├── memory
│ ├── system.slice
│ └── user.slice
├── net_cls -> net_cls,net_prio
├── net_cls,net_prio
├── net_prio -> net_cls,net_prio
├── perf_event
└── systemd
├── system.slice
└── user.slice
After starting systemd-machined:
.
├── blkio
│ ├── machine.slice
│ ├── system.slice
│ └── user.slice
├── cpu -> cpu,cpuacct
├── cpuacct -> cpu,cpuacct
├── cpu,cpuacct
│ ├── machine.slice
│ ├── system.slice
│ └── user.slice
├── cpuset
├── devices
│ ├── machine.slice
│ ├── system.slice
│ └── user.slice
├── freezer
├── hugetlb
├── memory
│ ├── machine.slice
│ ├── system.slice
│ └── user.slice
├── net_cls -> net_cls,net_prio
├── net_cls,net_prio
├── net_prio -> net_cls,net_prio
├── perf_event
└── systemd
├── machine.slice
├── system.slice
└── user.slice
systemd leaves out cpuset/freezer. So these are basically created by
libvirt and not by systemd. When I went through systemd code, it seems
cpuset/freezer is disabled on purpose.
systemd/src/core/main.c
static int initialize_join_controllers(void) {
/* By default, mount "cpu" + "cpuacct" together, and "net_cls"
* + "net_prio". We'd like to add "cpuset" to the mix, but
* "cpuset" doesn't really work for groups with no initialized
* attributes. */
Some more details here:
http://www.freedesktop.org/wiki/Software/systemd/ControlGroupInterface/
Note that the number of cgroup attributes currently exposed as
unit properties is limited. This will be extended later on, as
their kernel interfaces are cleaned up. For example cpuset or
freezer are currently not exposed at all due to the broken
inheritance semantics of the kernel logic.
>> 3) libvirt created /machine
>> As this was created manually by libvirt, should we delete it here in
>> libvirt daemon
>
> No, you can't assume /machine is created by libvirtd - it could have
> been created by the user, just like case 3.
I was thinking of an idea to create a stub directory "libvirt-owned" in
the machine/machine.slice if libvirt created this directory. So while on
the exit path if we find the directory empty and is owned by libvirt, we
can clean that up.
Regards,
Nikunj
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list