Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> [...]
> >> > I'm confused why we need a 'reset to default' - all we need is the ability
> >> > to change each parameter, and for the new value of that parameter
> >> > to be an empty string.
> >> 
> >> You argue syntax, I'm arguing semantics.
> >> 
> >> The command means "set parameter P to value V".  *Except* when V is "",
> >> it means something else, namely "reset parameter P to its default,
> >> whatever that may be".
> >> 
> >> This is (a) not general, because it won't do for cases where "" may
> >> occur as value, and (b) ugly.
> >> 
> >> Ugliness is the eye of the beholder.  Lack of generality isn't.
> >
> > No, I'm questioning why it's defined as 'reset parameter P to its default';
> > why do we need a way to do that?
> 
> Dan's commit message explains:
> 
>     Some of the migration parameters are strings, which default to NULL,
>     eg tls_hostname and tls_creds.
> 
>     The mgmt app will set the tls_creds parameter on both source and target
>     QEMU instances, in order to trigger use of TLS for migration.
> 
>     After performing a TLS encrypted migration though, migration might be
>     used for other reasons - for example, to save the QEMU state to a file.
>     We need TLS turned off when doing this, but the migrate-set-parameters
>     QAPI command does not provide any facility to clear/reset parameters
>     to their default state.
> 
>     If you simply ommit the tls_creds parameter in migrate-set-parameters,
>     then 'has_tls_creds' will be false and so no action will be taken. The
>     only option that works with migrate-set-parameters is to treat "" on
>     the wire as equivalent to requesting NULL. Failing that we would have
>     to create a new 'migrate-reset-parameters' method to explicitly put
>     a parameter back to its default value.

OK, I thought the default was empty string.
IMHO the problem here is we're just being too clever; lets just make
the default "" rather than NULL, and make that parameter always be
a string.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK