[libvirt] [PATCH 0/3] Fix the CI builds after commit 6ac402c456a

This series fixes the following failed CI builds: https://travis-ci.org/libvirt/libvirt/builds/563146026 https://ci.centos.org/view/libvirt/job/libvirt-check/537/ Successful build after these patches: https://travis-ci.org/eskultety/libvirt/builds/563439369 Erik Skultety (3): util: cgroup: Add missing parameter @maxthreads to virCgroupNewMachine test_driver: Fix testDomainSetMemoryFlags' behaviour on config change tests: optparse: Use --config with the setmaxmem command src/test/test_driver.c | 5 +++-- src/util/vircgroup.c | 1 + tests/virsh-optparse | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) -- 2.21.0

Commit @d5572f62e32 forgot to add maxthreads to the non-Linux definition of the function, thus breaking the MinGW build. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/vircgroup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 9daf62795e..2270a520d6 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2891,6 +2891,7 @@ virCgroupNewMachine(const char *name ATTRIBUTE_UNUSED, int *nicindexes ATTRIBUTE_UNUSED, const char *partition ATTRIBUTE_UNUSED, int controllers ATTRIBUTE_UNUSED, + unsigned int maxthreads ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", -- 2.21.0

On Thu, Jul 25, 2019 at 10:19:38AM +0200, Erik Skultety wrote:
Commit @d5572f62e32 forgot to add maxthreads to the non-Linux definition
Please drop the '@', it is not included in the default list of word separators of my terminal emulator.
of the function, thus breaking the MinGW build.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/vircgroup.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

When commit @6ac402c456a added the API whenever VIR_DOMAIN_MEM_MAXIMUM was passed the code always checked whether the domain was active and therefore failed with an error even though only a config change was requested. Fix the issue by replacing virDomainObjGetOneDef with virDomainObjGetOneDefState which tells us what definition we're performing the change on. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/test/test_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 56f6b78ecc..da044027bf 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2524,6 +2524,7 @@ static int testDomainSetMemoryFlags(virDomainPtr domain, virDomainObjPtr vm; virDomainDefPtr def; int ret = -1; + bool live = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -2532,11 +2533,11 @@ static int testDomainSetMemoryFlags(virDomainPtr domain, if (!(vm = testDomObjFromDomain(domain))) return -1; - if (!(def = virDomainObjGetOneDef(vm, flags))) + if (!(def = virDomainObjGetOneDefState(vm, flags, &live))) goto cleanup; if (flags & VIR_DOMAIN_MEM_MAXIMUM) { - if (virDomainObjIsActive(vm)) { + if (live) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot resize the maximum memory on an " "active domain")); -- 2.21.0

On Thu, Jul 25, 2019 at 10:19:39AM +0200, Erik Skultety wrote:
When commit @6ac402c456a added the API whenever VIR_DOMAIN_MEM_MAXIMUM
Same here.
was passed the code always checked whether the domain was active and therefore failed with an error even though only a config change was requested. Fix the issue by replacing virDomainObjGetOneDef with virDomainObjGetOneDefState which tells us what definition we're performing the change on.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/test/test_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

The virsh-optparse test broke after commit @6ac402c456a because it always assumed the max memory limit can be adjusted on a running domain which used to be the case in the old code. This is only a hot fix for the CI build. The proper fix here is to re-write the whole test in a self-test/unit-test manner where we only test virsh's ability to parse various values, not running actual commands. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tests/virsh-optparse | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/virsh-optparse b/tests/virsh-optparse index 1aef8d8d95..090d6c205c 100755 --- a/tests/virsh-optparse +++ b/tests/virsh-optparse @@ -219,13 +219,13 @@ test -s out && fail=1 compare exp-err err || fail=1 # Numeric value with valid suffix -virsh -q -c $test_url setmaxmem test 42MB >out 2>err || fail=1 +virsh -q -c $test_url setmaxmem test 42MB --config >out 2>err || fail=1 test -s out && fail=1 test -s err && fail=1 # Numeric value bigger than INT_MAX. No failure here because # scaled numeric values are unsigned long long -virsh -q -c $test_url setmaxmem test 2147483648 >out 2>err || fail=1 +virsh -q -c $test_url setmaxmem test 2147483648 --config >out 2>err || fail=1 test -s out && fail=1 test -s err && fail=1 @@ -248,7 +248,7 @@ test -s out && fail=1 compare exp-err err || fail=1 # Numeric value -virsh -q -c $test_url setmaxmem test 42 >out 2>err || fail=1 +virsh -q -c $test_url setmaxmem test 42 --config >out 2>err || fail=1 test -s out && fail=1 test -s err && fail=1 -- 2.21.0

On Thu, Jul 25, 2019 at 10:19:40AM +0200, Erik Skultety wrote:
The virsh-optparse test broke after commit @6ac402c456a because it
And here.
always assumed the max memory limit can be adjusted on a running domain which used to be the case in the old code. This is only a hot fix for the CI build. The proper fix here is to re-write the whole test in a self-test/unit-test manner where we only test virsh's ability to parse various values, not running actual commands.
Yes, this test is strange.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tests/virsh-optparse | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, Jul 25, 2019 at 10:38:42AM +0200, Ján Tomko wrote:
On Thu, Jul 25, 2019 at 10:19:40AM +0200, Erik Skultety wrote:
The virsh-optparse test broke after commit @6ac402c456a because it
And here.
always assumed the max memory limit can be adjusted on a running domain which used to be the case in the old code. This is only a hot fix for the CI build. The proper fix here is to re-write the whole test in a self-test/unit-test manner where we only test virsh's ability to parse various values, not running actual commands.
Yes, this test is strange.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tests/virsh-optparse | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Fixed and pushed all of them. Thanks, Erik

On Thu, Jul 25, 2019 at 10:19:37 +0200, Erik Skultety wrote:
This series fixes the following failed CI builds: https://travis-ci.org/libvirt/libvirt/builds/563146026 https://ci.centos.org/view/libvirt/job/libvirt-check/537/
Successful build after these patches: https://travis-ci.org/eskultety/libvirt/builds/563439369
Erik Skultety (3): util: cgroup: Add missing parameter @maxthreads to virCgroupNewMachine test_driver: Fix testDomainSetMemoryFlags' behaviour on config change tests: optparse: Use --config with the setmaxmem command
ACK series
participants (3)
-
Erik Skultety
-
Ján Tomko
-
Peter Krempa