[libvirt] [PATCH 0/2] Couple of test_driver fixes
I was too eager to minimize patch 89320788ac4 and did some mistakes. Here are the fixes. Michal Prívozník (2): test_driver: Don't access @vm after it was set to NULL test_driver: Don't report VIR_DOMAIN_DISK_ERROR_NONE src/test/test_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.21.0
If something goes wrong in testDomainGetDiskErrors() then we try to free any strings that were previously allocated in return array. Problem is, in my review of original patch (89320788ac4) I've mistakenly did some changes which result in possible NULL dereference (@vm is set to NULL as the first thing under cleanup label). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/test/test_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a4c17ef0df..cfa739ecca 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3058,7 +3058,7 @@ static int testDomainGetDiskErrors(virDomainPtr dom, virCheckFlags(0, -1); if (!(vm = testDomObjFromDomain(dom))) - goto cleanup; + return -1; if (virDomainObjCheckActive(vm) < 0) goto cleanup; @@ -3075,11 +3075,11 @@ static int testDomainGetDiskErrors(virDomainPtr dom, } cleanup: - virDomainObjEndAPI(&vm); if (ret < 0) { for (i = 0; i < MIN(vm->def->ndisks, maxerrors); i++) VIR_FREE(errors[i].disk); } + virDomainObjEndAPI(&vm); return ret; } -- 2.21.0
On Tue, 2019-05-14 at 17:12 +0200, Michal Privoznik wrote:
If something goes wrong in testDomainGetDiskErrors() then we try to free any strings that were previously allocated in return array. Problem is, in my review of original patch (89320788ac4) I've mistakenly did some changes which result in possible NULL dereference (@vm is set to NULL as the first thing under cleanup label).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/test/test_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
In my review of 89320788ac4 I've simplified assigning disk errors too much as the code I've changed it to will set VIR_DOMAIN_DISK_ERROR_NONE. This is in contradiction with our documentation which specifies that disks with no errors are not reported. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/test/test_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index cfa739ecca..ad89663de2 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3067,7 +3067,7 @@ static int testDomainGetDiskErrors(virDomainPtr dom, for (i = 0; i < MIN(vm->def->ndisks, maxerrors); i++) { if (VIR_STRDUP(errors[i].disk, vm->def->disks[i]->dst) < 0) goto cleanup; - errors[i].error = i % VIR_DOMAIN_DISK_ERROR_LAST; + errors[i].error = (i % (VIR_DOMAIN_DISK_ERROR_LAST - 1)) + 1; } ret = i; } else { -- 2.21.0
On Tue, 2019-05-14 at 17:12 +0200, Michal Privoznik wrote:
In my review of 89320788ac4 I've simplified assigning disk errors too much as the code I've changed it to will set VIR_DOMAIN_DISK_ERROR_NONE. This is in contradiction with our documentation which specifies that disks with no errors are not reported.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/test/test_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On 5/14/19 5:12 PM, Michal Privoznik wrote:
I was too eager to minimize patch 89320788ac4 and did some mistakes. Here are the fixes.
Michal Prívozník (2): test_driver: Don't access @vm after it was set to NULL test_driver: Don't report VIR_DOMAIN_DISK_ERROR_NONE
src/test/test_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Darn. I need to learn how to rm 00* before 'git format-patches' .... Obviously, only test_driver patches were ment to be sent as the others were ACKed and merged already. Michal
participants (2)
-
Andrea Bolognani -
Michal Privoznik