[libvirt] [RFC PATCH 06/11] nodedev: Introduce the mdev capability to the nodedev driver structure

[Erik Skultety]

> > +void virNodeDevCapMdevFree(virNodeDevCapMdevPtr mdev)
> > +{
> > +    if (!mdev)
> > +        return;
> > +
> > +    VIR_FREE(mdev->type);
> > +    VIR_FREE(mdev->name);
> > +    VIR_FREE(mdev->description);
> > +    VIR_FREE(mdev->device_api);
> > +    VIR_FREE(mdev);
> > +}
> > +

[...]

> > +
> > +typedef struct _virNodeDevCapMdev virNodeDevCapMdev;
> > +typedef virNodeDevCapMdev *virNodeDevCapMdevPtr;
> > +struct _virNodeDevCapMdev {
> > +    char *type;
> > +    char *name;
> > +    char *description;
> > +    char *device_api;
> > +    unsigned int available_instances;
> > +    unsigned int iommuGroupNumber;
> > +};
> > +
>
> Introducing this structure can be moved into the next patch, it's not
> used here.

It actually is, virNodeDevCapMdevFree uses is, and it also used in the snippet
below. Anyhow, I usually try to introduce concepts, data types and other
symbols first to make it a tiny bit easier for the reviewer, since this can
be easily missed when actually implementing function bodies, or logic in
general. I have no problem with moving everything related to the structure to
the following patch. Although in that case, if this patch would still be worth
having as separate or more-or-less merge it with the next one completely. Let
me know, what you find as the most plausible solution for you.

Erik

>
> >  typedef struct _virNodeDevCapPCIDev virNodeDevCapPCIDev;
> >  typedef virNodeDevCapPCIDev *virNodeDevCapPCIDevPtr;
> >  struct _virNodeDevCapPCIDev {
> > @@ -262,6 +274,7 @@ struct _virNodeDevCapData {
> >          virNodeDevCapStorage storage;
> >          virNodeDevCapSCSIGeneric sg;
> >          virNodeDevCapDRM drm;
> > +        virNodeDevCapMdev mdev;
> >      };
> >  };
> >