Re: [PATCH for-8.2 v2 1/2] qapi/migration: Deduplicate migration parameter field comments

[Peter Xu <peterx@redhat.com>]

On Tue, Aug 08, 2023 at 04:03:46PM -0400, Peter Xu wrote:
> On Sun, Aug 06, 2023 at 11:49:46AM -0400, Peter Xu wrote:
> > > I think we have a tradeoff here.  If perpetuating the unclean and ugly
> > > use of "" is what it takes to de-triplicate migration parameters, we may
> > > decide to accept that.
> > 
> > I don't think it's a must.  As Dan raised, we can convert str -> StrOrNull
> > for MigrationParameters. I assume it won't affect query-migrate-parameters
> > anyway OTOH.
> > 
> > I assume it means there's nothing yet obvious that we overlooked on the
> > whole idea.  Let me propose the formal patchset early next week.  It'll be
> > mostly the patch I attached but just add those extra logics for StrOrNull,
> > so the diffstat might be less attractive but hopefully still good enough to
> > be accepted.
> 
> The new StrOrNull approach doesn't work with current migration object
> properties.. as StrOrNull must be a pointer for @MigrationParameters not
> static, and it stops working with offsetof():
> 
> ../migration/options.c:218:5: error: cannot apply ‘offsetof’ to a non constant address
>   218 |     DEFINE_PROP_STRING("tls-creds", MigrationState, parameters.tls_creds->u.s),
>       |     ^~~~~~~~~~~~~~~~~~
> ../migration/options.c:219:5: error: cannot apply ‘offsetof’ to a non constant address
>   219 |     DEFINE_PROP_STRING("tls-hostname", MigrationState, parameters.tls_hostname->u.s),
>       |     ^~~~~~~~~~~~~~~~~~
> ../migration/options.c:220:5: error: cannot apply ‘offsetof’ to a non constant address
>   220 |     DEFINE_PROP_STRING("tls-authz", MigrationState, parameters.tls_authz->u.s),
>       |     ^~~~~~~~~~~~~~~~~~
> 
> Any easy way to fix this?  I.e., is there a way to declare StrOrNull (in
> MigrationParameters of qapi/migration.json) to be statically allocated
> rather than a pointer (just like default behavior of any uint* types)?

Posted a version with 'str' replacing 'StrOrNull'.  Let's move the
discussion there:

https://lore.kernel.org/r/20230814221947.353093-1-peterx@redhat.com

-- 
Peter Xu