[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]