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

Re: [libvirt] [PATCHv6 0/5] expose per-device blkio weight tuning, via string param



On Thu, Nov 03, 2011 at 12:05:56PM -0600, Eric Blake wrote:
> This is v6 of Hu's original proposal:
> v1: https://www.redhat.com/archives/libvir-list/2011-September/msg00334.html
> v4: https://www.redhat.com/archives/libvir-list/2011-October/msg00444.html
> v5: https://www.redhat.com/archives/libvir-list/2011-November/msg00060.html
> 
> Patches 1-3 are solid in my testing.  When I first started working
> on these patches, I was thinking it would be worth having
> VIR_TYPED_PARAM_STRING in 0.9.7; however, now that I've spent
> longer on it, I'm reluctant to push this new virTypedParameter API
> without at least one other API that takes advantage of a typed string.
> 
> I tested 1-3 by using patches 4 and 5, using all 4 combinations
> of pre- and post-patch clients and servers.  If either (or both)
> sides were old, then behavior is unchanged from 0.9.6 (no strings
> are present, a new client won't confuse an old server, a new
> server won't confuse an old client, and all non-string parameters
> are still passed in all cases); if both sides are new, then
> the string passing works as designed.
> 
> Patch 4 is close to feature-complete, and would be the first API to
> take advantage of patches 1-3, but it is pointless to apply it
> without at least one driver implementation.  Also, it has a user
> interface issue, in that '/path/to/dev1,weight;/path/to/dev2,weight'
> is awkward to type in a virsh command line; it might be nicer if
> if we used the string '/path/to/dev1,weight,/path/to/dev2,weight'
> instead, but that would mean some further parsing alterations.
> 
> Note that in patch 4, I renamed the virsh option to
> 'virsh blkiotune --device-weight', in part because having both
> '--weight-device' and '--weight' would interfere with my (future)
> plans to make virsh do unambiguous option prefix parsing, and in
> part because the XML is laid out with device as the primary name,
> with weight as a sub-feature of each device (just because cgroups
> named things backwards doesn't mean that we have to repeat that
> mistake).
> 
> Patch 5 is broken.  I've already spent too long on this series, so
> I'm hoping Hu can resume ownership of this series and post a working
> v7 that resolves my complaints.  More details in that particular
> patch message; but the short summary is that 'virsh blkiotune --live'
> and 'virsh blkiotune --config' must give identically formatted
> output.

Much thanks for your work on this series. I'll do v7 to address the
problems you mentioned in the summary of patch 5.

-- 
Thanks,
Hu Tao


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