[libvirt] [PATCH 10/14] Remove powerMgmt_valid field from capabilities struct

[Eric Blake]

On 11/29/2011 08:44 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> 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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111129/5d2c059c/attachment-0001.sig>