[libvirt] [PATCH 2/4] Add new virDomainShutdownFlags API

[Eric Blake]

On 10/05/2011 11:31 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange at redhat.com>
>
> 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 seperately in the future.

s/seperately/separately/

>
> Add stub impls of the new API for all existing drivers

well, except for qemu in the next patch :)

>   * _virDriver:
> @@ -881,6 +885,7 @@ struct _virDriver {
>      virDrvDomainGetBlockJobInfo domainGetBlockJobInfo;
>      virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed;
>      virDrvDomainBlockPull domainBlockPull;
> +    virDrvDomainShutdownFlags domainShutdownFlags;
>  };

Should we list this driver callback next to domainShutdown, to match the 
fact that most hypervisors now list the two callbacks side-by-side?  I 
don't know if gcc can generate slightly better code when the 
initializers appear in the same order in the hypervisors as they are 
declared in this struct.

>
>   /**
> + * virDomainShutdownFlags:
> + * @domain: a domain object
> + * @flags: optional flags
> + *
> + * Shutdown a domain, the domain object is still usable thereafter but
> + * the domain OS is being stopped. Note that the guest OS may ignore the
> + * request.  For guests that react to a shutdown request, the differences
> + * from virDomainDestroy() are that the guests disk storage will be in a

s/guests/guest's/ (probably copy and paste of an existing typo)

> + * stable state rather than having the (virtual) power cord pulled, and
> + * this command returns as soon as the shutdown request is issued rather
> + * than blocking until the guest is no longer running.
> + *
> + * If the domain is transient and has any snapshot metadata (see
> + * virDomainSnapshotNum()), then that metadata will automatically
> + * be deleted when the domain quits.
> + *
> + * If @flags is set to zero, then the hypervisor will chose the
> + * method of shutdown it considers best. To have greater control
> + * pass one of the shutdown flags

Should we list the flag names here, or at least the enum name so that 
the html docs will have a hyperlink to the flag names?

> +++ b/src/libvirt_public.syms
> @@ -493,6 +493,7 @@ LIBVIRT_0.9.7 {
>       global:
>           virDomainReset;
>           virDomainSnapshotGetParent;
> +        virDomainShutdownFlags;

I know, sorting doesn't matter here...  :)

> +++ b/src/openvz/openvz_driver.c
> @@ -1689,6 +1689,7 @@ static virDriver openvzDriver = {
>       .domainSuspend = openvzDomainSuspend, /* 0.8.3 */
>       .domainResume = openvzDomainResume, /* 0.8.3 */
>       .domainShutdown = openvzDomainShutdown, /* 0.3.1 */
> +    .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.7 */

You obviously aren't compiling openvz, as this is missing the new 
function openvzDomainShutdownFlags.

> +++ b/src/vmware/vmware_driver.c
> @@ -974,6 +974,7 @@ static virDriver vmwareDriver = {
>       .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */
>       .domainResume = vmwareDomainResume, /* 0.8.7 */
>       .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */
> +    .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.7 */

Same goes for you not compiling vmware.

> +++ b/src/xenapi/xenapi_driver.c
> @@ -773,12 +773,15 @@ xenapiDomainResume (virDomainPtr dom)
>    * Returns 0 on success or -1 in case of error
>    */
>   static int
> -xenapiDomainShutdown (virDomainPtr dom)
> +xenapiDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
>   {
>       /* vm.clean_shutdown */
>       xen_vm vm;
>       xen_vm_set *vms;
>       xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session;
> +
> +    virCheckFlags(0, -1);
> +
>       if (xen_vm_get_by_name_label(session,&vms, dom->name)&&  vms->size>  0) {
>           if (vms->size != 1) {
>               xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,
> @@ -801,6 +804,12 @@ xenapiDomainShutdown (virDomainPtr dom)
>       return -1;
>   }
>
> +static int
> +xenapiDomainShutdown(virDomainPtr dom)
> +{
> +    return xenapiDomainShutdownFlags(dom, 0);
> +}
> +

Here, you have the opposite problem - you forgot to register 
xenapiDomainShutdownFlags in the list of driver callbacks.

The fixes seem trivial enough, so Conditional ACK.

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