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

Re: [libvirt] [PATCH 1/3] Add public API for setting migration speed on the fly



On 03/21/2011 10:13 AM, Daniel P. Berrange wrote:
> It is possible to set a migration speed limit when starting
> migration. This new API allows the speed limit to be changed
> on the fly to adjust to changing conditions
> 
> * src/driver.h, src/libvirt.c, src/libvirt_public.syms,
>   include/libvirt/libvirt.h.in: Add virDomainMigrateSetMaxSpeed
> * src/esx/esx_driver.c, src/lxc/lxc_driver.c,
>   src/opennebula/one_driver.c, src/openvz/openvz_driver.c,
>   src/phyp/phyp_driver.c, src/qemu/qemu_driver.c,
>   src/remote/remote_driver.c, src/test/test_driver.c,
>   src/uml/uml_driver.c, src/vbox/vbox_tmpl.c,
>   src/vmware/vmware_driver.c, src/xen/xen_driver.c: Stub new
>   API

Missing mention of the new libxl driver...

> ---
>  include/libvirt/libvirt.h.in |    4 +++
>  src/driver.h                 |    5 ++++
>  src/esx/esx_driver.c         |    1 +
>  src/libvirt.c                |   47 ++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |    1 +
>  src/libxl/libxl_driver.c     |    1 +

but the diffstat shows you picked it up :)

> +++ b/include/libvirt/libvirt.h.in
> @@ -436,6 +436,10 @@ int virDomainMigrateSetMaxDowntime (virDomainPtr domain,
>                                      unsigned long long downtime,
>                                      unsigned int flags);
>  
> +int virDomainMigrateSetMaxSpeed(virDomainPtr domain,
> +                                unsigned long bandwidth,
> +                                unsigned int flags);

Hmm - SetMaxDowntime and SetMaxSpeed; this looks like we might be
running into scalability issues.  Are there likely to ever be other
migration parameters worth setting on the fly?  If so, is it better to
come up with an all-inclusive API, more like the memory and blkio
parameters have recently done (where you pass in a
string-name/union-value mapping for each parameter of interest through a
single API)?

However, if we think that there won't be too many other migration
parameters to be tweaked, then this looks fine.

>  
>  /**
> + * virDomainMigrateSetMaxSpeed:
> + * @domain: a domain object
> + * @bandwidth: migration bandwidth limit in Mbps
> + * @flags: fine-tuning flags, currently unused, use 0
> + *
> + * The maximum bandwidth (in Mbps) that will be used to do migration
> + * can be specified with the bandwidth parameter. Not all hypervisors
> + * will support a bandwidth cap
> + *
> + * Returns 0 in case of success, -1 otherwise.
> + */

Looks reasonable.
> +++ b/src/libvirt_public.syms
> @@ -431,6 +431,7 @@ LIBVIRT_0.9.0 {
>          virDomainSetMemoryFlags;
>          virEventRegisterDefaultImpl;
>          virEventRunDefaultImpl;
> +        virDomainMigrateSetMaxSpeed;

Should this list stay sorted?

ACK with those nits addressed.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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