Re: [PATCH v3 2/4] qtest/libqos/pci: Enforce balanced iomap/unmap

["Nicholas Piggin" <npiggin@gmail.com>]

On Mon Jan 20, 2025 at 3:29 PM AEST, Philippe Mathieu-Daudé wrote:
> Hi Nick,
>
> Only nitpicking comments...

Hey, no they're good comments actually.

>
> On 17/1/25 18:22, Nicholas Piggin wrote:
>> Add assertions to ensure a BAR is not mapped twice, and only
>> previously mapped BARs are unmapped. This can help catch some
>> bugs.
>> 
>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   tests/qtest/libqos/ahci.h       |  1 +
>>   tests/qtest/libqos/pci.h        |  2 ++
>>   tests/qtest/libqos/virtio-pci.h |  1 +
>>   tests/qtest/ahci-test.c         |  2 ++
>>   tests/qtest/libqos/ahci.c       |  6 ++++++
>>   tests/qtest/libqos/pci.c        | 32 +++++++++++++++++++++++++++++++-
>>   tests/qtest/libqos/virtio-pci.c |  6 +++++-
>>   7 files changed, 48 insertions(+), 2 deletions(-)
>
> Maybe put the AHCI fix in a preliminary patch?

Yeah, this was just laziness. I will fix.

>
>> diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
>> index 83896145235..9dc82ea723a 100644
>> --- a/tests/qtest/libqos/pci.h
>> +++ b/tests/qtest/libqos/pci.h
>
> Consider using a definition rather than a magic value:
>
>    #define PCI_BAR_COUNT 6

Now I look again at PCI code and it has PCI_NUM_REGIONS 7
where ROM slot is the last entry. qtests doesn't use it
AFAIKS but maybe it could(?) so should I just use that
existing define?

Thanks,
Nick