In my commit of v7.1.0-rc1~376 I've simplified the logic of handling @flags. My assumption back then was that calling virDomainSetMemory() is equivalent to virDomainSetMemoryFlags(flags = 0). But that is not the case, because it is equivalent to virDomainSetMemoryFlags(flags = VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old API. Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961118 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

On a Tuesday in 2021, Michal Privoznik wrote: > In my commit of v7.1.0-rc1~376 I've simplified the logic of > handling @flags. My assumption back then was that calling > virDomainSetMemory() is equivalent to > virDomainSetMemoryFlags(flags = 0). But that is not the case, > because it is equivalent to virDomainSetMemoryFlags(flags = > VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old > API. > > Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a before that commit, if the user did not specify any of:   config, live, current we used the old API. After this change, the new API will be used for those cases.

On Tue, May 18, 2021 at 03:02:55PM +0200, Ján Tomko wrote: > On a Tuesday in 2021, Michal Privoznik wrote: >> In my commit of v7.1.0-rc1~376 I've simplified the logic of >> handling @flags. My assumption back then was that calling >> virDomainSetMemory() is equivalent to >> virDomainSetMemoryFlags(flags = 0). But that is not the case, >> because it is equivalent to virDomainSetMemoryFlags(flags = >> VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old >> API. >> >> Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a > > before that commit, if the user did not specify any of: > config, live, current > we used the old API. > > After this change, the new API will be used for those cases. The question is if we support using upstream virsh with old libvirt where only non-flags API is available. If not we should drop the non-flags API usage from virsh completely.

On Tue, May 18, 2021 at 03:17:56PM +0200, Michal Prívozník wrote: > On 5/18/21 3:12 PM, Pavel Hrdina wrote: > > On Tue, May 18, 2021 at 03:02:55PM +0200, Ján Tomko wrote: > >> On a Tuesday in 2021, Michal Privoznik wrote: > >>> In my commit of v7.1.0-rc1~376 I've simplified the logic of > >>> handling @flags. My assumption back then was that calling > >>> virDomainSetMemory() is equivalent to > >>> virDomainSetMemoryFlags(flags = 0). But that is not the case, > >>> because it is equivalent to virDomainSetMemoryFlags(flags = > >>> VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old > >>> API. > >>> > >>> Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a > >> > >> before that commit, if the user did not specify any of: > >> config, live, current > >> we used the old API. > >> > >> After this change, the new API will be used for those cases. > > > > The question is if we support using upstream virsh with old libvirt > > where only non-flags API is available. If not we should drop the > > non-flags API usage from virsh completely. > > I think in general we do. In this specific case, > virDomainSetMemoryFlags() API was introduced in v0.9.0 release (which is > 10 years ago). And since we dropped RHEL-7 support recently and the > oldest QEMU we support is from late 2017 our attempt to be that > backwards compatible is questionable IMO. Exactly my question. We have some support matrix where libvirt should be able to compile and run but there is no explicit support policy for using virsh with old libvirt or for obsolete APIs that should not be used. I would vote to use the same matrix as we already have. That would allow us to remove probably all non-flags APIs from virsh code.