[libvirt] [PATCHv2 4/5] hyperv: Add basic driver for Microsoft Hyper-V

[Eric Blake]

On 08/03/2011 09:00 AM, Matthias Bolte wrote:
> Domain listing, basic information retrieval and domain life cycle
> management is implemented. But currently the domian XML output

s/domian/domain/

> lacks the complete devices section.
>
> The driver uses OpenWSMAN to directly communicate with an Hyper-V

s/an Hyper/a Hyper/

Since the H in Hyper is aspirated, we use 'a' instead of 'an'.  Blame 
English for being stupid.

> server over its WS-Management interface exposed via Microsoft WinRM.
>
> The driver is based on the work of Michael Sievers. This started in
> the same master program project group at the University of Paderborn
> as the ESX driver.
>
> See Michael's blog for details: http://hyperv4libvirt.wordpress.com/
> ---
>
> v2:
> - add more virCheckFlags to please recent syntax-check additions for flags

Yay - my syntax-check was worth something!

> +
> +    /* Check if the connection can be established and if the server has the
> +     * Hyper-V role installed. If the call to hypervGetMsvmComputerSystemList
> +     * succeeds than the connection has be established. If the returned list

s/has be/has been/

> +    /* Strip the string to fit more relevant information in 32 chars */
> +    tmp = processorList->data->Name;
> +
> +    while (*tmp != '\0') {
> +        if (STRPREFIX(tmp, "  ")) {
> +            memmove(tmp, tmp + 1, strlen(tmp + 1) + 1);
> +            continue;
> +        } else if (STRPREFIX(tmp, "(R)") || STRPREFIX(tmp, "(C)")) {
> +            memmove(tmp, tmp + 3, strlen(tmp + 3) + 1);
> +            continue;
> +        } else if (STRPREFIX(tmp, "(TM)")) {

Cute.  Hopefully no one complains about stripping copyright and 
trademark notations in our compressed strings.  I think you're okay, but 
IANAL.

> +
> +static int
> +hypervDomainDestroy(virDomainPtr domain)

Let's implement hypervDomainDestroyFlags while we're at it (you don't 
have to support any flags for now).

> +
> +static int
> +hypervDomainGetState(virDomainPtr domain, int *state, int *reason,
> +                     unsigned int flags)
> +{
> +    int result = -1;
> +    hypervPrivate *priv = domain->conn->privateData;
> +    Msvm_ComputerSystem *computerSystem = NULL;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (hypervMsvmComputerSystemFromDomain(domain,&computerSystem)<  0) {
> +        goto cleanup;
> +    }
> +
> +    *state = hypervMsvmComputerSystemEnabledStateToDomainState(computerSystem);
> +
> +    if (reason != NULL) {
> +        *reason = 0;
> +    }

Looked like you might have enough details for some population of 
*reason, but it can be saved for later patches.

> +
> +static char *
> +hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
> +{
> +    char *xml = NULL;
> +    hypervPrivate *priv = domain->conn->privateData;
> +    virDomainDefPtr def = NULL;
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    Msvm_ComputerSystem *computerSystem = NULL;
> +    Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL;
> +    Msvm_ProcessorSettingData *processorSettingData = NULL;
> +    Msvm_MemorySettingData *memorySettingData = NULL;
> +

My convention in the other drivers was to put a comment here saying why 
I didn't do a virCheckFlags - because virDomainDefFormat does instead.

> +
> +static int
> +hypervDomainCreate(virDomainPtr domain)

Again, implement hypervDomainCreateWithFlags (you can hard-code flags to 
0), and make hypervDomainCreate call into the more powerful interface.

> +
> +
> +static int
> +hypervDomainManagedSave(virDomainPtr domain, unsigned int flags)
> +{
> +    int result = -1;
> +    hypervPrivate *priv = domain->conn->privateData;
> +    Msvm_ComputerSystem *computerSystem = NULL;
> +    bool in_transition = false;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (hypervMsvmComputerSystemFromDomain(domain,&computerSystem)<  0) {
> +        goto cleanup;
> +    }
> +
> +    if (!hypervIsMsvmComputerSystemActive(computerSystem,&in_transition) ||
> +        in_transition) {
> +        HYPERV_ERROR(VIR_ERR_OPERATION_INVALID, "%s",
> +                     _("Domain is not active or is in state transition"));
> +        goto cleanup;
> +    }
> +
> +    result = hypervInvokeMsvmComputerSystemRequestStateChange
> +               (domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_SUSPENDED);

Ah - now I see - HyperV suspended corresponds to a managed save (the 
hypervisor process exits, and resuming starts a new process), while 
paused corresponds to vcpus stopped (the hypervisor process exists but 
does nothing).  You were correct in mapping HyperV suspended to shut off.

Conditional ACK - I pointed out a couple more flag-variant functions 
that are trivially implemented.  I'm okay if you post a delta patch for 
review, then squash it in before pushing, since that would be shorter 
than a full-blown v3.

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