[libvirt] [PATCH] test_driver: fix some bugs on testDomainGetDiskErrors

Ilias Stamatis stamatis.iliass at gmail.com
Tue Jul 2 08:27:50 UTC 2019


On Tue, Jul 2, 2019 at 10:04 AM Erik Skultety <eskultet at redhat.com> wrote:
>
> On Thu, Jun 20, 2019 at 05:11:57PM +0200, Ilias Stamatis wrote:
> > The current implementation has the following bugs:
> >
> > - the vm variable is accessed after calling virDomainObjEndAPI on it
> >
> > - if VIR_STRDUP fails and we jump to the cleanup section, we're calling
> > VIR_FREE on pointers for which we haven't allocated memory
> >
> > - the error type VIR_DOMAIN_DISK_ERROR_NONE is used which contradicts
> > the documentation of the API that says that disks with no errors are not
> > reported
> >
> > This patch fixes all of them and additionally reports errors only for
> > every second disk (instead of reporting errors for all disks) which was
> > the initial intention.
>
> I believe each patch should focus ideally on 1 thing only. So this patch should
> IMO be split into 3 (doesn't matter in what order), each fixing a different bug.
>
> Erik

Hmm. I think it's impossible to solve the first 2 separately. Because
by solving the 2nd one the 1st is solved as well. The 3rd one is
really adding a "-1" and a "+1" in the VIR_DOMAIN_DISK_ERROR_LAST
line. But this line is touched anyway by the previous fix as well. So
I thought it doesn't worth it to introduce a separate patch for it.

I think the patch in general is quite small so it shouldn't bet a huge
problem, but if you insist I can separate the 1-line change for fix
no3. from the ones before.

Ilias




More information about the libvir-list mailing list