Re: [PATCH v1 2/8] migration: should_skip() implemented

[Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>]

23.03.2022 13:53, Nikita Lapshin wrote:
> From: Nikita Lapshin <nikita.lapshin@virtuozzo.com>
> 
> For next changes it is convenient to make all decisions about
> sections skipping in one function.
> 
> Signed-off-by: Nikita Lapshin <nikita.lapshin@openvz.org>
> ---
>   migration/savevm.c | 54 ++++++++++++++++++++++++----------------------
>   1 file changed, 28 insertions(+), 26 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 02ed94c180..c68f187ef7 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -943,6 +943,15 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se,
>       return vmstate_save_state(f, se->vmsd, se->opaque, vmdesc);
>   }
>   
> +static bool should_skip(SaveStateEntry *se)
> +{
> +    if (se->ops && se->ops->is_active && !se->ops->is_active(se->opaque)) {
> +        return true;
> +    }
> +
> +    return false;

that may be simply

    return se->ops && se->ops->is_active && !se->ops->is_active(se->opaque);

But may be in future commits the code will be more complicated and we prepare for it now, will see.

> +}
> +
>   /*
>    * Write the header for device section (QEMU_VM_SECTION START/END/PART/FULL)
>    */

[..]

>           trace_savevm_section_start(se->idstr, se->section_id);
>   
> @@ -1417,6 +1417,9 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>               trace_savevm_section_skip(se->idstr, se->section_id);
>               continue;
>           }
> +        if (should_skip(se)) {
> +            continue;
> +        }

Except for this and ...

>   
>           trace_savevm_section_start(se->idstr, se->section_id);
>   
> @@ -1522,10 +1525,8 @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size,
>           if (!se->ops || !se->ops->save_live_pending) {
>               continue;
>           }
> -        if (se->ops->is_active) {
> -            if (!se->ops->is_active(se->opaque)) {
> -                continue;
> -            }
> +        if (should_skip(se)) {
> +            continue;
>           }
>           se->ops->save_live_pending(f, se->opaque, threshold_size,
>                                      res_precopy_only, res_compatible,
> @@ -1635,6 +1636,9 @@ int qemu_save_device_state(QEMUFile *f)
>           if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) {
>               continue;
>           }
> +        if (should_skip(se)) {
> +            continue;
> +        }

this all other chunks are simple refactoring.

Let's do no-logic-change refactoring in a separate commit, and these two changes in another one, with description what and why.

>   
>           save_section_header(f, se, QEMU_VM_SECTION_FULL);
>   
> @@ -2542,10 +2546,8 @@ static int qemu_loadvm_state_setup(QEMUFile *f)
>           if (!se->ops || !se->ops->load_setup) {
>               continue;
>           }
> -        if (se->ops->is_active) {
> -            if (!se->ops->is_active(se->opaque)) {
> -                continue;
> -            }
> +        if (should_skip(se)) {
> +            continue;
>           }
>   
>           ret = se->ops->load_setup(f, se->opaque);


-- 
Best regards,
Vladimir