On Monday, 20 February 2017 at 5:07 PM, Martin Kletzander wrote:

On Mon, Feb 20, 2017 at 04:32:19PM +0800, Eli Qiao wrote:

--

Best regards

Eli

天涯无处不重逢

a leaf duckweed belongs to the sea, where not to meet in life

Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Sunday, 19 February 2017 at 10:51 PM, Marcelo Tosatti wrote:

Hi Eli Qiao,

Question about removing resctrlfs directories with empty "tasks" file.

/* This domain is not created by libvirt, so we don't care

* about the tasks, add a fake one to prevent

* virResCtrlRefresh

* remove it from sysfs */

virResCtrlAddTask(p, 1);

virResCtrlAppendDomain(p);

/* Refresh all domains', remove the domains which has no task ids.

* This will be used after VM pause, restart, destroy etc.

*/

static int

virResCtrlRefresh(void)

Why are you doing this (removing directories which have no tasks in

them) ? The code should only read information from other

CAT reservations, not write to them.

When a VM get shutdown, their pids will disappear from resctrl’s directory, we can check pids to see if VM get KIlled or not.

The reason why I would like check if tasks file empty is that: there maybe cases that libvirtd not tracking VM’s status, and the VM gone, left a empty directory with no tasks assigned, and cause resource leak.

I think I can refine this later, keep a pointer of ResDomain in VM’s private data and use that to destroy VM’s resctrl directory.

but this won’t work for exception, still resource leak happened.

Function qemuProcessStop() will *always* be called to cleanup after VMs,

all exceptions should be taken into account, so don't worry, just do

proper cleanup.

This is not for cleanup purposes, since on VM shutdown the resctrlfs

directories are properly removed:

/* Remove the Domain from sysfs, this should only success no pids in

* tasks

* of a partition.

*/

static

int virResCtrlRemoveDomain(const char *name)

{

char *path = NULL;

int rc = 0;

if ((rc = virAsprintf(&path, "%s/%s", RESCTRL_DIR, name)) < 0)

return rc;

rc = rmdir(path);

VIR_FREE(path);

return rc;

}

Should not write to other directories 'tasks' file.

Other Apps can have a lock to resctrl before it write tasks to prevent libvirtd remove empty task directory.