On Mon, Dec 10, 2012 at 09:28:06 +0100, Viktor Mihajlovski wrote:
On 12/10/2012 06:19 AM, Ichikawa, Ken wrote:
> This patchset adds new APIs virDomainGetCPUMode and virDomainSetCPUMode,
> and adds new virsh commands cpu-getmode and cpu-setmode by using these APIs.
>
> virDomainGetCPUMode allows to get cpu mode of a running or persistent domain.
> virDomainSetCPUMode allows to set cpu mode of a persistent domain.
This proposal does not make any sense to me. In general, we don't provide APIs
that duplicate what we can already do by changing domain XML. Since
virDomainSetCPUMode can only affect persistent config (you cannot unplug the
old CPU and plug the new one while the domain is running), the API does not
add anything what could not be done with virDomainDefineXML.
> These APIs and virsh commands are useful because:
> - No longer need to mess around with XML for changing cpu mode.
> - People who want more performance can change cpu mode easily.
There's libvirt-gconfig for those who don't want to mess with XML.
> - If there is a person who has a trouble with host-mode and
> host-passthrough, we can help easily by one-liner like
> "# virsh cpu-setmode <domain> custom"
> Then, default cpu is used after next boot.
However, default CPU is most likely not what anyone would want. It's better to
use one of the predefined models that.
Anyway, if an easy way to change the CPU mode is required, a patch
implementing new virsh command that would fetch domain XML, update it, and
define back could be acceptable (I think we have some of these in virsh
already), but NACK to these new APIs.
BTW, next time, please send individual patches in a series as replies to cover
letter.
Jirka