Re: [PATCH v5 08/13] migration/multifd: Add new migration option for multifd DSA offloading.

[Fabiano Rosas]

Fabiano Rosas <farosas@suse.de> writes:

> Yichen Wang <yichen.wang@bytedance.com> writes:
>
>> On Thu, Jul 11, 2024 at 2:53 PM Yichen Wang <yichen.wang@bytedance.com> 
>> wrote:
>>
>>> diff --git a/migration/options.c b/migration/options.c
>>> index 645f55003d..f839493016 100644
>>> --- a/migration/options.c
>>> +++ b/migration/options.c
>>> @@ -29,6 +29,7 @@
>>>  #include "ram.h"
>>>  #include "options.h"
>>>  #include "sysemu/kvm.h"
>>> +#include <cpuid.h>
>>>
>>>  /* Maximum migrate downtime set to 2000 seconds */
>>>  #define MAX_MIGRATE_DOWNTIME_SECONDS 2000
>>> @@ -162,6 +163,10 @@ Property migration_properties[] = {
>>>      DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState,
>>>                         parameters.zero_page_detection,
>>>                         ZERO_PAGE_DETECTION_MULTIFD),
>>> +    /* DEFINE_PROP_ARRAY("dsa-accel-path", MigrationState, x, */
>>> +    /*                    parameters.dsa_accel_path, qdev_prop_string, 
>>> char *), */
>
> This is mostly correct, I think, you just need to create a field in
> MigrationState to keep the length (instead of x). However, I found out
> just now that this only works with QMP. Let me ask for other's
> opinions...
>
>>> +    /* DEFINE_PROP_STRING("dsa-accel-path", MigrationState, */
>>> +    /*                    parameters.dsa_accel_path), */
>>>
>>>      /* Migration capabilities */
>>>      DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
>>
>> I changed the dsa-accel-path to be a ['str'], i.e. strList* in C.
>> However, I am having a hard time about how to define the proper
>> properties here. I don't know what MACRO to use and I can't find good
>> examples... Need some guidance about how to proceed. Basically I will
>> need this to pass something like '-global
>> migration.dsa-accel-path="/dev/dsa/wq0.0"' in cmdline, or
>> "migrate_set_parameter dsa-accel-path" in QEMU CLI. Don't know how to
>> pass strList there.
>>
>> Thanks very much!
>
> @Daniel, @Markus, any idea here?
>
> If I'm reading this commit[1] right, it seems we decided to disallow
> passing of arrays without JSON, which affects -global on the
> command-line and HMP.
>
> 1- b06f8b500d (qdev: Rework array properties based on list visitor,
> 2023-11-09)
>
> QMP shell:
> (QEMU) migrate-set-parameters dsa-accel-path=['a','b']
> {"return": {}}
>
> HMP:
> (qemu) migrate_set_parameter dsa-accel-path "['a','b']"
> qemu-system-x86_64: ../qapi/string-input-visitor.c:343: parse_type_str:
> Assertion `siv->lm == LM_NONE' failed.
>
> Any recommendation? I believe all migration parameters so far can be set
> via those means, I don't think we can allow only this one to be
> QMP-only.
>
> Or am I just missing something?

I guess we could just skip adding property like Steve did here:

https://lore.kernel.org/r/1719776434-435013-10-git-send-email-steven.sistare@oracle.com