6:03 p.m.
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
po/POTFILES.in | 1 +
src/Makefile.am | 25 ++-
src/hyperv/.gitignore | 1 +
Can we move this to the top-level .gitignore, instead of adding yet one
more file?
src/hyperv/hyperv_private.h | 7 +
src/hyperv/hyperv_wmi.c | 684 +++++++++++++++++++++++++++++++++
src/hyperv/hyperv_wmi.h | 121 ++++++
src/hyperv/hyperv_wmi_classes.c | 37 ++
src/hyperv/hyperv_wmi_classes.h | 94 +++++
src/hyperv/hyperv_wmi_generator.input | 294 ++++++++++++++
src/hyperv/hyperv_wmi_generator.py | 309 +++++++++++++++
src/hyperv/openwsman.h | 47 +++
Good - it looks like these are all hand-maintained, and that you left
the generated files out of git.
@@ -843,6 +859,12 @@ libvirt_driver_esx_la_DEPENDENCIES =
$(ESX_DRIVER_GENERATED)
endif
+BUILT_SOURCES += $(HYPERV_DRIVER_GENERATED)
+
+$(HYPERV_DRIVER_GENERATED): $(srcdir)/hyperv/hyperv_wmi_generator.input \
+ $(srcdir)/hyperv/hyperv_wmi_generator.py
+ $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/hyperv/hyperv_wmi_generator.py
Don't we need a $(PYTHON) in here, rather than just relying on
#!/usr/bin/env python in the .py file header to work?
+++ b/src/hyperv/.gitignore
@@ -0,0 +1 @@
+*.generated.*
At the top level, this would be /src/hyperv/*.generated.*
+
+
+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?
+ if (data != NULL) {
+#if 0
+ /* FIXME: ws_serializer_free_mem is broken in openwsman<= 2.2.6,
+ * see hypervFreeObject for a detailed explanation. */
Should this be conditional on something like #if
WS_SERIALIZER_FREE_MEM_WORKS, instead of #if 0?
+
+#if 0
+ /* FIXME: ws_serializer_free_mem is broken in openwsman<= 2.2.6,
+ * but this is not that critical, because openwsman keeps
+ * track of all allocations of the deserializer and frees
+ * them in wsmc_release. So this doesn't result in a real
+ * memory leak, but just in piling up unused memory until
+ * the connection is closed. */
Definitely a worthwhile comment.
+
+ /* Check return value */
+ returnValue = ws_xml_get_xpath_value(response, (char
*)"/s:Envelope/s:Body/p:RequestStateChange_OUTPUT/p:ReturnValue");
Nasty that we have to cast away const. Oh well.
+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?
+
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_STARTING:
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SNAPSHOTTING:
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SAVING:
+ return VIR_DOMAIN_RUNNING;
Wow - more states than other domains. Wonder if they are worth exposing
in the libvirt API eventually; but for now, I'm okay with collapsing
them as you have done.
+++ 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?
+++ 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).
+
+ # write output files
+ header.write("/* Generated by hyperv_wmi_generator.py */\n\n\n\n")
+ source.write("/* Generated by hyperv_wmi_generator.py */\n\n\n\n")
+ classes_typedef.write("/* Generated by hyperv_wmi_generator.py
*/\n\n\n\n")
+ classes_header.write("/* Generated by hyperv_wmi_generator.py
*/\n\n\n\n")
+ classes_source.write("/* Generated by hyperv_wmi_generator.py */\n\n\n\n")
Is it worth factoring that message into a reusable constant rather than
repeating it five times?
+#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 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.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org