[libvirt] [PATCH] test_driver: implement virDomainGetDiskErrors

[Michal Privoznik]

On 5/9/19 7:35 PM, Ilias Stamatis wrote:
> On Thu, May 9, 2019 at 4:36 PM Michal Privoznik <mprivozn at redhat.com> wrote:
>>
>> On 5/7/19 10:23 PM, Ilias Stamatis wrote:
>>> Return the number of disks present in the configuration of the fake
>>> driver when called with @errors as NULL and @maxerrors as 0.
>>>
>>> Otherwise return 0 as the number of errors encountered.
>>>
>>> Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
>>> ---
>>>    src/test/test_driver.c | 27 +++++++++++++++++++++++++++
>>>    1 file changed, 27 insertions(+)
>>>
>>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>>> index 460c896ef6..5fa9ab30f1 100644
>>> --- a/src/test/test_driver.c
>>> +++ b/src/test/test_driver.c
>>> @@ -3046,6 +3046,32 @@ static int testDomainSetAutostart(virDomainPtr domain,
>>>        return 0;
>>>    }
>>>
>>> +static int testDomainGetDiskErrors(virDomainPtr dom,
>>> +                                   virDomainDiskErrorPtr errors,
>>> +                                   unsigned int maxerrors ATTRIBUTE_UNUSED,
>>> +                                   unsigned int flags)
>>> +{
>>> +    int ret = -1;
>>> +    virDomainObjPtr vm = NULL;
>>> +
>>> +    virCheckFlags(0, -1);
>>> +
>>> +    if (!(vm = testDomObjFromDomain(dom)))
>>> +        goto cleanup;
>>> +
>>> +    if (virDomainObjCheckActive(vm) < 0)
>>> +        goto cleanup;
>>> +
>>> +    if (!errors)
>>> +        ret = vm->def->ndisks;
>>> +    else
>>> +        ret = 0;
>>
>> Don't we want to actually set some errors? That might be more helpful
>> because mgmt app can actually test if it reports disk errors properly.
>>
>> Michal
> 
> Right, that makes sense.
> 
> The number of disks can actually vary because the test connection can
> be configured differently, plus I'll implement virDomainAttachDevice
> for the test driver too.
> 
> The possible error codes are just 2: VIR_DOMAIN_DISK_ERROR_UNSPEC and
> VIR_DOMAIN_DISK_ERROR_NO_SPACE
> 
> So, should it be randomly decided at runtime whether or not to report
> an error for each disk present and which error code will it actually
> be?
> 
> Or would it be better to take a deterministic approach and report
> let's say an error for the first 2 disks (if there are that many at
> all), each with a different error code, and return no errors for all
> the remaining disks?

Yeah, I think the latter makes sense. For simplicity, we can have two 
loops, one inside the other and set the first disk to have no error, the 
second to have VIR_DOMAIN_DISK_ERROR_UNSPEC, the third to have 
VIR_DOMAIN_DISK_ERROR_NO_SPACE, the fourth to have no error again and so 
on. This is perfectly predictible and thus easy to test and it gets a 
result that 'looks' believable.

But whatever you'll come up with will be fine really.

Michal