Re: [PATCH v2 14/29] migration/ram: Introduce 'fixed-ram' migration capability

[Fabiano Rosas]

Markus Armbruster <armbru@redhat.com> writes:

> Fabiano Rosas <farosas@suse.de> writes:
>
>> Add a new migration capability 'fixed-ram'.
>>
>> The core of the feature is to ensure that each ram page has a specific
>> offset in the resulting migration stream. The reason why we'd want
>> such behavior are two fold:
>>
>>  - When doing a 'fixed-ram' migration the resulting file will have a
>>    bounded size, since pages which are dirtied multiple times will
>>    always go to a fixed location in the file, rather than constantly
>>    being added to a sequential stream. This eliminates cases where a vm
>>    with, say, 1G of ram can result in a migration file that's 10s of
>>    GBs, provided that the workload constantly redirties memory.
>>
>>  - It paves the way to implement DIRECT_IO-enabled save/restore of the
>>    migration stream as the pages are ensured to be written at aligned
>>    offsets.
>>
>> For now, enabling the capability has no effect. The next couple of
>> patches implement the core funcionality.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  docs/devel/migration.rst | 14 ++++++++++++++
>>  migration/options.c      | 37 +++++++++++++++++++++++++++++++++++++
>>  migration/options.h      |  1 +
>>  migration/savevm.c       |  1 +
>>  qapi/migration.json      |  5 ++++-
>>  5 files changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
>> index c3e1400c0c..6f898b5dbd 100644
>> --- a/docs/devel/migration.rst
>> +++ b/docs/devel/migration.rst
>> @@ -566,6 +566,20 @@ Others (especially either older devices or system 
>> devices which for
>>  some reason don't have a bus concept) make use of the ``instance id``
>>  for otherwise identically named devices.
>>  
>> +Fixed-ram format
>> +----------------
>> +
>> +When the ``fixed-ram`` capability is enabled, a slightly different
>> +stream format is used for the RAM section. Instead of having a
>> +sequential stream of pages that follow the RAMBlock headers, the dirty
>> +pages for a RAMBlock follow its header. This ensures that each RAM
>> +page has a fixed offset in the resulting migration stream.
>
> This requires the migration stream to be seekable, as documented in the
> QAPI schema below.  I think it's worth documenting here, as well.
>

Ok.

>> +
>> +The ``fixed-ram`` capaility can be enabled in both source and
>> +destination with:
>> +
>> +    ``migrate_set_capability fixed-ram on``
>
> Effect of enabling on the destination?
>
> What happens when we enable it only on one end?
>

qemu-system-x86_64: Capability fixed-ram is off, but received capability is on
qemu-system-x86_64: load of migration failed: Invalid argument

So I guess that *can* be enabled up there should become a *must*.

>> +
>>  Return path
>>  -----------
>>  
>
> [...]
>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 74f12adc0e..1317dd32ab 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -527,6 +527,9 @@
>>  #     VM before migration for an optimal migration performance.
>>  #     Enabled by default. (since 8.1)
>>  #
>> +# @fixed-ram: Migrate using fixed offsets for each RAM page. Requires
>
> Two spaces between sentences for consistency, please.
>
>> +#             a seekable transport such as a file.  (since 8.1)
>
> What is a migration transport?  migration.json doesn't define the term.
>

The medium that transports the migration. We are about to define some
terms at the QAPI series:

[PATCH v15 00/14] migration: Modify 'migrate' and 'migrate-incoming'
QAPI commands for migration
20231023182053.8711-1-farosas@suse.de">https://lore.kernel.org/r/20231023182053.8711-1-farosas@suse.de

> Which transports are seekable?
>

The ones that implement QIO_CHANNEL_FEATURE_SEEKABLE. Currently only
QIOChannelFile.

> Out of curiosity: what happens if the transport isn't seekable?
>

We fail the migration. At migration_channels_and_uri_compatible():

    if (migration_needs_seekable_channel() &&
        !uri_supports_seeking(uri)) {
        error_setg(errp, "Migration requires seekable transport (e.g. file)");
        compatible = false;
    }

>> +#
>>  # Features:
>>  #
>>  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>> @@ -543,7 +546,7 @@
>>             { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>>             'validate-uuid', 'background-snapshot',
>>             'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
>> -           'dirty-limit', 'auto-pause'] }
>> +           'dirty-limit', 'auto-pause', 'fixed-ram'] }
>>  
>>  ##
>>  # @MigrationCapabilityStatus: