[libvirt] [v7 00/10] Support cache tune in libvirt

[Marcelo Tosatti]

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.

Can you explain how the resource leak can happen?

Why it is not possible to maintain that information inside libvirt
itself?

> > 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.

Well then libvirt will never be able to change resctrl filesystem?