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

Re: [libvirt] [PATCH 5/5] setmem: add the new options to "virsh setmem" command

On 03/10/2011 12:44 AM, Paolo Bonzini wrote:
> On 03/09/2011 04:59 PM, Daniel P. Berrange wrote:
>>> >  +    if (!flags) {
>>> >  +        if (virDomainSetMemory(dom, kilobytes) != 0) {
>>> >  +            ret = FALSE;
>>> >  +        }
>> Indentation went a little bit wrong here. Also  '!flags' could
>> in fact be '!flags || (flags == VIR_DOMAIN_MEM_LIVE)'  since if
>> only the --live flag was set, we can still use the original API
>> call for greater backcompatibility.
> Perhaps this choice of APIs should be done in libvirt rather than virsh
> (with virsh always calling the new API).  This way all clients can talk
> to an older remote libvirt without having to care about
> virDomainSetMemory vs. virDomainSetMemoryFlags.

Perhaps so, but that's a bigger task better left for a separate patch
(if we do that in libvirt.c for one API, we should do it for all of them
that fit the same pattern of a new function that subsumes functionality
of an older one - I can think of several).

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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