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

On 7/10/19 2:09 AM, Peter Krempa wrote: 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