Am 01.08.2011 17:28, schrieb Anthony Liguori: > On 08/01/2011 10:22 AM, Stefan Hajnoczi wrote: >> 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. > > I'm strongly opposed to this. There needs to be a single consistent way > to determine supported operations with QMP. > > And that single mechanism already exists--query_commands. > >> 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? > > For the sake of overall QMP sanity, I think block_set_hostcache is > really our only option.
Ideally we should have blockdev_add, and blockdev_set would just take the same arguments and update the given driver. But we don't have blockdev_add today, so whatever works for your as a temporary solution... Kevin