9:08 a.m.
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
9:08 a.m.
New subject: [PATCH 1/2] virhostdevtest: Initialize hostdev @subsys
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(a)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
9:38 a.m.
New subject: [PATCH 1/2] virhostdevtest: Initialize hostdev @subsys
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(a)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
9:08 a.m.
New subject: [PATCH 2/2] virhostdevtest: Decrease possibility of uninitialized @subsys
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(a)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
9:31 a.m.
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(a)redhat.com>
tests/virhostdevtest.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
--
2.39.1
655
days inactive
655
days old
4 comments
3 participants
participants (3)
-
Ján Tomko -
Martin Kletzander -
Michal Privoznik