[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Wed, Oct 12, 2011 at 03:49:26PM -0600, Eric Blake wrote:
> On 10/12/2011 01:07 AM, Zhi Yong Wu wrote:
> >On Tue, Oct 11, 2011 at 10:59 PM, Adam Litke<agl us ibm com>  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>

Since the top level element is 'blkiotune', I think it would
make sense for the per-disk element to be 'iotune' instead
of 'iothrottle', because not all of the tunables will neccessarily
imply throttling.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]