[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration: allow clearing migration string para
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters |
Date: |
Wed, 15 Mar 2017 10:49:29 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Wed, Mar 15, 2017 at 10:36:35AM +0000, Dr. David Alan Gilbert wrote:
> * Markus Armbruster (address@hidden) wrote:
> > "Dr. David Alan Gilbert" <address@hidden> writes:
> >
> > > * Markus Armbruster (address@hidden) wrote:
> > >> "Dr. David Alan Gilbert" <address@hidden> 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.
>From an internal migration code POV I think NULL is right, but none the
less I'll create a patch showing what using "" internally would look
like so we can compare
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
- Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters, (continued)
- Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters, Dr. David Alan Gilbert, 2017/03/14
- Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters, Markus Armbruster, 2017/03/15
- Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters, Dr. David Alan Gilbert, 2017/03/15
- Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters, Markus Armbruster, 2017/03/15
- Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters, Dr. David Alan Gilbert, 2017/03/15
- Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters,
Daniel P. Berrange <=