[PATCH 3/3] hyperv: Implement domainSnapshotCreateXML()

Jonathon Jongsma via Devel posted 3 patches 4 days, 4 hours ago
[PATCH 3/3] hyperv: Implement domainSnapshotCreateXML()
Posted by Jonathon Jongsma via Devel 4 days, 4 hours ago
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/hyperv/hyperv_driver.c |  82 ++++++++++++++++++++++
 src/hyperv/hyperv_wmi.c    | 139 +++++++++++++++++++++++++++++++++++++
 src/hyperv/hyperv_wmi.h    |   6 ++
 3 files changed, 227 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 841d9ccaa5..12af6bd337 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -4163,6 +4163,87 @@ hypervDomainSnapshotLookupByName(virDomainPtr domain,
 }
 
 
+static virDomainSnapshotPtr
+hypervDomainSnapshotCreateXML(virDomainPtr domain,
+                              const char *xmlDesc,
+                              unsigned int flags)
+{
+    hypervPrivate *priv = domain->conn->privateData;
+    g_autoptr(Msvm_ComputerSystem) computerSystem = NULL;
+    g_autoptr(hypervInvokeParamsList) params = NULL;
+    g_autoptr(virDomainSnapshotDef) def = NULL;
+    g_autoptr(GHashTable) snapshotSettings = NULL;
+    g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
+    g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER;
+    g_autoptr(Msvm_VirtualSystemSettingData) snapshot = NULL;
+    g_auto(WsXmlDocH) response = NULL;
+    char uuid_string[VIR_UUID_STRING_BUFLEN];
+
+    virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, NULL);
+
+    def = virDomainSnapshotDefParseString(xmlDesc, priv->xmlopt, NULL, NULL, 0);
+    if (!def)
+        return NULL;
+
+    if (def->ndisks) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("disk snapshots not yet supported for hyperv"));
+        return NULL;
+    }
+
+    virUUIDFormat(domain->uuid, uuid_string);
+
+    if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0)
+        return NULL;
+
+    virBufferEscapeSQL(&eprQuery,
+                       MSVM_COMPUTERSYSTEM_WQL_SELECT "WHERE Name = '%s'",
+                       uuid_string);
+
+    params = hypervCreateInvokeParamsList("CreateSnapshot",
+                                          MSVM_VIRTUALSYSTEMSNAPSHOTSERVICE_SELECTOR,
+                                          Msvm_VirtualSystemSnapshotService_WmiInfo);
+    if (!params)
+        return NULL;
+
+    if (hypervAddEprParam(params, "AffectedSystem", &eprQuery,
+                          Msvm_ComputerSystem_WmiInfo) < 0)
+        return NULL;
+
+    snapshotSettings = hypervCreateEmbeddedParam(Msvm_VirtualSystemSnapshotSettingData_WmiInfo);
+    if (!snapshotSettings)
+        return NULL;
+
+    if (hypervSetEmbeddedProperty(snapshotSettings, "ConsistencyLevel",
+                                  (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) ?
+                                  HYPERV_SNAPSHOT_CONSISTENCY_APP_CONSISTENT :
+                                  HYPERV_SNAPSHOT_CONSISTENCY_CRASH_CONSISTENT) < 0)
+        return NULL;
+
+    if (hypervAddEmbeddedParam(params, "SnapshotSettings", &snapshotSettings,
+                               Msvm_VirtualSystemSnapshotSettingData_WmiInfo) < 0)
+        return NULL;
+
+    hypervAddSimpleParam(params, "SnapshotType", HYPERV_SNAPSHOT_TYPE_FULL);
+
+    if (hypervInvokeMethod(priv, &params, &response) < 0)
+        return NULL;
+
+    /* The CreateSnapshot method will generally return an async job rather
+     * than returning the 'ResultingSnapshot' output parameter directly
+     * in the response. Although hypervInvokeMethod() polls the async job to
+     * completion when this occurs, the response will not include the output
+     * parameters. So we use this helper function to build a query to fetch the
+     * resulting object from the async job associations. */
+    if (hypervResponseGetOutputParam(priv, response, "ResultingSnapshot",
+                                  Msvm_VirtualSystemSettingData_WmiInfo,
+                                  (hypervObject**) &snapshot) < 0)
+        return NULL;
+
+    return virGetDomainSnapshot(domain, snapshot->data->InstanceID);
+}
+
+
 static int
 hypervDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                            unsigned int flags)
@@ -4493,6 +4574,7 @@ static virHypervisorDriver hypervHypervisorDriver = {
     .domainSnapshotCurrent = hypervDomainSnapshotCurrent, /* 12.2.0 */
     .domainSnapshotGetParent = hypervDomainSnapshotGetParent, /* 12.2.0 */
     .domainSnapshotDelete = hypervDomainSnapshotDelete, /* 12.2.0 */
+    .domainSnapshotCreateXML = hypervDomainSnapshotCreateXML, /* 12.2.0 */
 };
 
 
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index dab7abe8cf..9809eac1e5 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -867,6 +867,145 @@ hypervInvokeMethod(hypervPrivate *priv,
     return 0;
 }
 
