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

Re: [libvirt] [RFC v4] Export KVM Host Power Management capabilities



On 08/09/2011 09:29 AM, Srivatsa S. Bhat wrote:
+    if(caps->host.powerMgmt_valid) {
+        /* The PM query was successful. */
+        if(caps->host.powerMgmt) {
+            /* The host supports some PM features. */
+            unsigned int pm = caps->host.powerMgmt;
+            virBufferAddLit(&xml, "<power_management>\n");
+            while(pm) {

Our style is to use space after keywords and before opening '(' (that is, you should be using 'if (', 'while ('); we only omit space on function calls (your use of 'virBufferAddLit(' is correct).

+++ b/src/qemu/qemu_capabilities.c
@@ -794,6 +794,8 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps)
      struct utsname utsname;
      virCapsPtr caps;
      int i;
+    int status = -1;
+    unsigned int pmbitmask = 0;

You don't need this temporary.  Instead,...

      char *xenner = NULL;

      /* Really, this never fails - look at the man-page. */
@@ -824,6 +826,22 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps)
          old_caps->host.cpu = NULL;
      }

+    /* Add the power management features of the host */
+
+    status = virGetPMCapabilities(&pmbitmask);
+    if(status<  0) {
+        caps->host.powerMgmt_valid = false;

VIR_ALLOC guaranteed that false was already the default.

+        VIR_WARN("Failed to get host power management capabilities");
+    } else {
+        /* The PM query succeeded. */

...you can just do the assignment in place:

if (virGetPMCapabilities(&caps->host.powerMgmt) < 0)
    VIR_WARN("Failed to get host power management capabilities");
else
    caps->host.powerMgmt_valid = true;

+int
+virGetPMCapabilities(unsigned int * bitmask)

Our style does not use space after '*' for pointers:

unsigned int *bitmask

+    if((path = virFindFileInPath("pm-is-supported")) == NULL) {
+        virUtilError(VIR_ERR_INTERNAL_ERROR,
+                     "%s", _("Failed to get the path of pm-is-supported"));
+        return -1;
+    }

Overkill - virCommand already does this check on your behalf.

+
+    /* Check support for Suspend-to-RAM (S3) */
+    cmd = virCommandNew(path);
+    virCommandAddArg(cmd, "--suspend");
+    if(virCommandRun(cmd,&status)<  0) {
+        virUtilError(VIR_ERR_INTERNAL_ERROR,
+                     "%s", _("Failed to run command"
+                             "'pm-is-supported --suspend'"));

Overkill - virCommand already issues a (much better) error message on your behalf.

I'd use:

int ret = -1;
int status;

cmd = virCommandNewArgList("pm-is-supported", "--suspend", NULL);
if (virCommandRun(cmd, &status) < 0)
    goto cleanup;
if (status == 0)
    *bitmask |= 1U << VIR_HOST_PM_S3;
virCommandFree(cmd);
cmd = virCommandNewArgList("pm-is-supported", "--hibernate", NULL);
if (virCommandRun(cmd, &status) < 0)
    goto cleanup;
if (status == 0)
    *bitmask |= 1U << VIR_HOST_PM_S4;
ret = 0;

cleanup:
virCommandFree(cmd);
return ret;

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


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