[libvirt] [PATCHv2 3/5] hyperv: Add OpenWSMAN based client for the Hyper-V WMI API

[Matthias Bolte]

2011/8/20 Eric Blake <eblake at redhat.com>:
> On 08/03/2011 09:00 AM, Matthias Bolte wrote:
>>
>> Add a generator script to generate the structs and serialization
>> information for OpenWSMAN.
>>
>> openwsman.h collects workarounds for problems in OpenWSMAN<= 2.2.6.
>> There are also disabled sections that would use ws_serializer_free_mem
>> but can't because it's broken in OpenWSMAN<= 2.2.6. Patches to fix
>> this have been posted upstream.
>> ---
>>
>> v2:
>> - no changes

>> +
>> +
>> +int
>> +hyperyVerifyResponse(WsManClient *client, WsXmlDocH response,
>> +                     const char *detail)
>> +{
>> +    int lastError = wsmc_get_last_error(client);
>> +    int responseCode = wsmc_get_response_code(client);
>> +    WsManFault *fault;
>> +
>> +    if (lastError != WS_LASTERR_OK) {
>> +        HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR,
>> +                     _("Transport error during %s: %s (%d)"),
>> +                     detail,
>> wsman_transport_get_last_error_string(lastError),
>> +                     lastError);
>> +        return -1;
>> +    }
>> +
>> +    if (responseCode != 200&&  responseCode != 400&&  responseCode !=
>> 500) {
>
> Magic numbers.  Should these have names?

As there are used only once here, I'll add a comment, instead of an
enum for now.

>> +int
>> +hypervMsvmComputerSystemEnabledStateToDomainState
>> +  (Msvm_ComputerSystem *computerSystem)
>> +{
>> +    switch (computerSystem->data->EnabledState) {
>> +      case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_UNKNOWN:
>> +        return VIR_DOMAIN_NOSTATE;
>> +
>> +      case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_ENABLED:
>> +        return VIR_DOMAIN_RUNNING;
>> +
>> +      case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_DISABLED:
>> +        return VIR_DOMAIN_SHUTOFF;
>> +
>> +      case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_PAUSED:
>> +        return VIR_DOMAIN_PAUSED;
>> +
>> +      case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED:
>> +        return VIR_DOMAIN_SHUTOFF;
>
> Is suspended really more like shutoff or paused?  What's the distinction
> being used by hyperv for these states?

Suspended maps to libvirt's managed save.

>> +++ b/src/hyperv/hyperv_wmi_generator.input
>> @@ -0,0 +1,294 @@
>> +#
>> +# Definitions of WMI classes used as input for the
>> hyperv_wmi_generator.py
>> +# script.
>> +#
>> +# This format is line-based, so end-of-line is important.
>> +#
>> +#
>> +# Class definition:
>> +#
>> +# class<name>
>> +#<type>  <name>
>> +#     ...
>> +# end
>> +#
>> +# Allowed values for<type>  are: boolean, string, datetime, int8, int16,
>> +# int32, int64, uint8, uint16, uint32 and uint64
>> +#
>> +# The property<name>  can be followed by [] to define a dynamic array.
>> +#
>> +
>> +
>> +class Msvm_ComputerSystem
>> +    string   Caption
>
> I'm trusting that these definitions match a documented layout somewhere.  Is
> there a reference URL worth embedding as a comment here, in case we need to
> refer back to the original struct definition that this copies from?

They are all documented in MSDN, I'll add a link to it.

>> +++ b/src/hyperv/hyperv_wmi_generator.py
>> @@ -0,0 +1,309 @@
>> +#!/usr/bin/env python
>
> My python's weak, but I did glance through all of the generated files, and
> they look pretty sane, so I'm acking on the basis of the output.
>
>> +def main():
>> +    if "srcdir" in os.environ:
>> +        input_filename = os.path.join(os.environ["srcdir"],
>> "hyperv/hyperv_wmi_generator.input")
>> +        output_dirname = os.path.join(os.environ["srcdir"], "hyperv")
>> +    else:
>> +        input_filename = os.path.join(os.getcwd(),
>> "hyperv_wmi_generator.input")
>> +        output_dirname = os.getcwd()
>
> I'm hoping this plays well with VPATH builds (so far, I've only tested
> in-tree builds).

This generator is based on the ESX one, therefore it should work in
VPATH builds, otherwise you should have seen problems with the ESX
generator in VPATH builds in the past.

Also the else case with getcwd will only be executed when the
generator is run manually, as the Makefile always passes srcdir to it.

>> +#ifndef __OPENWSMAN_H__
>> +# define __OPENWSMAN_H__
>> +
>> +/* Workaround openwsman<= 2.2.6 unconditionally defining optarg. Just
>> pretend
>> + * that u/os.h was already included. Need to explicitly include time.h
>> because
>> + * wsman-xml-serializer.h needs it and u/os.h would have included it. */
>> +# include<time.h>
>> +# define _LIBU_OS_H_
>> +# include<wsman-api.h>
>
> Oh the gross things we have to do.  The comment is worth its weight in gold.

I'm currently in the process of getting some patches into openwsman
upstream to fix those problems.

> I pointed out a couple of nits, but nothing jumped out at me as a
> showstopper.  ACK, and I'm happy if you push without posting a v3.

Here's the interdiff I'm going to apply, before I push the whole series.

-- 
Matthias Bolte
http://photron.blogspot.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003_interdiff.patch
Type: text/x-diff
Size: 5140 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110826/c1457171/attachment-0001.bin>