Re: [PATCH v5 6/6] tests/qtest/pvpanic: add tests for pvshutdown event

[Thomas Huth]

On 29/01/2024 20.28, Thomas Weißschuh wrote:
> Validate that a shutdown via the pvpanic device emits the correct
> QMP events.
>
> Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
> ---
>   tests/qtest/pvpanic-pci-test.c | 39 +++++++++++++++++++++++++++++++++++++++
>   tests/qtest/pvpanic-test.c     | 29 +++++++++++++++++++++++++++++
>   2 files changed, 68 insertions(+)
>
> diff --git a/tests/qtest/pvpanic-pci-test.c b/tests/qtest/pvpanic-pci-test.c
> index b372caf41dc0..e1c05d383219 100644
> --- a/tests/qtest/pvpanic-pci-test.c
> +++ b/tests/qtest/pvpanic-pci-test.c
> @@ -85,11 +85,50 @@ static void test_panic(void)
>       qtest_quit(qts);
>   }
>   
> +static void test_pvshutdown(void)
> +{
> +    uint8_t val;
> +    QDict *response, *data;
> +    QTestState *qts;
> +    QPCIBus *pcibus;
> +    QPCIDevice *dev;
> +    QPCIBar bar;
> +
> +    qts = qtest_init("-device pvpanic-pci,addr=04.0");
> +    pcibus = qpci_new_pc(qts, NULL);
> +    dev = qpci_device_find(pcibus, QPCI_DEVFN(0x4, 0x0));
> +    qpci_device_enable(dev);
> +    bar = qpci_iomap(dev, 0, NULL);
> +
> +    qpci_memread(dev, bar, 0, &val, sizeof(val));
> +    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
> +
> +    val = 4;

Could you use PVPANIC_SHUTDOWN here instead of the magic value?

> +    qpci_memwrite(dev, bar, 0, &val, sizeof(val));
> +
> +    response = qtest_qmp_eventwait_ref(qts, "GUEST_PVSHUTDOWN");
> +    qobject_unref(response);
> +
> +    response = qtest_qmp_eventwait_ref(qts, "SHUTDOWN");
> +    g_assert(qdict_haskey(response, "data"));
> +    data = qdict_get_qdict(response, "data");
> +    g_assert(qdict_haskey(data, "guest"));
> +    g_assert(qdict_get_bool(data, "guest"));
> +    g_assert(qdict_haskey(data, "reason"));
> +    g_assert_cmpstr(qdict_get_str(data, "reason"), ==, "guest-shutdown");
> +    qobject_unref(response);
> +
> +    g_free(dev);
> +    qpci_free_pc(pcibus);
> +    qtest_quit(qts);
> +}
> +
>   int main(int argc, char **argv)
>   {
>       g_test_init(&argc, &argv, NULL);
>       qtest_add_func("/pvpanic-pci/panic", test_panic);
>       qtest_add_func("/pvpanic-pci/panic-nopause", test_panic_nopause);
> +    qtest_add_func("/pvpanic-pci/pvshutdown", test_pvshutdown);
>   
>       return g_test_run();
>   }
> diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
> index ccc603472f5d..ff1f25f46586 100644
> --- a/tests/qtest/pvpanic-test.c
> +++ b/tests/qtest/pvpanic-test.c
> @@ -58,11 +58,40 @@ static void test_panic(void)
>       qtest_quit(qts);
>   }
>   
> +static void test_pvshutdown(void)
> +{
> +    uint8_t val;
> +    QDict *response, *data;
> +    QTestState *qts;
> +
> +    qts = qtest_init("-device pvpanic");
> +
> +    val = qtest_inb(qts, 0x505);
> +    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
> +
> +    qtest_outb(qts, 0x505, 0x4);

dito, use PVPANIC_SHUTDOWN instead of 4 ?

(as a separate clean-up, we should maybe also introduce a #define for 0x505 
one day, but that's something for another patch)

> +    response = qtest_qmp_eventwait_ref(qts, "GUEST_PVSHUTDOWN");
> +    qobject_unref(response);
> +
> +    response = qtest_qmp_eventwait_ref(qts, "SHUTDOWN");
> +    g_assert(qdict_haskey(response, "data"));
> +    data = qdict_get_qdict(response, "data");
> +    g_assert(qdict_haskey(data, "guest"));
> +    g_assert(qdict_get_bool(data, "guest"));
> +    g_assert(qdict_haskey(data, "reason"));
> +    g_assert_cmpstr(qdict_get_str(data, "reason"), ==, "guest-shutdown");
> +    qobject_unref(response);
> +
> +    qtest_quit(qts);
> +}
> +
>   int main(int argc, char **argv)
>   {
>       g_test_init(&argc, &argv, NULL);
>       qtest_add_func("/pvpanic/panic", test_panic);
>       qtest_add_func("/pvpanic/panic-nopause", test_panic_nopause);
> +    qtest_add_func("/pvpanic/pvshutdown", test_pvshutdown);
>   
>       return g_test_run();
>   }

With PVPANIC_SHUTDOWN instead of 4:
Reviewed-by: Thomas Huth <thuth@redhat.com>