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

Eric Blake <eblake@xxxxxxxxxx> · Wed, 05 Oct 2011 17:02:45 -0600




On 10/05/2011 11:31 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@xxxxxxxxxx>

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@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





