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

Re: [libvirt] [PATCH v2 2/4] Add new virDomainShutdownFlags API



On 01/23/2012 07:48 AM, Michal Privoznik wrote:
> Add a new API virDomainShutdownFlags and define:
> 
>     VIR_DOMAIN_SHUTDOWN_DEFAULT        = 0,
>     VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0),
>     VIR_DOMAIN_SHUTDOWN_GUEST_AGENT    = (1 << 1),
> 
> Also define some flags for the reboot API
> 
>     VIR_DOMAIN_REBOOT_DEFAULT        = 0,
>     VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0),
>     VIR_DOMAIN_REBOOT_GUEST_AGENT    = (1 << 1),
> 
> Although these two APIs currently have the same flags, using
> separate enums allows them to expand separately in the future.
> 
> Add stub impls of the new API for all existing drivers

> ---
> +++ b/include/libvirt/libvirt.h.in
> @@ -1200,7 +1200,22 @@ virDomainPtr            virDomainLookupByUUID   (virConnectPtr conn,
>  virDomainPtr            virDomainLookupByUUIDString     (virConnectPtr conn,
>                                                          const char *uuid);
>  
> +typedef enum {
> +    VIR_DOMAIN_SHUTDOWN_DEFAULT        = 0,        /* hypervisor to choose */

/to choose/choice/

> +    VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0), /* Send ACPI event */
> +    VIR_DOMAIN_SHUTDOWN_GUEST_AGENT    = (1 << 1), /* Use guest agent */
> +} virDomainShutdownFlagValues;
> +
>  int                     virDomainShutdown       (virDomainPtr domain);
> +int                     virDomainShutdownFlags  (virDomainPtr domain,
> +                                                 unsigned int flags);
> +
> +typedef enum {
> +    VIR_DOMAIN_REBOOT_DEFAULT        = 0,        /* hypervisor to choose */

and again.

> + * If @flags is set to zero, then the hypervisor will chose the

s/chose/choose/

> + * method of shutdown it considers best. To have greater control
> + * pass exactly one of the virDomainShutdownFlagValues.

Does it make sense to enforce the mutual exclusion of these flags here?
 For example, in various functions that take exactly one of
VIR_DOMAIN_AFFECT_{LIVE,CONFIG}, we enforce the mutual exclusion in
libvirt.c, so that hypervisor drivers don't have to repeat the check.

>   * virDomainReboot:
>   * @domain: a domain object
> - * @flags: extra flags; not used yet, so callers should always pass 0
> + * @flags: bitwise-OR of virDomainRebootFlagValues
>   *
>   * Reboot a domain, the domain object is still usable there after but
>   * the domain OS is being stopped for a restart.
>   * Note that the guest OS may ignore the request.
>   *
> + * If @flags is set to zero, then the hypervisor will chose the

s/chose/choose/

I can live with this with just the spelling nits fixed, so:

ACK

However, if you want to post a v3 with the mutual exclusion check added
to libvirt.c, that would also be reasonable.

-- 
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]