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

Re: [libvirt] [PATCH 02/15] Introduce virDomainResumeFlags



On 02/03/2014 09:16 AM, Michal Privoznik wrote:
> So far, we have just bare virDomainResume() API that resumes a domain.
> However, in the future there might occur a case, in which we may want
> to modify resume behavior slightly. In that case, @flags are useful.
> 
> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  include/libvirt/libvirt.h.in |  2 ++
>  src/driver.h                 |  5 ++++
>  src/libvirt.c                | 63 ++++++++++++++++++++++++++++++++++++--------
>  src/libvirt_public.syms      |  1 +
>  src/remote/remote_driver.c   |  1 +
>  src/remote/remote_protocol.x | 13 ++++++++-
>  src/remote_protocol-structs  |  5 ++++
>  7 files changed, 78 insertions(+), 12 deletions(-)

Same story as 1/15, where I like the change even if you don't have a
plan for the flags, but you should get a second opinion.

Also, I could see squashing 1 and 2 into a single patch - it doesn't
make sense to introduce one function without the other, especially since
you are touching cross-documentation to call out the new function names.

>  
>  /**
> + * virDomainResumeFlags:
> + * @domain: a domain object
> + * @flags: extra flags, not used yet, so callers should always pass 0
> + *
> + * Resume a suspended domain, the process is restarted from the state
> + * where it was frozen by calling virDomainSuspend() or
> + * virDomainSuspendFlags(). This function may require privileged
> + * access Moreover, resume may not be supported if domain is in some
> + * special state like VIR_DOMAIN_PMSUSPENDED.
> + *

As in patch 1, you need to add a comment to virDomainResume() mentioning
that it is short for virDomainResumeFlags(,0).


> +++ b/src/libvirt_public.syms
> @@ -648,6 +648,7 @@ LIBVIRT_1.2.1 {
>  LIBVIRT_1.2.2 {
>      global:
>          virDomainSuspendFlags;
> +        virDomainResumeFlags;

Not essential, but I like listing new APIs in alphabetical order.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
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]