On 11/29/2011 08:44 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> If we ensure that virNodeSuspendGetTargetMask always resets
> *bitmask to zero upon failure, there is no need for the
> powerMgmt_valid field.
>
> * src/util/virnodesuspend.c: Ensure *bitmask is zero upon
> failure
> * src/conf/capabilities.c, src/conf/capabilities.h: Remove
> powerMgmt_valid field
> * src/qemu/qemu_capabilities.c: Remove powerMgmt_valid
I'm not sure about this one. This is a semantic change in the resulting
XML.
> - 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) {
> - int bit = ffs(pm) - 1;
> - virBufferAsprintf(&xml, " <%s/>\n",
> - virCapsHostPMTargetTypeToString(bit));
> - pm &= ~(1U << bit);
> - }
> - virBufferAddLit(&xml, " </power_management>\n");
> - } else {
> - /* The host does not support any PM feature. */
> - virBufferAddLit(&xml, " <power_management/>\n");
Before, we had three cases (and documented them!):
no <power_management> in the XML - we could not query (or older server)
explicit <power_management/> - we successfully queried, but lack power
management
explicit <power_management> with sub-elements - what features are supported
> + /* 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) {
> + int bit = ffs(pm) - 1;
> + virBufferAsprintf(&xml, " <%s/>\n",
> + virCapsHostPMTargetTypeToString(bit));
> + pm &= ~(1U << bit);
> }
> + virBufferAddLit(&xml, " </power_management>\n");
> + } else {
> + /* The host does not support any PM feature. */
> + virBufferAddLit(&xml, " <power_management/>\n");
Whereas after the patch, we now _always_ output a form of
<power_management>, thus collapsing 3 states into 2.
On the other hand, what is a user going to do about the difference
between the first two? There's nothing useful they can do by knowing
whether a host lacks power management or whether libvirt lacked the
ability to query power management; either way, it means they can't use
power management API.
So I'm 70-30 in favor of this patch.
At any rate, ACK to the code, but you also need to update
docs/formatcaps.html.in to match, if we agree to go with it.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list