[libvirt] [PATCH 0/2] virQEMUCapsFillDomainCaps: Drop redundant parameter
Jiri Denemark (2): qemu: Remove redundant parameter in virQEMUCapsFillDomainCaps domaincapstest: Don't read data from host src/qemu/qemu_capabilities.c | 5 ++--- src/qemu/qemu_capabilities.h | 3 +-- src/qemu/qemu_driver.c | 2 +- tests/Makefile.am | 7 +++++++ tests/domaincapsmock.c | 26 ++++++++++++++++++++++++++ tests/domaincapstest.c | 5 ++--- 6 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 tests/domaincapsmock.c -- 2.9.0
virttype is already included in domCaps, no need to pass it separately. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 5 ++--- src/qemu/qemu_capabilities.h | 3 +-- src/qemu/qemu_driver.c | 2 +- tests/domaincapstest.c | 3 +-- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 28d5321..2c0b29d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4337,8 +4337,7 @@ int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps, virFirmwarePtr *firmwares, - size_t nfirmwares, - virDomainVirtType virttype) + size_t nfirmwares) { virDomainCapsOSPtr os = &domCaps->os; virDomainCapsDeviceDiskPtr disk = &domCaps->disk; @@ -4348,7 +4347,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine); - if (virttype == VIR_DOMAIN_VIRT_KVM) { + if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM) { int hostmaxvcpus = virHostCPUGetKVMMaxVCPUs(); if (hostmaxvcpus >= 0) domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9d891c8..affb639 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -492,7 +492,6 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps, virFirmwarePtr *firmwares, - size_t nfirmwares, - virDomainVirtType virttype); + size_t nfirmwares); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 61d184b..749f8a1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18411,7 +18411,7 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, goto cleanup; if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, - cfg->firmwares, cfg->nfirmwares, virttype) < 0) + cfg->firmwares, cfg->nfirmwares) < 0) goto cleanup; ret = virDomainCapsFormat(domCaps); diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 01ebfcc..ae31146 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -129,8 +129,7 @@ fillQemuCaps(virDomainCapsPtr domCaps, if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, cfg->firmwares, - cfg->nfirmwares, - VIR_DOMAIN_VIRT_QEMU) < 0) + cfg->nfirmwares) < 0) goto cleanup; /* The function above tries to query host's KVM & VFIO capabilities by -- 2.9.0
On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
virttype is already included in domCaps, no need to pass it separately.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 5 ++--- src/qemu/qemu_capabilities.h | 3 +-- src/qemu/qemu_driver.c | 2 +- tests/domaincapstest.c | 3 +-- 4 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 28d5321..2c0b29d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4337,8 +4337,7 @@ int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps, virFirmwarePtr *firmwares, - size_t nfirmwares, - virDomainVirtType virttype) + size_t nfirmwares) { virDomainCapsOSPtr os = &domCaps->os; virDomainCapsDeviceDiskPtr disk = &domCaps->disk; @@ -4348,7 +4347,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine); - if (virttype == VIR_DOMAIN_VIRT_KVM) { + if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM) { int hostmaxvcpus = virHostCPUGetKVMMaxVCPUs(); if (hostmaxvcpus >= 0) domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9d891c8..affb639 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -492,7 +492,6 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps, virFirmwarePtr *firmwares, - size_t nfirmwares, - virDomainVirtType virttype); + size_t nfirmwares);
#endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 61d184b..749f8a1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18411,7 +18411,7 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, goto cleanup;
if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, - cfg->firmwares, cfg->nfirmwares, virttype) < 0) + cfg->firmwares, cfg->nfirmwares) < 0) goto cleanup;
ret = virDomainCapsFormat(domCaps); diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 01ebfcc..ae31146 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -129,8 +129,7 @@ fillQemuCaps(virDomainCapsPtr domCaps,
if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, cfg->firmwares, - cfg->nfirmwares, - VIR_DOMAIN_VIRT_QEMU) < 0) + cfg->nfirmwares) < 0) goto cleanup;
/* The function above tries to query host's KVM & VFIO capabilities by
This should be patch 2/2 in the series. ACK with that changed. -- Andrea Bolognani / Red Hat / Virtualization
virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM domains. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/Makefile.am | 7 +++++++ tests/domaincapsmock.c | 26 ++++++++++++++++++++++++++ tests/domaincapstest.c | 2 +- 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/domaincapsmock.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 444e0fd..1639540 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -404,6 +404,7 @@ test_libraries = libshunload.la \ virrandommock.la \ virhostcpumock.la \ nssmock.la \ + domaincapsmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -919,6 +920,12 @@ vircaps2xmltest_SOURCES = \ vircaps2xmltest.c testutils.h testutils.c vircaps2xmltest_LDADD = $(LDADDS) + +domaincapsmock_la_SOURCES = domaincapsmock.c +domaincapsmock_la_CFLAGS = $(AM_CFLAGS) +domaincapsmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +domaincapsmock_la_LIBADD = $(MOCKLIBS_LIBS) + domaincapstest_SOURCES = \ domaincapstest.c testutils.h testutils.c domaincapstest_LDADD = $(LDADDS) diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c new file mode 100644 index 0000000..5266b73 --- /dev/null +++ b/tests/domaincapsmock.c @@ -0,0 +1,26 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "virhostcpu.h" + +int +virHostCPUGetKVMMaxVCPUs(void) +{ + return -1; +} diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index ae31146..5b7b7d0 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -385,4 +385,4 @@ mymain(void) return ret; } -VIRT_TEST_MAIN(mymain) +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/domaincapsmock.so") -- 2.9.0
On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM domains. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/Makefile.am | 7 +++++++ tests/domaincapsmock.c | 26 ++++++++++++++++++++++++++ tests/domaincapstest.c | 2 +- 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/domaincapsmock.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 444e0fd..1639540 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -404,6 +404,7 @@ test_libraries = libshunload.la \ virrandommock.la \ virhostcpumock.la \ nssmock.la \ + domaincapsmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -919,6 +920,12 @@ vircaps2xmltest_SOURCES = \ vircaps2xmltest.c testutils.h testutils.c vircaps2xmltest_LDADD = $(LDADDS) + +domaincapsmock_la_SOURCES = domaincapsmock.c +domaincapsmock_la_CFLAGS = $(AM_CFLAGS) +domaincapsmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +domaincapsmock_la_LIBADD = $(MOCKLIBS_LIBS) + domaincapstest_SOURCES = \ domaincapstest.c testutils.h testutils.c domaincapstest_LDADD = $(LDADDS) diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c new file mode 100644 index 0000000..5266b73 --- /dev/null +++ b/tests/domaincapsmock.c @@ -0,0 +1,26 @@ +/*
Missing copyright statement.
+ * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + *
Unneeded empty line.
+ */ + +#include <config.h> + +#include "virhostcpu.h" + +int +virHostCPUGetKVMMaxVCPUs(void) +{ + return -1;
As discussed offline, returning -1 signals the caller that an error has occurred. We should return a positive number instead, either something like 160 (so we can check that limiting vCPUs based on MAX_VCPUS works) or INT_MAX (so we know it'll never be smaller than the QEMU limit).
+} diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index ae31146..5b7b7d0 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -385,4 +385,4 @@ mymain(void) return ret; } -VIRT_TEST_MAIN(mymain) +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/domaincapsmock.so")
ACK with that fixed. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jun 27, 2016 at 16:41:08 +0200, Andrea Bolognani wrote:
On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM domains. diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c new file mode 100644 index 0000000..5266b73 --- /dev/null +++ b/tests/domaincapsmock.c @@ -0,0 +1,26 @@ +/*
Missing copyright statement.
That's deliberate. ...
We should return a positive number instead, either something like 160 (so we can check that limiting vCPUs based on MAX_VCPUS works) or INT_MAX (so we know it'll never be smaller than the QEMU limit).
INT_MAX is better. In the future we can expand this to be configurable per test case so that we can check more combinations. Jirka
On Mon, 2016-06-27 at 16:53 +0200, Jiri Denemark wrote:
On Mon, Jun 27, 2016 at 16:41:08 +0200, Andrea Bolognani wrote:
On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM domains. diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c new file mode 100644 index 0000000..5266b73 --- /dev/null +++ b/tests/domaincapsmock.c @@ -0,0 +1,26 @@ +/* Missing copyright statement. That's deliberate.
The "How to Apply These Terms to Your New Libraries" section of the LGPL explicitly states
[...] each file should have at least the "copyright" line and a pointer to where the full notice is found.
so I don't think you can just add the license text without a corresponding copyright statement. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jun 27, 2016 at 15:50:44 +0200, Jiri Denemark wrote:
virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM domains. diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c new file mode 100644 index 0000000..5266b73 --- /dev/null +++ b/tests/domaincapsmock.c @@ -0,0 +1,26 @@ +#include <config.h> + +#include "virhostcpu.h" + +int +virHostCPUGetKVMMaxVCPUs(void) +{ + return -1; +}
Actually, this should return INT_MAX rather than an error. Jirka
participants (2)
-
Andrea Bolognani -
Jiri Denemark