On Thu, Jul 28, 2011 at 10:29 AM, Stefan Hajnoczi <stefa...@gmail.com> wrote: > On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori <anth...@codemonkey.ws> > wrote: >> On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote: >>> >>> On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori<anth...@codemonkey.ws> >>> wrote: >>>>> >>>>> Index: qemu/hmp-commands.hx >>>>> =================================================================== >>>>> --- qemu.orig/hmp-commands.hx >>>>> +++ qemu/hmp-commands.hx >>>>> @@ -70,6 +70,20 @@ but should be used with extreme caution. >>>>> resizes image files, it can not resize block devices like LVM volumes. >>>>> ETEXI >>>>> >>>>> + { >>>>> + .name = "block_set", >>>>> + .args_type = "device:B,device:O", >>>>> + .params = "device [prop=value][,...]", >>>>> + .help = "Change block device parameters >>>>> [hostcache=on/off]", >>>>> + .user_print = monitor_user_noop, >>>>> + .mhandler.cmd_new = do_block_set, >>>>> + }, >>>>> +STEXI >>>>> +@item block_set @var{config} >>>>> +@findex block_set >>>>> +Change block device parameters (eg: hostcache=on/off) while guest is >>>>> running. >>>>> +ETEXI >>>>> + >>>> >>>> block_set_hostcache() please. >>>> >>>> Multiplexing commands is generally a bad idea. It weakens typing. In >>>> the >>>> absence of a generic way to set block device properties, implementing >>>> properties as generic in the QMP layer seems like a bad idea to me. >>> >>> The idea behind block_set was to have a unified interface for changing >>> block device parameters at runtime. This prevents us from reinventing >>> new commands from scratch. For example, block I/O throttling is >>> already queued up to add run-time parameters. >>> >>> Without a unified command we have a bulkier QMP/HMP interface, >>> duplicated code, and possibly inconsistencies in syntax between the >>> commands. Isn't the best way to avoid these problems a unified >>> interface? >>> >>> I understand the lack of type safety concern but in this case we >>> already have to manually pull parsed arguments (i.e. cast to specific >>> types and deal with invalid input). To me this is a reason *for* >>> using a unified interface like block_set. >> >> Think about it from a client perspective. How do I determine which >> properties are supported by this version of QEMU? I have no way to identify >> programmatically what arguments are valid for block_set. >> >> OTOH, if you have strong types like block_set_hostcache, query-commands >> tells me exactly what's supported. > > Use query-block and see if 'hostcache' is there. If yes, then the > hostcache parameter is available. If we allow BlockDrivers to have > their own runtime parameters then query-commands does not tell you > anything because the specific BlockDriver may or may not support that > runtime parameter - you need to use query-block.
Let's reach agreement here. The choices are: 1. Top-level block_set command. Supported parameters are discovered by looking query-block output. 2. Top-level command for each parameter (e.g. block_set_hostcache). Supported parameters are easily discoverable via query-commands. If individual block devices support different sets of parameters then they may have to return -ENOTSUPP. I like the block_set approach. Anthony, Kevin, Supriya: Any thoughts? Stefan