Re: [libvirt] [PATCH 05/11] hyperv: add hypervWqlQuery struct.
On Sat, 2017-04-01 at 14:27 +0200, Matthias Bolte wrote:
2017-03-30 18:47 GMT+02:00 Dawid Zamirski
<dzamirski(a)datto.com>:
> This struct is to be passed to enumerate-and-pull wsman request (to
> run
> "Select" queries) and provides the hypervWmiClassInfoListPtr
> instance
> from which we can extract the version specific info using the new
> hypervGetWmiClassInfo function (currently unused)
> ---
> src/hyperv/hyperv_wmi.c | 35 +++++++++++++++++++++++++++++++++++
> src/hyperv/hyperv_wmi.h | 8 ++++++++
> 2 files changed, 43 insertions(+)
>
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index 309edac..42ce507 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -44,6 +44,41 @@
>
> #define VIR_FROM_THIS VIR_FROM_HYPERV
>
> +static int
> +hypervGetWmiClassInfo(hypervPrivate *priv,
> hypervWmiClassInfoListPtr list,
> + hypervWmiClassInfoPtr *info)
> +{
> + const char *version = "v2";
> + size_t i;
> +
> + if (list->count == 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("The WMI class info list is empty"));
> + return -1;
> + }
> +
> + if (list->count == 1) {
> + *info = list->objs[0];
> + return 0;
Here you assume that if there is only one variation of a WMI class
that it is the v1 variation, and you don't do any version check. At
the moment this is fine, because your assumption holds. But this is
not future proof. If v2-only WMI classes are added then this check
will happily return a v2 WMI class info for a v1 connection.
Yep, that's a problem that needs to be addressed. The reason why I had
it like this was because there are WMI classes that apply to both e.g
Win32_* or CIM_* classes - the only ones currently "versioned" are
Msvm_* classes while the rest is assumed to be "shared". So this code
was assuming if there's just one version then it's "shared" but yeah I
did not think about the case you pointed out.
To address this, I'm going to set version to NULL in the generator for
"shared" classes using the following rule:
* there's only one version specified in .input file
* it does not have explicitly defined version
So if there's a class that's v2-only without v1 equivalent it needs to
be "versioned" in the .input file, e.g "v2/Msvm_VirtualEthernetSwitch,
and vice-versa
This should cover the following cases:
* v1 and v2 classes (list->count > 1, match version == priv-version)
* v2-only or v1-only classes (list->count == 1,
match version == priv-
version)
* "shared" classes (list->count == 1,
match version == NULL)
Regards,
Dawid