[libvirt] [PATCH v2 1/9] vbox: Add various vir*Flags API

[Eric Blake]

On 7/10/19 2:09 AM, Peter Krempa wrote:
> On Tue, Jul 09, 2019 at 12:46:30 -0500, Eric Blake wrote:
>> Even though we don't accept any flags, it is unfriendly to callers
>> that use the modern API to have to fall back to the flag-free API.
>>
>> Signed-off-by: Eric Blake <eblake at redhat.com>
>> ---
>>  src/vbox/vbox_common.c | 24 ++++++++++++++++++++++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> [...]
> 
>> @@ -2775,6 +2788,11 @@ static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory)
>>      return ret;
>>  }
>>
>> +static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory)
>> +{
>> +    return vboxDomainSetMemoryFlags(dom, memory, 0);
> 
> The old API was operating on a live VM only so this shim should imply
> VIR_DOMAIN_AFFECT_LIVE.

Hmm. Commit 667ac11e used flag 0 for the test driver. But if I want to
reduce the number of driver callbacks by instead teaching the remote
driver when to call the legacy RPC, it's easier if ALL drivers shim-call
SetMemoryFlags(VIR_DOMAIN_AFFECT_LIVE) rather than some calling with 0
and others calling with AFFECT_LIVE.

The current list:
v5.5.0:src/hyperv/hyperv_driver.c:    return
hypervDomainSetMemoryFlags(domain, memory, 0);
v5.5.0:src/libxl/libxl_driver.c:    return
libxlDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_LIVE);
v5.5.0:src/libxl/libxl_driver.c:    return
libxlDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM);
v5.5.0:src/lxc/lxc_driver.c:    return lxcDomainSetMemoryFlags(dom,
newmem, VIR_DOMAIN_AFFECT_LIVE);
v5.5.0:src/lxc/lxc_driver.c:    return lxcDomainSetMemoryFlags(dom,
newmax, VIR_DOMAIN_MEM_MAXIMUM);
v5.5.0:src/qemu/qemu_driver.c:    return qemuDomainSetMemoryFlags(dom,
newmem, VIR_DOMAIN_AFFECT_LIVE);
v5.5.0:src/qemu/qemu_driver.c:    return qemuDomainSetMemoryFlags(dom,
memory, VIR_DOMAIN_MEM_MAXIMUM);

my series adds:
test
vbox
xenapi

So it looks like hyperv has a bug in being inconsistent from everyone
else, if I follow your advice for the 3 new shims, although it currently
lacks a SetMaxMemory API at all.

You also appear to be right that making SetMaxMemory a shim everywhere
for SetMemoryFlags(VIR_DOMAIN_MEM_MAXMIUM) is a sensible idea, at least
for drivers that have a notion of setting max memory, but since not all
drivers had SetMaxMemory, it's harder to argue that the presence of the
new interface must require the legacy pair (especially if the new
interface blindly requires a specific flags pattern).  I'll have to look
more closely at what each driver is doing.

Our public docs currently state:

 * and will fail for transient domains. If neither flag is specified
 * (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain
 * modifies persistent setup, while an active domain is hypervisor-dependent
 * on whether just live or both live and persistent state is changed.

which means that calling SetMemory() may be a synonym to either
SetMemoryFlags(_CURRENT==0) or to SetMemoryFlags(_LIVE).  But by
tweaking hyperv, we could change it so that the spec is more
tightly-worded as always being equivalent to SetMemoryFlags(_LIVE).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190710/a1a54d9f/attachment-0001.sig>