[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune

[Lei Li]

On 11/15/2011 12:46 PM, Eric Blake wrote:
> On 11/14/2011 09:33 PM, Lei Li wrote:
>> This patch add new pulic API virDomainSetBlockIoTune and
>> virDomainGetBlockIoTune.
>>
>> +/**
>> + * virDomainSetBlockIoTune:
>> + * @dom: pointer to domain object
>> + * @disk: Fully-qualified disk name
> Hmm. Fully-qualified name here,
>
>> + * @params: Pointer to blkio parameter objects
>> + * @nparams: Number of blkio parameters (this value can be the same or
>> + *           less than the number of parameters supported)
>> + * @flags: An OR'ed set of virDomainModificationImpact
>> + *
>> + * Change all or a subset of the per-device block IO tunables.
>> + *
>> + * The @disk parameter is the name of the block device.  Get this
>> + * by calling virDomainGetXMLDesc and finding the<target dev='...'>
>> + * attribute within //domain/devices/disk.  (For example, "xvda").
> but device shorthand here.
>
> We probably ought to accept both forms, just as we do with
> virDomainBlockStats, as of commit 89b6284f (oh, and that means that I
> ought to update virDomainBlockStats to document the same thing) - which
> means this can be a separate cleanup to make libvirt.c docs consistent
> as well as teaching the new API to use the domain_conf.c
> virDomainDiskIndexByName method to allow both forms.  I can get to that
> after finishing my review of this series, if you don't beat me.
>
Yes, both fully-qualified  and shorthand name can all work.



>> + * Get all block IO tunable parameters for a given device.  On input,
>> + * @nparams gives the size of the @params array; on output, @nparams
>> + * gives how many slots were filled with parameter information, which
>> + * might be less but will not exceed the input value.
>> + *
>> + * As a special case, calling with @params as NULL and @nparams as 0 on
>> + * input will cause @nparams on output to contain the number of parameters
>> + * supported by the hypervisor. The caller should then allocate @params
>> + * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API
>> + * agiain.
> Trailing space (running 'make syntax-check' will gripe).
>
Thanks..:)

> s/agiain/again/
>
> Also, probably worth mentioning that the value of @nparams may be
> disk-specific, so the user is best off reprobing an appropriate nparams
> for each disk rather than assuming the parameters from 1 disk apply to
> all others.
>
  The QEMU Block I/O Throttling support these whole 6 parameters, so I think
  display all the fields might make sense.


>> +++ b/src/libvirt_public.syms
>> @@ -496,6 +496,8 @@ LIBVIRT_0.9.7 {
>>           virDomainSnapshotGetParent;
>>           virDomainSnapshotListChildrenNames;
>>           virDomainSnapshotNumChildren;
>> +        virDomainSetBlockIoTune;
>> +        virDomainGetBlockIoTune;
>>   } LIBVIRT_0.9.5;
> This needs to be in a new section LIBVIRT_0.9.8 based off of
> LIBVIRT_0.9.5 (we aren't changing 0.9.7 at this point).
>
> The problems I mentioned are minor enough that I don't mind fixing them
> prior to pushing, if the rest of the series looks sane; but if later
> review finds more problems later in the series, we may need a v5 series
> instead.  I'll see about reviewing the rest of the series this week
> (I've also got quite a few nwfilter patches backed up in my todo queue).
>
OK. :) Thank you.

>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


-- 
Lei