[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_m
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge |
Date: |
Fri, 31 Oct 2014 09:31:53 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.0 |
Am 28.10.2014 um 12:11 schrieb Stefan Hajnoczi:
> On Sat, Oct 25, 2014 at 06:55:50PM +0200, Peter Lieven wrote:
>> diff --git a/block.c b/block.c
>> index f05ea0c..f3da5dd 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -884,6 +884,10 @@ static QemuOptsList bdrv_runtime_opts = {
>> .name = "node-name",
>> .type = QEMU_OPT_STRING,
>> .help = "Node name of the block device node",
>> + },{
>> + .name = "write-merging",
>> + .type = QEMU_OPT_BOOL,
>> + .help = "enable write merging (default: true)",
> QEMU_OPT_BOOL takes "on"/"off" instead of "true"/"false":
> s/true/on/
>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 2095f9e..74d1960 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -214,6 +214,8 @@
>> #
>> # @detect_zeroes: detect and optimize zero writes (Since 2.1)
>> #
>> +# @write_merging: true if write merging is enabled (Since 2.2)
>> +#
>> # @bps: total throughput limit in bytes per second is specified
>> #
>> # @bps_rd: read throughput limit in bytes per second is specified
>> @@ -250,6 +252,7 @@
>> '*backing_file': 'str', 'backing_file_depth': 'int',
>> 'encrypted': 'bool', 'encryption_key_missing': 'bool',
>> 'detect_zeroes': 'BlockdevDetectZeroesOptions',
>> + 'write_merging': 'bool',
>> 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
>> 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
>> 'image': 'ImageInfo',
>> @@ -1185,6 +1188,10 @@
>> # (default: false)
>> # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
>> # (default: off)
>> +# @write-merging: #optional enable the merging of write requests
>> +# also known as multiwrite_merge (Since 2.2)
>> +# (default: true, but this might change in the future
>> +# depending on format/protocol/features used)
>> #
>> # Since: 1.7
>> ##
>> @@ -1198,7 +1205,8 @@
>> '*rerror': 'BlockdevOnError',
>> '*werror': 'BlockdevOnError',
>> '*read-only': 'bool',
>> - '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
>> + '*detect-zeroes': 'BlockdevDetectZeroesOptions',
>> + '*write-merging': 'bool' } }
>>
>> ##
>> # @BlockdevOptionsFile
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 22cf3b9..d2f756f 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -432,6 +432,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
>> "
>> [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
>> " [,readonly=on|off][,copy-on-read=on|off]\n"
>> " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
>> + " [,write-merging=on|off]\n"
>> " [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
>> " [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
>> " [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 1abd619..2c20207 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -2104,6 +2104,7 @@ Each json-object contain the following:
>> - "iops_size": I/O size when limiting by iops (json-int)
>> - "detect_zeroes": detect and optimize zero writing (json-string)
>> - Possible values: "off", "on", "unmap"
>> + - "write_merging": enable merging of write requests (json-bool)
>> - "image": the detail of the image, it is a json-object containing
>> the following:
>> - "filename": image file name (json-string)
>> @@ -2181,6 +2182,7 @@ Example:
>> "iops_wr_max": 0,
>> "iops_size": 0,
>> "detect_zeroes": "on",
>> + "write_merging": "true",
>> "image":{
>> "filename":"disks/test.qcow2",
>> "format":"qcow2",
> The big question is whether these user-visible interfaces make sense if
> write merging will be moved from block.c into virtio-blk.c in the
> future. Once an interface has been added it cannot be removed.
Would you make it a feature of the virtio-blk-pci device then?
Peter
>
> I think write merging in block.c is fine since other emulated storage
> controllers like virtio-scsi might also choose to use it.
>
> But I want to raise the point for discussion in case others have strong
> feelings.
>
> Stefan