Re: [libvirt] [RFC PATCH 1/5] Add new API virDomainBlockIoThrottle

On 10/12/2011 01:07 AM, Zhi Yong Wu wrote: >On Tue, Oct 11, 2011 at 10:59 PM, Adam Litke&lt;agl(a)us.ibm.com&gt; wrote: >>On Mon, Oct 10, 2011 at 09:45:09PM +0800, Lei HH Li wrote: >> >>Hi Lei. You are missing a patch summary at the top of this email. In your >>summary you want to let reviewers know what the patch is doing. For example, >>this patch defines the new virDomainBlockIoThrottle API and specifies the XML >>schema. Also at the top of the patch you have an opportunity to explain why you >>made a particular design decision. For example, you could explain why you chose >I think so:). We should explain why we create one new libvirt >commands, not extending blkiotune. >BTW: Can we CCed these patches to those related developers to get >their comments? (e.g, Daniel, Gui JianFeng, etc) > >>to represent the throttling inside the<disk> tag rather than alongside the >><blkiotune> settings. I think the answer to this question lies in how it will be used. Looking at your patch, right now, we have: <domain> <blkiotune> <weight>nnn</weight> </blkiotune> </domain> which is global to the domain, but blkio throttling is specified per-disk and can vary across multiple disks. So mention in your commit message that you are proposing a per-disk element, which is why it does not fit into the existing <blkiotune>: <domain> <devices> <disk> <iothrottle .../> </disk> </devices> </domain>