[libvirt] [RFC v2] Export KVM Host Power Management capabilities
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host. The script "pm-is-supported" (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag "<power_management>" is introduced in the XML under the <host> tag. Eg: When the host supports both S3 and S4, the XML looks like this: <capabilities> <host> <uuid>dc699581-48a2-11cb-b8a8-9a0265a79bbe</uuid> <cpu> <arch>i686</arch> <model>coreduo</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> <feature name='xtpr'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> <feature name='pbe'/> <feature name='tm'/> <feature name='ht'/> <feature name='ss'/> <feature name='acpi'/> <feature name='ds'/> </cpu> <power_management> <<<=== New host power management features <S3/> <S4/> </power_management> <migration_features> <live/> <uri_transports> <uri_transport>tcp</uri_transport> </uri_transports> </migration_features> </host> . . . However in case the host does not support any power management feature, then the XML will not contain the <power_management> tag. The initial discussion about this patch was done in [1]. And the choice to name the new tag as "power_management" was discussed in [2]. Please let me know your comments and feedback. References: ---------- [1] Exporting KVM host power saving capabilities through libvirt http://thread.gmane.org/gmane.comp.emulators.libvirt/40886 [2] http://article.gmane.org/gmane.comp.emulators.libvirt/41688 Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> --- src/conf/capabilities.c | 34 +++++++++++++++++++++++++++ src/conf/capabilities.h | 7 ++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 10 ++++++++ src/util/util.c | 52 ++++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 7 ++++++ 6 files changed, 112 insertions(+), 0 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 2f243ae..94423dc 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -166,6 +166,10 @@ virCapabilitiesFree(virCapsPtr caps) { virCapabilitiesFreeNUMAInfo(caps); + for(i = 0; i < caps->host.npowerMgmt ; i++) + VIR_FREE(caps->host.powerMgmt[i]); + VIR_FREE(caps->host.powerMgmt); + for (i = 0 ; i < caps->host.nmigrateTrans ; i++) VIR_FREE(caps->host.migrateTrans[i]); VIR_FREE(caps->host.migrateTrans); @@ -201,6 +205,27 @@ virCapabilitiesAddHostFeature(virCapsPtr caps, return 0; } +/** + * virCapabilitiesAddHostPowerManagement: + * @caps: capabilities to extend + * @name: name of power management feature + * + * Registers a new host power management feature, eg: 'S3' or 'S4' + */ +int +virCapabilitiesAddHostPowerManagement(virCapsPtr caps, + const char *name) +{ + if(VIR_RESIZE_N(caps->host.powerMgmt, caps->host.npowerMgmt_max, + caps->host.npowerMgmt, 1) < 0) + return -1; + + if((caps->host.powerMgmt[caps->host.npowerMgmt] = strdup(name)) == NULL) + return -1; + caps->host.npowerMgmt++; + + return 0; +} /** * virCapabilitiesAddHostMigrateTransport: @@ -686,6 +711,15 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " </cpu>\n"); + if(caps->host.npowerMgmt) { + virBufferAddLit(&xml, " <power_management>\n"); + for (i = 0; i < caps->host.npowerMgmt ; i++) { + virBufferAsprintf(&xml, " <%s/>\n", + caps->host.powerMgmt[i]); + } + virBufferAddLit(&xml, " </power_management>\n"); + } + if (caps->host.offlineMigrate) { virBufferAddLit(&xml, " <migration_features>\n"); if (caps->host.liveMigrate) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index e2fa1d6..eb6e561 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -105,6 +105,9 @@ struct _virCapsHost { size_t nfeatures; size_t nfeatures_max; char **features; + size_t npowerMgmt; + size_t npowerMgmt_max; + char **powerMgmt; int offlineMigrate; int liveMigrate; size_t nmigrateTrans; @@ -186,6 +189,10 @@ virCapabilitiesAddHostFeature(virCapsPtr caps, const char *name); extern int +virCapabilitiesAddHostPowerManagement(virCapsPtr caps, + const char *name); + +extern int virCapabilitiesAddHostMigrateTransport(virCapsPtr caps, const char *name); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 830222b..5754fdd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -41,6 +41,7 @@ virCapabilitiesAddGuestFeature; virCapabilitiesAddHostFeature; virCapabilitiesAddHostMigrateTransport; virCapabilitiesAddHostNUMACell; +virCapabilitiesAddHostPowerManagement; virCapabilitiesAllocMachines; virCapabilitiesDefaultGuestArch; virCapabilitiesDefaultGuestEmulator; @@ -1025,6 +1026,7 @@ safezero; virArgvToString; virAsprintf; virBuildPathInternal; +virCheckPMCapability; virDirCreate; virEmitXMLWarning; virEnumFromString; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3f36212..6e969a7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -824,6 +824,16 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps) old_caps->host.cpu = NULL; } + /* Add the power management features of the host */ + + /* Check for Suspend-to-RAM support (S3) */ + if(virCheckPMCapability(HOST_PM_S3) == 0) + virCapabilitiesAddHostPowerManagement(caps, "S3"); + + /* Check for Suspend-to-Disk support (S4) */ + if(virCheckPMCapability(HOST_PM_S4) == 0) + virCapabilitiesAddHostPowerManagement(caps, "S4"); + virCapabilitiesAddHostMigrateTransport(caps, "tcp"); diff --git a/src/util/util.c b/src/util/util.c index 03a9e1a..9893597 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2641,3 +2641,55 @@ or other application using the libvirt API.\n\ return 0; } + +/** + * Check the Power Management Capabilities of the host system. + * The script 'pm-is-supported' (from the pm-utils package) is run + * to find out if the capability is supported by the host. + * + * @capability: capability to check for + * HOST_PM_S3: Check for Suspend-to-RAM support + * HOST_PM_S4: Check for Suspend-to-Disk support + * + * Returns 0 if supported, -1 if not supported. + */ +int +virCheckPMCapability(int capability) +{ + + char *path = NULL; + int status = -1, ret = -1; + virCommandPtr cmd; + + if((path = virFindFileInPath("pm-is-supported")) == NULL) + return -1; + + cmd = virCommandNew(path); + switch(capability) { + case HOST_PM_S3: + /* Check support for suspend (S3) */ + virCommandAddArg(cmd, "--suspend"); + break; + + case HOST_PM_S4: + /* Check support for hibernation (S4) */ + virCommandAddArg(cmd, "--hibernate"); + break; + + default: + goto cleanup; + } + + if(virCommandRun(cmd, &status) < 0) + goto cleanup; + + /* Check return code of command == 0 for success */ + if(status == 0) + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(path); + return ret; +} + diff --git a/src/util/util.h b/src/util/util.h index af8b15d..c428eb4 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -272,4 +272,11 @@ bool virIsDevMapperDevice(const char *devname) ATTRIBUTE_NONNULL(1); int virEmitXMLWarning(int fd, const char *name, const char *cmd) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +/* Power Management Capabilities of the host system */ +# define HOST_PM_S3 1 /* Suspend-to-RAM */ +# define HOST_PM_S4 2 /* Suspend-to-Disk */ + +int virCheckPMCapability(int capability); + #endif /* __VIR_UTIL_H__ */
On 08/05/2011 05:54 AM, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
The script "pm-is-supported" (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag "<power_management>" is introduced in the XML under the<host> tag.
</cpu> <power_management> <<<=== New host power management features <S3/> <S4/> </power_management>
Nice.
However in case the host does not support any power management feature, then the XML will not contain the<power_management> tag.
Wouldn't it be better to include <power_management/> (ie. an empty tag) to explicitly document that power management capabilities were successfully probed but none found, and leave the case of omitted <power_management> to imply that no probe was done in the first place (either because libvirt predates this xml addition, or because pm-is-supported is not found)?
src/conf/capabilities.c | 34 +++++++++++++++++++++++++++ src/conf/capabilities.h | 7 ++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 10 ++++++++ src/util/util.c | 52 ++++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 7 ++++++ 6 files changed, 112 insertions(+), 0 deletions(-)
Incomplete - this also needs to touch docs/schemas/capability.rng to validate the new XML in the rng grammar, as well as docs/formatcaps.html.in to at least demonstrate the new XML in the red-shaded portion of the example (actually, that page really needs some TLC, because it is lacking lots of details about the available XML, but that's an independent project).
+/** + * virCapabilitiesAddHostPowerManagement: + * @caps: capabilities to extend + * @name: name of power management feature + * + * Registers a new host power management feature, eg: 'S3' or 'S4' + */ +int +virCapabilitiesAddHostPowerManagement(virCapsPtr caps, + const char *name) +{ + if(VIR_RESIZE_N(caps->host.powerMgmt, caps->host.npowerMgmt_max, + caps->host.npowerMgmt, 1)< 0) + return -1; + + if((caps->host.powerMgmt[caps->host.npowerMgmt] = strdup(name)) == NULL) + return -1;
This returns -1 without calling virReportOOMError(),...
@@ -686,6 +711,15 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&xml, "</cpu>\n");
+ if(caps->host.npowerMgmt) { + virBufferAddLit(&xml, "<power_management>\n"); + for (i = 0; i< caps->host.npowerMgmt ; i++) { + virBufferAsprintf(&xml, "<%s/>\n", + caps->host.powerMgmt[i]); + } + virBufferAddLit(&xml, "</power_management>\n"); + }
See my above thoughts - this should support an explicit <power_management/> when we were able to successfully probe but found no capabilities; which means an extra bool in the _virCapsHost struct.
}
+ /* Add the power management features of the host */ + + /* Check for Suspend-to-RAM support (S3) */ + if(virCheckPMCapability(HOST_PM_S3) == 0) + virCapabilitiesAddHostPowerManagement(caps, "S3");
...yet here, you aren't checking the return value for failure. That's a silent bug on OOM, which is not appropriate. One of the two places needs to call virReportOOMError(), and the caller must not discard failure.
+ + /* Check for Suspend-to-Disk support (S4) */ + if(virCheckPMCapability(HOST_PM_S4) == 0) + virCapabilitiesAddHostPowerManagement(caps, "S4");
You are manually converting between #define HOST_PM_S* and strings; it seems like it would be better to introduce an enum type and use the VIR_ENUM magic to make the translation automated, as well as more extensible in the future. And if you do that, then _virCapsHost can track an array of enum values instead of an array of strings, for a more compact internal representation.
+/** + * Check the Power Management Capabilities of the host system. + * The script 'pm-is-supported' (from the pm-utils package) is run + * to find out if the capability is supported by the host. + * + * @capability: capability to check for + * HOST_PM_S3: Check for Suspend-to-RAM support + * HOST_PM_S4: Check for Suspend-to-Disk support + * + * Returns 0 if supported, -1 if not supported.
I think this should be: 0 if query successful but unsupported, 1 if supported, and -1 if error (such as pm-is-supported not installed). The error can safely be ignored if the caller doesn't care, but having the tri-state return value will make it possible to later add any code that explicitly cares.
+ */ +int +virCheckPMCapability(int capability) +{ + + char *path = NULL; + int status = -1, ret = -1; + virCommandPtr cmd; + + if((path = virFindFileInPath("pm-is-supported")) == NULL) + return -1;
Should we update the libvirt.spec file to guarantee this dependency on Fedora installations? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/05/2011 09:21 PM, Eric Blake wrote:
On 08/05/2011 05:54 AM, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
The script "pm-is-supported" (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag "<power_management>" is introduced in the XML under the<host> tag.
</cpu> <power_management> <<<=== New host power management features <S3/> <S4/> </power_management>
Nice.
However in case the host does not support any power management feature, then the XML will not contain the<power_management> tag.
Wouldn't it be better to include <power_management/> (ie. an empty tag) to explicitly document that power management capabilities were successfully probed but none found, and leave the case of omitted <power_management> to imply that no probe was done in the first place (either because libvirt predates this xml addition, or because pm-is-supported is not found)?
src/conf/capabilities.c | 34 +++++++++++++++++++++++++++ src/conf/capabilities.h | 7 ++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 10 ++++++++ src/util/util.c | 52 ++++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 7 ++++++ 6 files changed, 112 insertions(+), 0 deletions(-)
Incomplete - this also needs to touch docs/schemas/capability.rng to validate the new XML in the rng grammar, as well as docs/formatcaps.html.in to at least demonstrate the new XML in the red-shaded portion of the example (actually, that page really needs some TLC, because it is lacking lots of details about the available XML, but that's an independent project).
+/** + * virCapabilitiesAddHostPowerManagement: + * @caps: capabilities to extend + * @name: name of power management feature + * + * Registers a new host power management feature, eg: 'S3' or 'S4' + */ +int +virCapabilitiesAddHostPowerManagement(virCapsPtr caps, + const char *name) +{ + if(VIR_RESIZE_N(caps->host.powerMgmt, caps->host.npowerMgmt_max, + caps->host.npowerMgmt, 1)< 0) + return -1; + + if((caps->host.powerMgmt[caps->host.npowerMgmt] = strdup(name)) == NULL) + return -1;
This returns -1 without calling virReportOOMError(),...
@@ -686,6 +711,15 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&xml, "</cpu>\n");
+ if(caps->host.npowerMgmt) { + virBufferAddLit(&xml, "<power_management>\n"); + for (i = 0; i< caps->host.npowerMgmt ; i++) { + virBufferAsprintf(&xml, "<%s/>\n", + caps->host.powerMgmt[i]); + } + virBufferAddLit(&xml, "</power_management>\n"); + }
See my above thoughts - this should support an explicit <power_management/> when we were able to successfully probe but found no capabilities; which means an extra bool in the _virCapsHost struct.
}
+ /* Add the power management features of the host */ + + /* Check for Suspend-to-RAM support (S3) */ + if(virCheckPMCapability(HOST_PM_S3) == 0) + virCapabilitiesAddHostPowerManagement(caps, "S3");
...yet here, you aren't checking the return value for failure. That's a silent bug on OOM, which is not appropriate. One of the two places needs to call virReportOOMError(), and the caller must not discard failure.
+ + /* Check for Suspend-to-Disk support (S4) */ + if(virCheckPMCapability(HOST_PM_S4) == 0) + virCapabilitiesAddHostPowerManagement(caps, "S4");
You are manually converting between #define HOST_PM_S* and strings; it seems like it would be better to introduce an enum type and use the VIR_ENUM magic to make the translation automated, as well as more extensible in the future. And if you do that, then _virCapsHost can track an array of enum values instead of an array of strings, for a more compact internal representation.
+/** + * Check the Power Management Capabilities of the host system. + * The script 'pm-is-supported' (from the pm-utils package) is run + * to find out if the capability is supported by the host. + * + * @capability: capability to check for + * HOST_PM_S3: Check for Suspend-to-RAM support + * HOST_PM_S4: Check for Suspend-to-Disk support + * + * Returns 0 if supported, -1 if not supported.
I think this should be:
0 if query successful but unsupported, 1 if supported, and -1 if error (such as pm-is-supported not installed). The error can safely be ignored if the caller doesn't care, but having the tri-state return value will make it possible to later add any code that explicitly cares.
+ */ +int +virCheckPMCapability(int capability) +{ + + char *path = NULL; + int status = -1, ret = -1; + virCommandPtr cmd; + + if((path = virFindFileInPath("pm-is-supported")) == NULL) + return -1;
Should we update the libvirt.spec file to guarantee this dependency on Fedora installations?
Hi, Thank you Eric for your kind review. I'll post the next iteration of the patch by incorporating most of the changes you have suggested. -- Regards, Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Linux Technology Center, IBM India Systems and Technology Lab
On Fri, Aug 05, 2011 at 05:24:13PM +0530, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
The script "pm-is-supported" (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag "<power_management>" is introduced in the XML under the <host> tag.
Eg: When the host supports both S3 and S4, the XML looks like this:
<capabilities>
<host> <uuid>dc699581-48a2-11cb-b8a8-9a0265a79bbe</uuid> <cpu> <arch>i686</arch> <model>coreduo</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> <feature name='xtpr'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> <feature name='pbe'/> <feature name='tm'/> <feature name='ht'/> <feature name='ss'/> <feature name='acpi'/> <feature name='ds'/> </cpu> <power_management> <<<=== New host power management features <S3/> <S4/> </power_management> <migration_features> <live/> <uri_transports> <uri_transport>tcp</uri_transport> </uri_transports> </migration_features> </host> . . .
However in case the host does not support any power management feature, then the XML will not contain the <power_management> tag.
The initial discussion about this patch was done in [1]. And the choice to name the new tag as "power_management" was discussed in [2].
Please let me know your comments and feedback.
Exposing info in the capabilities is great, if applications can actually do something with this info. There are no APIs in libvirt for controlling the host OS power management state, so I don't see what use this XML addition is on its own. ie, if an application using libvirt has to resort to spawning '/usr/sbin/pm-suspend' to actually do anything, then there's no real benefit to having this info in libvirt, so they have to go outside of libvirt anyway. So while the XML design/proposal may well be fine, until we actually have some host power management control APIs in libvirt, I'm inclined to NACK this addition to capabilities. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/05/2011 10:11 AM, Daniel P. Berrange wrote:
On Fri, Aug 05, 2011 at 05:24:13PM +0530, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
Exposing info in the capabilities is great, if applications can actually do something with this info. There are no APIs in libvirt for controlling the host OS power management state, so I don't see what use this XML addition is on its own. ie, if an application using libvirt has to resort to spawning '/usr/sbin/pm-suspend' to actually do anything, then there's no real benefit to having this info in libvirt, so they have to go outside of libvirt anyway.
So while the XML design/proposal may well be fine, until we actually have some host power management control APIs in libvirt, I'm inclined to NACK this addition to capabilities.
Does that mean that we need to add a new API: int virNodeSuspend(virConnectPtr conn, unsigned int flags) that returns 0 if the host will be suspended after a short delay (note that this must return before the suspend actually takes place, because after the suspend, the connection cannot return data until it resumes), and -1 where unsupported? Also, do we need to probe if the connection has a wake-on-lan capability or some other way to kick it back out of S3 or S4 when it is time to start using the node again? I tend to agree with Dan's assessment that until there is something in libvirt that can make use of this information, then exposing it through libvirt is pointless. That is, if the only way to make use of the information is to call other programs, and since the information was obtained by another program, you haven't cut any other programs out of the loop by exposing the capability through libvirt. But if libvirt itself can be remotely told to suspend a host, then you've removed the need for external programs, and libvirt would indeed need to expose whether it supports the new feature of suspending a host. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* Eric Blake <eblake@redhat.com> [2011-08-05 10:32:31]:
On 08/05/2011 10:11 AM, Daniel P. Berrange wrote:
On Fri, Aug 05, 2011 at 05:24:13PM +0530, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
Exposing info in the capabilities is great, if applications can actually do something with this info. There are no APIs in libvirt for controlling the host OS power management state, so I don't see what use this XML addition is on its own. ie, if an application using libvirt has to resort to spawning '/usr/sbin/pm-suspend' to actually do anything, then there's no real benefit to having this info in libvirt, so they have to go outside of libvirt anyway.
So while the XML design/proposal may well be fine, until we actually have some host power management control APIs in libvirt, I'm inclined to NACK this addition to capabilities.
Does that mean that we need to add a new API:
int virNodeSuspend(virConnectPtr conn, unsigned int flags)
that returns 0 if the host will be suspended after a short delay (note that this must return before the suspend actually takes place, because after the suspend, the connection cannot return data until it resumes), and -1 where unsupported? Also, do we need to probe if the connection has a wake-on-lan capability or some other way to kick it back out of S3 or S4 when it is time to start using the node again?
We can easily implement the suspend part, but will have little difficulty on the resume. On the suspend front, we need an asynchronous mechanism to trigger the suspend after say 5-10 seconds. Is there any other similar async libvrit API that will return success before completing the action? Next we will need a blocker flag indicating that the connection is suspended, or maybe just terminate the connection. For the resume, we have couple of methods: * Resume on wake-on-lan and send the magic packet or kick from another 'nearby' node * Have a timed sleep where we can wakeup on RTC alarm and check back if there is new work, sleep again after a timeout. * Explore other device based wakeup mechanism that can be easily controlled from a libvirt client.
I tend to agree with Dan's assessment that until there is something in libvirt that can make use of this information, then exposing it through libvirt is pointless. That is, if the only way to make use of the information is to call other programs, and since the information was obtained by another program, you haven't cut any other programs out of the loop by exposing the capability through libvirt. But if libvirt itself can be remotely told to suspend a host, then you've removed the need for external programs, and libvirt would indeed need to expose whether it supports the new feature of suspending a host.
Agreed, we would definitely like to have this feature exploited through libvirt, we can cleanly suspend without using an external program. --Vaidy
On Tue, Aug 09, 2011 at 11:25:28AM +0530, Vaidyanathan Srinivasan wrote:
* Eric Blake <eblake@redhat.com> [2011-08-05 10:32:31]:
On 08/05/2011 10:11 AM, Daniel P. Berrange wrote:
So while the XML design/proposal may well be fine, until we actually have some host power management control APIs in libvirt, I'm inclined to NACK this addition to capabilities.
I tend to agree
Does that mean that we need to add a new API:
int virNodeSuspend(virConnectPtr conn, unsigned int flags)
that returns 0 if the host will be suspended after a short delay (note that this must return before the suspend actually takes place, because after the suspend, the connection cannot return data until it resumes), and -1 where unsupported? Also, do we need to probe if the connection has a wake-on-lan capability or some other way to kick it back out of S3 or S4 when it is time to start using the node again?
We can easily implement the suspend part, but will have little difficulty on the resume.
On the suspend front, we need an asynchronous mechanism to trigger the suspend after say 5-10 seconds. Is there any other similar async libvrit API that will return success before completing the action?
Next we will need a blocker flag indicating that the connection is suspended, or maybe just terminate the connection.
For the resume, we have couple of methods:
* Resume on wake-on-lan and send the magic packet or kick from another 'nearby' node
* Have a timed sleep where we can wakeup on RTC alarm and check back if there is new work, sleep again after a timeout.
I wold try to implement something based on this. What I think is important is make sure we don't implement a NodeSuspend API if there is a risk of being trapped and not being able to wake up the node from the API. And that means that we must be able to make the resume from the node itself. An RTC timer is the best way to do this. But I think this requires a change of API. If we agree we can't rely on an external node wake up, and since the connection is likely to drop after the Node actually goes to sleep (either by explicit client close or because of the TCP/IP timeout), we can't expect a libvirt API like virNodeResume() to work. So I believe the API we need to to tell a node to go to sleep for a given duration, the call will fail if we fail to set up the RTC wakeup (and at libvirtd init time we should make sure the RTC does work). Maybe the host will wake up earlier for some reason, but the management layer knows that after that duration it should be able to reopen a connection to the Node , then it can decide to send it back to sleep if it still not needed, but at least from an API point of view this is complete.
Agreed, we would definitely like to have this feature exploited through libvirt, we can cleanly suspend without using an external program.
I'm fine with that as long as we are garanteed we can also resume without using an external program too. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
* Daniel P. Berrange <berrange@redhat.com> [2011-08-05 17:11:50]:
On Fri, Aug 05, 2011 at 05:24:13PM +0530, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
The script "pm-is-supported" (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag "<power_management>" is introduced in the XML under the <host> tag.
Eg: When the host supports both S3 and S4, the XML looks like this:
<capabilities>
<host> <uuid>dc699581-48a2-11cb-b8a8-9a0265a79bbe</uuid> <cpu> <arch>i686</arch> <model>coreduo</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> <feature name='xtpr'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> <feature name='pbe'/> <feature name='tm'/> <feature name='ht'/> <feature name='ss'/> <feature name='acpi'/> <feature name='ds'/> </cpu> <power_management> <<<=== New host power management features <S3/> <S4/> </power_management> <migration_features> <live/> <uri_transports> <uri_transport>tcp</uri_transport> </uri_transports> </migration_features> </host> . . .
However in case the host does not support any power management feature, then the XML will not contain the <power_management> tag.
The initial discussion about this patch was done in [1]. And the choice to name the new tag as "power_management" was discussed in [2].
Please let me know your comments and feedback.
Exposing info in the capabilities is great, if applications can actually do something with this info. There are no APIs in libvirt for controlling the host OS power management state, so I don't see what use this XML addition is on its own. ie, if an application using libvirt has to resort to spawning '/usr/sbin/pm-suspend' to actually do anything, then there's no real benefit to having this info in libvirt, so they have to go outside of libvirt anyway.
So while the XML design/proposal may well be fine, until we actually have some host power management control APIs in libvirt, I'm inclined to NACK this addition to capabilities.
As we discussed in the previous RFC, exporting the capability is only the first step to allow systems management software to easily discover the capability subject to platform policies. Actual invocation of S3/S4 currently has to be outside of libvirt because wakeup involves some sort of out-of-band event. The management software is expected to handle the state transitions until we could device an in-band method through libvirt. However the discovery and policy implications can be managed by the libvirt infrastructure instead of building another framework. Similar to S3/S4 capability, other power management features like cpufreq/cpuidle polices and power-performance bias can be exported and controlled by libvirt through a similar mechanism with a good policy management framework. This features enables optimizations that are implemented above libvirt layer and opens up opportunity to discover and use platform power management features. --Vaidy
On Fri, Aug 05, 2011 at 11:52:07PM +0530, Vaidyanathan Srinivasan wrote:
* Daniel P. Berrange <berrange@redhat.com> [2011-08-05 17:11:50]:
On Fri, Aug 05, 2011 at 05:24:13PM +0530, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
The script "pm-is-supported" (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag "<power_management>" is introduced in the XML under the <host> tag.
Eg: When the host supports both S3 and S4, the XML looks like this:
<capabilities>
<host> <uuid>dc699581-48a2-11cb-b8a8-9a0265a79bbe</uuid> <cpu> <arch>i686</arch> <model>coreduo</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> <feature name='xtpr'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> <feature name='pbe'/> <feature name='tm'/> <feature name='ht'/> <feature name='ss'/> <feature name='acpi'/> <feature name='ds'/> </cpu> <power_management> <<<=== New host power management features <S3/> <S4/> </power_management> <migration_features> <live/> <uri_transports> <uri_transport>tcp</uri_transport> </uri_transports> </migration_features> </host> . . .
However in case the host does not support any power management feature, then the XML will not contain the <power_management> tag.
The initial discussion about this patch was done in [1]. And the choice to name the new tag as "power_management" was discussed in [2].
Please let me know your comments and feedback.
Exposing info in the capabilities is great, if applications can actually do something with this info. There are no APIs in libvirt for controlling the host OS power management state, so I don't see what use this XML addition is on its own. ie, if an application using libvirt has to resort to spawning '/usr/sbin/pm-suspend' to actually do anything, then there's no real benefit to having this info in libvirt, so they have to go outside of libvirt anyway.
So while the XML design/proposal may well be fine, until we actually have some host power management control APIs in libvirt, I'm inclined to NACK this addition to capabilities.
As we discussed in the previous RFC, exporting the capability is only the first step to allow systems management software to easily discover the capability subject to platform policies. Actual invocation of S3/S4 currently has to be outside of libvirt because wakeup involves some sort of out-of-band event. The management software is expected to handle the state transitions until we could device an in-band method through libvirt. However the discovery and policy implications can be managed by the libvirt infrastructure instead of building another framework.
Similar to S3/S4 capability, other power management features like cpufreq/cpuidle polices and power-performance bias can be exported and controlled by libvirt through a similar mechanism with a good policy management framework.
This features enables optimizations that are implemented above libvirt layer and opens up opportunity to discover and use platform power management features.
I really don't see that adding this to the capabilities enables any feature at all. As mentioned before, if you have to go outside of libvirt to control this feature, then having discovery in libvirt has near zero benefit. Adding the capability info as the first step is really the wrong way around. Only once we actually have some functionality related to this present in the libvirt API, is there any need to have discovery in libvirt. We don't want libvirt to be a general dumping ground for discovery of arbitrary host OS functionality, which is not controllable via libvirt. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
* Daniel P. Berrange <berrange@redhat.com> [2011-08-08 16:38:48]:
On Fri, Aug 05, 2011 at 11:52:07PM +0530, Vaidyanathan Srinivasan wrote:
* Daniel P. Berrange <berrange@redhat.com> [2011-08-05 17:11:50]:
On Fri, Aug 05, 2011 at 05:24:13PM +0530, Srivatsa S. Bhat wrote:
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host.
The script "pm-is-supported" (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag "<power_management>" is introduced in the XML under the <host> tag.
Eg: When the host supports both S3 and S4, the XML looks like this:
<capabilities>
<host> <uuid>dc699581-48a2-11cb-b8a8-9a0265a79bbe</uuid> <cpu> <arch>i686</arch> <model>coreduo</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> <feature name='xtpr'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> <feature name='pbe'/> <feature name='tm'/> <feature name='ht'/> <feature name='ss'/> <feature name='acpi'/> <feature name='ds'/> </cpu> <power_management> <<<=== New host power management features <S3/> <S4/> </power_management> <migration_features> <live/> <uri_transports> <uri_transport>tcp</uri_transport> </uri_transports> </migration_features> </host> . . .
However in case the host does not support any power management feature, then the XML will not contain the <power_management> tag.
The initial discussion about this patch was done in [1]. And the choice to name the new tag as "power_management" was discussed in [2].
Please let me know your comments and feedback.
Exposing info in the capabilities is great, if applications can actually do something with this info. There are no APIs in libvirt for controlling the host OS power management state, so I don't see what use this XML addition is on its own. ie, if an application using libvirt has to resort to spawning '/usr/sbin/pm-suspend' to actually do anything, then there's no real benefit to having this info in libvirt, so they have to go outside of libvirt anyway.
So while the XML design/proposal may well be fine, until we actually have some host power management control APIs in libvirt, I'm inclined to NACK this addition to capabilities.
As we discussed in the previous RFC, exporting the capability is only the first step to allow systems management software to easily discover the capability subject to platform policies. Actual invocation of S3/S4 currently has to be outside of libvirt because wakeup involves some sort of out-of-band event. The management software is expected to handle the state transitions until we could device an in-band method through libvirt. However the discovery and policy implications can be managed by the libvirt infrastructure instead of building another framework.
Similar to S3/S4 capability, other power management features like cpufreq/cpuidle polices and power-performance bias can be exported and controlled by libvirt through a similar mechanism with a good policy management framework.
This features enables optimizations that are implemented above libvirt layer and opens up opportunity to discover and use platform power management features.
I really don't see that adding this to the capabilities enables any feature at all. As mentioned before, if you have to go outside of libvirt to control this feature, then having discovery in libvirt has near zero benefit. Adding the capability info as the first step is really the wrong way around. Only once we actually have some functionality related to this present in the libvirt API, is there any need to have discovery in libvirt. We don't want libvirt to be a general dumping ground for discovery of arbitrary host OS functionality, which is not controllable via libvirt.
Fair point. As Eric also mentioned lets work out the mechanism to invoke S3/S4 on host. I am not opposed to designing the API for invocation through libvirt, it will only help exploitation of this power saving feature much better. Lets continue discussion on Eric API suggestion. --Vaidy
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Srivatsa S. Bhat -
Vaidyanathan Srinivasan