[PATCH 0/2] virhostdevtest: Initialize hostdev @subsys
While the first one qualifies to be pushed as trivial, the second less so. I'll wait a while and if there's no reply I'll just push these, as the build is currently broken. Michal Prívozník (2): virhostdevtest: Initialize hostdev @subsys virhostdevtest: Decrease possibility of uninitialized @subsys tests/virhostdevtest.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) -- 2.39.1
With recent work on storing original PCI stats in _virDomainHostdevSubsysPCI struct, the virhostdevtest can across a latent bug we had. Only some parts of the virDomainHostdevSubsys structure are initialized. Incidentally, subsys->u.pci.origstates is not one of them. This lead to unexpected crashes at runtime. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virhostdevtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index b9e16dd4e8..92bafcbb49 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -123,7 +123,7 @@ myInit(void) size_t i; for (i = 0; i < nhostdevs; i++) { - virDomainHostdevSubsys subsys; + virDomainHostdevSubsys subsys = {0}; hostdevs[i] = virDomainHostdevDefNew(); if (!hostdevs[i]) goto cleanup; -- 2.39.1
On a Monday in 2023, Michal Privoznik wrote:
With recent work on storing original PCI stats in _virDomainHostdevSubsysPCI struct, the virhostdevtest can across
can across? :)
a latent bug we had. Only some parts of the virDomainHostdevSubsys structure are initialized. Incidentally, subsys->u.pci.origstates is not one of them. This lead to unexpected crashes at runtime.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virhostdevtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index b9e16dd4e8..92bafcbb49 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -123,7 +123,7 @@ myInit(void) size_t i;
for (i = 0; i < nhostdevs; i++) { - virDomainHostdevSubsys subsys; + virDomainHostdevSubsys subsys = {0};
To avoid rewriting the commit message, I think you can squash this into the second patch, since the change gets removed anyway. Jano
hostdevs[i] = virDomainHostdevDefNew(); if (!hostdevs[i]) goto cleanup; -- 2.39.1
With the current way the myInit() is written, it's fairly easy to miss initialization of @subsys variable as the variable is allocated firstly on the stack and then it's assigned to hostdev[i] which was allocated using g_new0() (this it is containing nothing but all zeroes). Make the subsys point to the corresponding member in hostdev[i] from the start. This way only the important bits are overwritten and the rest stays initialized to zero. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virhostdevtest.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index 92bafcbb49..1aed0d2b6d 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -123,23 +123,23 @@ myInit(void) size_t i; for (i = 0; i < nhostdevs; i++) { - virDomainHostdevSubsys subsys = {0}; + virDomainHostdevSubsys *subsys; hostdevs[i] = virDomainHostdevDefNew(); if (!hostdevs[i]) goto cleanup; hostdevs[i]->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; - subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; - subsys.u.pci.addr.domain = 0; - subsys.u.pci.addr.bus = 0; - subsys.u.pci.addr.slot = i + 1; - subsys.u.pci.addr.function = 0; - subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; - hostdevs[i]->source.subsys = subsys; + subsys = &hostdevs[i]->source.subsys; + subsys->type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; + subsys->u.pci.addr.domain = 0; + subsys->u.pci.addr.bus = 0; + subsys->u.pci.addr.slot = i + 1; + subsys->u.pci.addr.function = 0; + subsys->u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; } for (i = 0; i < nhostdevs; i++) { - virDomainHostdevSubsys subsys = hostdevs[i]->source.subsys; - if (!(dev[i] = virPCIDeviceNew(&subsys.u.pci.addr))) + virDomainHostdevSubsys *subsys = &hostdevs[i]->source.subsys; + if (!(dev[i] = virPCIDeviceNew(&subsys->u.pci.addr))) goto cleanup; virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO); -- 2.39.1
On Mon, Feb 06, 2023 at 04:08:26PM +0100, Michal Privoznik wrote:
While the first one qualifies to be pushed as trivial, the second less so. I'll wait a while and if there's no reply I'll just push these, as the build is currently broken.
Michal Prívozník (2): virhostdevtest: Initialize hostdev @subsys virhostdevtest: Decrease possibility of uninitialized @subsys
Both Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
tests/virhostdevtest.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
-- 2.39.1
participants (3)
-
Ján Tomko -
Martin Kletzander -
Michal Privoznik