[libvirt PATCH v2 05/10] nodedev: add mdev support to virNodeDeviceCreateXML()

Erik Skultety eskultet at redhat.com
Thu Jun 11 13:52:59 UTC 2020


On Tue, Jun 09, 2020 at 04:43:45PM -0500, Jonathon Jongsma wrote:
> With recent additions to the node device xml schema, an xml schema can
> now describe a mdev device sufficiently for libvirt to create and start
> the device using the mdevctl utility.
>
> Note that some of the the configuration for a mediated device must be
> passed to mdevctl as a JSON-formatted file. In order to avoid creating
> and cleaning up temporary files, the JSON is instead fed to stdin and we
> pass the filename /dev/stdin to mdevctl. While this may not be portable,
> neither are mediated devices, so I don't believe it should cause any
> problems.
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
...
>
> +static int
> +virNodeDeviceObjListFindMediatedDeviceByUUIDCallback(const void *payload,
> +                                                     const void *name G_GNUC_UNUSED,
> +                                                     const void *opaque G_GNUC_UNUSED)

opaque is not unused.

...

> +
> +
> +virCommandPtr
> +nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
> +                                 bool persist,
> +                                 char **uuid_out)
> +{
> +    virCommandPtr cmd;
> +    const char *subcommand;
> +    g_autofree char *json = NULL;
> +    g_autofree char *mdevctl = virFindFileInPath(MDEVCTL);
> +    g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
> +
> +    if (!mdevctl)
> +        return NULL;
> +
> +    if (!parent_pci) {
> +        virReportError(VIR_ERR_NO_NODE_DEVICE,
> +                       _("unable to find PCI address for parent device '%s'"), def->parent);
> +        return NULL;
> +    }
> +
> +    if (nodeDeviceDefToMdevctlConfig(def, &json) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("couldn't convert node device def to mdevctl JSON"));
> +        return NULL;
> +    }
> +
> +    if (persist)
> +        subcommand = "define";

"define" doesn't create an mdev unless '--auto' is passed, so having this as
part of MdevctlStartCommand feels confusing, I think this helper should be
named GetMdevctlCmdline and let the caller pass the correct subcommand.

Regards,
Erik




More information about the libvir-list mailing list