Peter Xu <pet...@redhat.com> writes:
> On Thu, May 30, 2024 at 07:54:07PM +1000, Nicholas Piggin wrote:
>> Postcopy requires userfaultfd support, which requires tmpfs if a memory
>> file is used.
>>
>> This adds back support for /dev/shm memory files, but adds preallocation
>> to skip environments where that mount is limited in size.
>>
>> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
>
> Thanks for doing this regardless.
>
>> ---
>> tests/qtest/migration-test.c | 77 ++++++++++++++++++++++++++++++++----
>> 1 file changed, 69 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 86eace354e..5078033ded 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -11,6 +11,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>>
>> #include "libqtest.h"
>> #include "qapi/qmp/qdict.h"
>> @@ -553,6 +554,7 @@ typedef struct {
>> */
>> bool hide_stderr;
>> bool use_memfile;
>> + bool use_shm_memfile;
>
> Nitpick: when having both, it's slightly confusing on the name, e.g. not
> clear whether use_memfile needs to be set to true too if use_shm_memfile=true.
>
> Maybe "use_tmpfs_memfile" and "use_shm_memfile"?
>
>> /* only launch the target process */
>> bool only_target;
>> /* Use dirty ring if true; dirty logging otherwise */
>> @@ -739,7 +741,62 @@ static int test_migrate_start(QTestState **from,
>> QTestState **to,
>> ignore_stderr = "";
>> }
>>
>> - if (args->use_memfile) {
>> + if (!qtest_has_machine(machine_alias)) {
>> + g_autofree char *msg = g_strdup_printf("machine %s not supported",
>> + machine_alias);
>> + g_test_skip(msg);
>> + return -1;
>> + }
>> +
>> + if (args->use_shm_memfile) {
>> +#if defined(__NR_userfaultfd) && defined(__linux__)
>
> IIUC we only need defined(__linux__) as there's nothing to do with uffd yet?
>
>> + int fd;
>> + uint64_t size;
>> +
>> + if (getenv("GITLAB_CI")) {
>> + /*
>> + * Gitlab runners are limited to 64MB shm size and despite
>> + * pre-allocation there is concern that concurrent tests
>> + * could result in nondeterministic failures. Until all shm
>> + * usage in all CI tests is found to fail gracefully on
>> + * ENOSPC, it is safer to avoid large allocations for now.
>> + *
>> + * https://lore.kernel.org/qemu-devel/875xuwg4mx....@suse.de/
>> + */
>> + g_test_skip("shm tests are not supported in Gitlab CI
>> environment");
>> + return -1;
>> + }
>
> I'm not sure whether this is Fabiano's intention. I'm wondering whether we
> can drop this and just let it still run there.
>
It was my intention. But I overlooked the fact that the current shm
cannot even run one migration test already.
> Other tests not detecting avaiablility of shmem looks like a separate issue
> to be fixed to me, regardless of this.
>
> My wild guess is since we're doing memory_size+64K below then if test won't
> fail others won't either, as normally the shmem quota should normally be
> power of 2 anyway.. then it should always fit another few MBs if this one.
> While this test is ready to fail gracefully now.
>
I agree. Let's drop this part then.
