[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] RFC: virsh setmem: configure inactive domains' memory size



On Thu, Feb 24, 2011 at 02:43:03PM -0700, Eric Blake wrote:
> On 02/24/2011 03:25 AM, Taku Izumi wrote:
> > 
> > Currently "virsh setmem" is not allowed to use against inactive domains.
> > By applying this patch, we can change the memory size of inactive domains
> > by "virsh setmem".
> > 
> > 
> > -    if (virDomainSetMemory(dom, kilobytes) != 0) {
> > -        ret = FALSE;
> > +    if (virDomainIsActive(dom) == 1) {
> > +        if (virDomainSetMemory(dom, kilobytes) == 0)
> 
> This is slightly racy - in between the time that you probe if the domain
> is active and actually request to set memory, the domain may have gone
> inactive (or vice versa, in between the time you see that it is inactive
> and start modifying the xml, it may have become active).
> 
> I think the better fix here would be to add a new API,
> virDomainSetMemoryFlags, similar to virDomainUpdateDeviceFlags, where
> the flags can request whether you want to affect running, persistent, or
> both configurations all from a single API.  Falling back to XML editing
> when the new API is not present is reasonable, but not an excuse for not
> providing the right interface to begin with.
> 
> So, I'm debating whether to apply this now or whether to add the better
> API first; anyone else want to chime in?

If we want this kind of functionality in virsh, then IMHO we should
have an API for it. The resulting code will be much simpler than
what this does in virsh.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]