+static char*
+hypervOutputParamReferenceId(WsXmlNodeH node)
+{
+    WsXmlNodeH selector_set = NULL;
+    WsXmlNodeH selector = NULL;
+    int i = 0;
+
+    if (node)
+        selector_set = ws_xml_find_in_tree(node, XML_NS_WS_MAN, "SelectorSet", TRUE);
+
+    if (selector_set) {
+        for (i = 0; (selector = ws_xml_get_child(selector_set, i, XML_NS_WS_MAN, "Selector")) != NULL; i++) {
+            if (STREQ_NULLABLE(ws_xml_find_attr_value(selector, NULL, "Name"), "InstanceID")) {
+                return ws_xml_get_node_text(selector);
+            }
+        }
+    }
+    return NULL;
+}
+
+
+/*
+ * hypervResponseGetOutputParam()
+ *
+ * Extracts an output parameter from a WMI method response and retrieves the
+ * referenced object.
+ *
+ * Handles both synchronous and asynchronous cases. When a method is
+ * synchronous, the parameter will be directly present in the response document.
+ * When the method returns asynchronously, we need to fetch the result parameter
+ * via its associations with the Msvm_ConcreteJob object referenced in the
+ * response document.
+ *
+ * Parameters:
+ *   @priv: hypervPrivate object associated with the connection
+ *   response: The WMI method response document
+ *   paramName: The output parameter name (e.g., "ResultingSnapshot")
+ *   paramClass: The WMI class info for the expected result type
+ *   outParam: return location for the named output parameter
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+int
+hypervResponseGetOutputParam(hypervPrivate *priv,
+                             WsXmlDocH response,
+                             const char *paramName,
+                             hypervWmiClassInfo *paramClassInfo,
+                             hypervObject **outParam)
+{
+    g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER;
+    WsXmlNodeH body = NULL;
+    WsXmlNodeH output_node = NULL;
+    char *output_name = NULL;
+    char *provider_ns = NULL;
+    const char *rv_str = NULL;
+    int return_code;
+    int i = 0;
+
+    body = ws_xml_get_soap_body(response);
+    if (!body) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not find SOAP Body in response"));
+        return -1;
+    }
+
+    /* Find the $(METHOD)_OUTPUT node in the SOAP Body */
+    for (i = 0; (output_node = ws_xml_get_child(body, i, NULL, NULL)) != NULL; i++) {
+        output_name = ws_xml_get_node_local_name(output_node);
+        if (output_name && g_str_has_suffix(output_name, "_OUTPUT"))
+            break;
+        output_node = NULL;
+    }
+
+    if (!output_node) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not find _OUTPUT node in method response"));
+        return -1;
+    }
+
+    provider_ns = ws_xml_get_node_name_ns(output_node);
+
+    rv_str = ws_xml_get_node_text(ws_xml_get_child(output_node, 0, provider_ns, "ReturnValue"));
+    if (!rv_str || virStrToLong_i(rv_str, NULL, 10, &return_code) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not get ReturnValue from method response"));
+        return -1;
+    }
+
+    if (return_code == CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) {
+        /* if this was a synchronous response, the output parameter should contain
+         * the id of an object, so we can simply look it up by its instance ID */
+        WsXmlNodeH param_node = ws_xml_get_child(output_node, 0, provider_ns, paramName);
+        const char *out_param_id = NULL;
+
+        if (param_node)
+            out_param_id = hypervOutputParamReferenceId(param_node);
+
+        if (!out_param_id) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Could not find output parameter '%1$s' in response"),
+                           paramName);
+            return -1;
+        }
+        VIR_DEBUG("Method response was synchronous: %1$s = %2$s", paramName, out_param_id);
+        virBufferAsprintf(&query, "SELECT * FROM %s ", paramClassInfo->name);
+        virBufferEscapeSQL(&query, "WHERE InstanceID='%s'", out_param_id);
+    } else if (return_code == CIM_RETURNCODE_TRANSITION_STARTED) {
+        /* if this was an asynchronous response, we have to get the output
+         * parameter from its association with the async job */
+        WsXmlNodeH job_node = ws_xml_get_child(output_node, 0, provider_ns, "Job");
+        const char *job_id = NULL;
+
+        if (job_node)
+            job_id = hypervOutputParamReferenceId(job_node);
+
+        if (!job_id) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Could not find Job ID in async method response"));
+            return -1;
+        }
+        VIR_DEBUG("Method response was asynchronous. Job ID = %1$s", job_id);
+        virBufferEscapeSQL(&query,
+            "ASSOCIATORS OF {Msvm_ConcreteJob.InstanceID='%s'} ",
+            job_id);
+        virBufferAsprintf(&query,
+            "WHERE AssocClass = Msvm_AffectedJobElement "
+            "ResultClass = %s",
+            paramClassInfo->name);
+    } else {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unexpected return code %1$d in method response"),
+                       return_code);
+        return -1;
+    }
+
+    hypervGetWmiClassList(priv, paramClassInfo, &query, outParam);
+    return 0;
+}
+
 /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * Object
  */
diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h
index 0560f83e0d..36ca85a592 100644
--- a/src/hyperv/hyperv_wmi.h
+++ b/src/hyperv/hyperv_wmi.h
@@ -166,6 +166,12 @@ int hypervInvokeMethod(hypervPrivate *priv,
                        hypervInvokeParamsList **paramsPtr,
                        WsXmlDocH *res);
 
+int hypervResponseGetOutputParam(hypervPrivate *priv,
+                                 WsXmlDocH response,
+                                 const char *paramName,
+                                 hypervWmiClassInfo *paramClassInfo,
+                                 hypervObject **outParam);
+
 /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * CIM/Msvm_ReturnCode
  */
-- 
2.53.0
Re: [PATCH 3/3] hyperv: Implement domainSnapshotCreateXML()
Posted by Peter Krempa via Devel 14 hours ago
On Thu, Mar 19, 2026 at 13:39:50 -0500, Jonathon Jongsma via Devel wrote:

Please mention at least the limitations of the implementation in the
commit message.


> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
>  src/hyperv/hyperv_driver.c |  82 ++++++++++++++++++++++
>  src/hyperv/hyperv_wmi.c    | 139 +++++++++++++++++++++++++++++++++++++
>  src/hyperv/hyperv_wmi.h    |   6 ++
>  3 files changed, 227 insertions(+)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 841d9ccaa5..12af6bd337 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -4163,6 +4163,87 @@ hypervDomainSnapshotLookupByName(virDomainPtr domain,
>  }
>  
>  
> +static virDomainSnapshotPtr
> +hypervDomainSnapshotCreateXML(virDomainPtr domain,
> +                              const char *xmlDesc,
> +                              unsigned int flags)
> +{
> +    hypervPrivate *priv = domain->conn->privateData;
> +    g_autoptr(Msvm_ComputerSystem) computerSystem = NULL;
> +    g_autoptr(hypervInvokeParamsList) params = NULL;
> +    g_autoptr(virDomainSnapshotDef) def = NULL;
> +    g_autoptr(GHashTable) snapshotSettings = NULL;
> +    g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
> +    g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER;
> +    g_autoptr(Msvm_VirtualSystemSettingData) snapshot = NULL;
> +    g_auto(WsXmlDocH) response = NULL;
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +
> +    virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, NULL);
> +
> +    def = virDomainSnapshotDefParseString(xmlDesc, priv->xmlopt, NULL, NULL, 0);
> +    if (!def)
> +        return NULL;
> +
> +    if (def->ndisks) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("disk snapshots not yet supported for hyperv"));

The message makes it sound like snapshotting disks is not supported,
while what this check does is to prevent only configuring what to
snapshot.

If you don't want to allow configure what to snapshot, you also need to
include rejection of the memory snapshot configuration because that
doesn't seem to be configurable either.


> +        return NULL;
> +    }
> +
> +    virUUIDFormat(domain->uuid, uuid_string);
> +
> +    if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0)
> +        return NULL;

[...]

> @@ -4493,6 +4574,7 @@ static virHypervisorDriver hypervHypervisorDriver = {
>      .domainSnapshotCurrent = hypervDomainSnapshotCurrent, /* 12.2.0 */
>      .domainSnapshotGetParent = hypervDomainSnapshotGetParent, /* 12.2.0 */
>      .domainSnapshotDelete = hypervDomainSnapshotDelete, /* 12.2.0 */
> +    .domainSnapshotCreateXML = hypervDomainSnapshotCreateXML, /* 12.2.0 */
>  };
>  
>  
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index dab7abe8cf..9809eac1e5 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -867,6 +867,145 @@ hypervInvokeMethod(hypervPrivate *priv,
>      return 0;
>  }
>  
> +static char*
> +hypervOutputParamReferenceId(WsXmlNodeH node)
> +{
> +    WsXmlNodeH selector_set = NULL;
> +    WsXmlNodeH selector = NULL;
> +    int i = 0;
> +
> +    if (node)
> +        selector_set = ws_xml_find_in_tree(node, XML_NS_WS_MAN, "SelectorSet", TRUE);
> +
> +    if (selector_set) {
> +        for (i = 0; (selector = ws_xml_get_child(selector_set, i, XML_NS_WS_MAN, "Selector")) != NULL; i++) {
> +            if (STREQ_NULLABLE(ws_xml_find_attr_value(selector, NULL, "Name"), "InstanceID")) {
> +                return ws_xml_get_node_text(selector);
> +            }
> +        }
> +    }
> +    return NULL;
> +}
> +
> +
> +/*
> + * hypervResponseGetOutputParam()
> + *
> + * Extracts an output parameter from a WMI method response and retrieves the
> + * referenced object.
> + *
> + * Handles both synchronous and asynchronous cases. When a method is
> + * synchronous, the parameter will be directly present in the response document.
> + * When the method returns asynchronously, we need to fetch the result parameter
> + * via its associations with the Msvm_ConcreteJob object referenced in the
> + * response document.

This doesn't seem to be entirely related to snapshots and even if yes it
seems to be more of an infrastructure improvement rather than directly
tied to the implemnetation. Thus it really looks like material for a
separate patch.