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

[Eric Blake]

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.

> + * 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).

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.

> +++ 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).

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111114/b26f8bb2/attachment-0001.sig>