All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Li, Liang Z" <liang.z.li@intel.com>,
	Markus Armbruster <armbru@redhat.com>
Cc: "quintela@redhat.com" <quintela@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"lcapitulino@redhat.com" <lcapitulino@redhat.com>,
	"amit.shah@redhat.com" <amit.shah@redhat.com>,
	"Zhang, Yang Z" <yang.z.zhang@intel.com>,
	"dgilbert@redhat.com" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [v5 12/12] migration: Add commands to set and query parameter
Date: Thu, 19 Mar 2015 08:33:35 -0600	[thread overview]
Message-ID: <550ADE3F.8000102@redhat.com> (raw)
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E4D2726@shsmsx102.ccr.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

On 03/18/2015 08:30 PM, Li, Liang Z wrote:

>>> +'migrate-set-parameters',
>>> +  'data': { 'parameters': ['MigrationParameterStatus'] } }
>>
>> The command takes a list of key-value pairs.  Looks like this (example stolen
>> from your patch to qmp-commands.hx):
>>
>>     { "execute": "migrate-set-parameters",
>>       "arguments": { "parameters":
>>                      [ { "parameter": "compress-level", "value": 1 } ] } }
>>
>> Awkward.  I'd very much prefer
>>
>>     { "execute": "migrate-set-parameters",
>>       "arguments": { "compress-level", 1 } }
>>
>> I.e. the command simply takes the parameters as optional arguments.
>> Simpler, and a natural use of the schema language.

Indeed, if we are going to add a new command, then we don't need to
worry about making it super-generic, and can avoid the nesting of
complex types.

> 
> Yes, it seems complicated.  Eric suggested to use this type of interface, because it can 
> support different type of parameters if some new parameters will be added.  

When I originally suggested complex nested types, I was suggesting that
we _reuse_ the existing migrate-set-capabilities (by making it set both
boolean and integer capabilities in one go).  If we do that, then we
need the struct complexity.  But since we are proposing a new command
rather than shoe-horning into an existing command, we might as well do
it cleanly to begin with.  So I like Markus' suggestion that we
eliminate some of the complexity and just go with specifying parameters
directly.


>> I'd very much prefer a simple object instead:
>>
>>     { "return": { "compress-level": 1,
>>                   "compress-threads": 8,
>>                   "decompress-threads": 2 } }
>>
> 
> The same reason.

I like the idea as well.  Sorry for misleading you into a more complex
initial implementation when a simpler will do.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

      parent reply	other threads:[~2015-03-19 14:33 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-11  3:06 [Qemu-devel] [PATCH v5 0/12] migration: Add a new feature to do live migration Liang Li
2015-02-11  3:06 ` [Qemu-devel] [v5 01/12] docs: Add a doc about multiple thread compression Liang Li
2015-02-11  8:46   ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 02/12] migration: Add the framework of multi-thread compression Liang Li
2015-02-11  8:52   ` Juan Quintela
2015-02-11  8:55   ` Juan Quintela
2015-02-11 11:10   ` Juan Quintela
2015-02-12  7:24     ` Li, Liang Z
2015-02-12 12:31       ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 03/12] migration: Add the framework of multi-thread decompression Liang Li
2015-02-11  8:52   ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 04/12] qemu-file: Add compression functions to QEMUFile Liang Li
2015-02-12 12:06   ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 05/12] arch_init: Alloc and free data struct for compression Liang Li
2015-02-11  9:03   ` Juan Quintela
2015-02-12  5:32     ` Li, Liang Z
2015-02-12 12:05       ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 06/12] arch_init: Add and free data struct for decompression Liang Li
2015-02-11  9:04   ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 07/12] migration: Split the function ram_save_page Liang Li
2015-02-11  9:08   ` Juan Quintela
2015-02-11 11:02   ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 08/12] migration: Add the core code of multi-thread compression Liang Li
2015-02-11 11:44   ` Juan Quintela
2015-02-12  7:43     ` Li, Liang Z
2015-03-02  2:46     ` Li, Liang Z
2015-02-11  3:06 ` [Qemu-devel] [v5 09/12] migration: Make compression co-work with xbzrle Liang Li
2015-02-11 11:46   ` Juan Quintela
2015-02-12  2:24     ` Li, Liang Z
2015-02-12 12:22       ` Juan Quintela
2015-02-12 15:10         ` Li, Liang Z
2015-02-11  3:06 ` [Qemu-devel] [v5 10/12] migration: Add the core code for decompression Liang Li
2015-02-11 11:48   ` Juan Quintela
2015-02-11  3:06 ` [Qemu-devel] [v5 11/12] migration: Add interface to control compression Liang Li
2015-02-11  3:06 ` [Qemu-devel] [v5 12/12] migration: Add commands to set and query parameter Liang Li
2015-02-11 11:53   ` Juan Quintela
2015-03-11 16:57   ` Dr. David Alan Gilbert
2015-03-12 10:30   ` Markus Armbruster
2015-03-19  2:30     ` Li, Liang Z
2015-03-19  7:49       ` Markus Armbruster
2015-03-19 14:21         ` Li, Liang Z
2015-03-20  5:16           ` Li, Liang Z
2015-03-19 14:33       ` Eric Blake [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=550ADE3F.8000102@redhat.com \
    --to=eblake@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=liang.z.li@intel.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=yang.z.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.