Re: [PATCH v5 1/4] migration: introduce 'background-snapshot' migration capability

From: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
To: Peter Xu <peterx@redhat.com>
Cc: qemu-devel@nongnu.org, Den Lunev <den@openvz.org>,
	Eric Blake <eblake@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH v5 1/4] migration: introduce 'background-snapshot' migration capability
Date: Wed, 9 Dec 2020 10:08:03 +0300	[thread overview]
Message-ID: <70b3aacf-61b8-ad82-8e5d-8b104135727d@virtuozzo.com> (raw)
In-Reply-To: <20201208154735.GA21402@xz-x1>

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On 08.12.2020 18:47, Peter Xu wrote:
> On Fri, Dec 04, 2020 at 12:31:00PM +0300, Andrey Gruzdev wrote:
>> +static
>> +WriteTrackingSupport migrate_query_write_tracking(void)
>> +{
>> +    static WriteTrackingSupport wt_support = WT_SUPPORT_UNKNOWN;
> Better to be non-static - consider uncompatible memory can be hot plugged after
> it's once set.
>
> With static removed (I think most of below if clauses can be unwrapped too, but
> your call), feel free to add:
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> Thanks,

Yes, quite possible case with memory hot plug that you mentioned..
Thanks, I'll change it to non-static.

>> +
>> +    /* Check UFFD-IO and memory backends for compatibility only once */
>> +    if (wt_support == WT_SUPPORT_UNKNOWN) {
>> +        wt_support = WT_SUPPORT_ABSENT;
>> +        /* Check if kernel supports required UFFD features */
>> +        if (ram_write_tracking_available()) {
>> +            wt_support = WT_SUPPORT_AVAILABLE;
>> +            /*
>> +             * Check if current memory configuration is
>> +             * compatible with required UFFD features.
>> +             */
>> +            if (ram_write_tracking_compatible()) {
>> +                wt_support = WT_SUPPORT_COMPATIBLE;
>> +            }
>> +        }
>> +    }
>> +
>> +    return wt_support;
>> +}
>> +#endif /* CONFIG_LINUX */

[-- Attachment #2: Type: text/html, Size: 2106 bytes --]