[libvirt] [PATCH 05/11] hyperv: add hypervWqlQuery struct.

[Dawid Zamirski]

On Sat, 2017-04-01 at 14:27 +0200, Matthias Bolte wrote:
> 2017-03-30 18:47 GMT+02:00 Dawid Zamirski <dzamirski at 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