On Fri, Feb 05, 2016 at 18:01:30 +0300, Nikolay Shirokovskiy wrote:
>
>
> On 04.02.2016 21:11, Jiri Denemark wrote:
> > On Thu, Jan 28, 2016 at 10:04:29 +0300, Nikolay Shirokovskiy wrote:
> >> From: ShaoHe Feng <shaohe.feng@xxxxxxxxx>
> >>
> >> Current compression does not use all range of parameter values
> >> so let's use some of them as 'unspecified' values. These
> >> values will be used to mark parameters that were not specified
> >> on migrate command line. Thus we check that qemu does not
> >> use these values when we read parameters.
> >>
> >> Signed-off-by: ShaoHe Feng <shaohe.feng@xxxxxxxxx>
> >> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
> >> ---
> >> src/qemu/qemu_monitor.c | 29 +++++++++++++
> >> src/qemu/qemu_monitor.h | 16 +++++++
> >> src/qemu/qemu_monitor_json.c | 87 +++++++++++++++++++++++++++++++++++++
> >> src/qemu/qemu_monitor_json.h | 5 +++
> >> src/qemu/qemu_monitor_text.c | 100 +++++++++++++++++++++++++++++++++++++++++++
> >> src/qemu/qemu_monitor_text.h | 5 +++
> >
> >>
> >>
> >> int
> >> +qemuMonitorGetMigrationCompressParametersMT(qemuMonitorPtr mon,
> >> + qemuMonitorMigrationMTParametersPtr params)
> >
> > I think a more general qemuMonitor[GS]etMigrationParameters [gs]etting
> > all parameters at once would be a bit better. After all, it all boils
> > down to query-migrate-parameters and migrate-set-parameters and having
> > separate functions for each group of parameters would mean we'd have to
> > call the QMP commands several times.
>
> AFAIK it is not so. We can't set(get) xbzrle options (only size now) and
> multithread options togather. We neet "query-migrate-cache-size" for the
> first and "query-migrate-parameters" for the second.
Yes, that's true. I didn't explicitly say so, but I was thinking about
future additions to query-migrate-perameters. It's very likely that new
parameters will be added and I think we should cover them all in one
QMP call. Rather than calling several qemuMonitorGet* functions which
would all run query-migrate-perameters command, but each of them would
be interested only in a subset of parameters.
We did similar stuff in the past with some query-block... command which
we called for every single disk again and again even though it returned
all of them at once. It's cleaner to just call it once, fill in a
structure describing the whole reply and do the filtering one layer up
to avoid calling the same command and parsing the same reply several
times.
> >> +int qemuMonitorJSONSetMigrationCompressParametersMT(qemuMonitorPtr mon,
> >> + qemuMonitorMigrationMTParametersPtr params)
> >> +{
> >> + int ret = -1;
> >> + virJSONValuePtr cmd;
> >> + virJSONValuePtr reply = NULL;
> >> +
> >> + cmd = qemuMonitorJSONMakeCommand("migrate-set-parameters",
> >> + "i:compress-level", params->level,
> >> + "u:compress-threads", params->threads,
> >> + "u:decompress-threads", params->dthreads,
> >> + NULL);
> >
> > Is passing an "undefined" value for any of these parameters allowed
> > (i.e., will QEMU use a default value) or do we always have to set all of
> > them?
>
> QEMU is good enough to take partial parameters specs. In this case the other
> just stay untouched. But I never use this command in this mode and specify
> all parameters. I do it to make sure we don't get compression parameters
> from previous failed migration attempt. See patch number 5.
Yeah, I saw that and it was the reason I asked the question :-)
> Basically instead of reverting parameters on unsuccessful migration I
> use hardcoded default values for parameters that are not specified on
> migration.
Somehow I don't like the hardcoded default values. Either we should
force users to specify all parameters or we could check current
values from QEMU, change some of them as requested by a user, migrate,
and revert the values back to their original values. Or we can just let
another migration attempt reuse the previous values (as we do with other
parameters, I believe).
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list