On Tue, Jun 16, 2020 at 09:27:54AM -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(a)redhat.com>
---
...
+
char *
nodeDeviceGetXMLDesc(virNodeDevicePtr device,
@@ -492,6 +518,25 @@ nodeDeviceFindNewDevice(virConnectPtr conn,
}
+static virNodeDevicePtr
+nodeDeviceFindNewMediatedDeviceFunc(virConnectPtr conn,
+ const void *opaque)
+{
+ const char *uuid = opaque;
empty line here..
+ return nodeDeviceLookupMediatedDeviceByUUID(conn, uuid, 0);
+}
+
+
+static virNodeDevicePtr
+nodeDeviceFindNewMediatedDevice(virConnectPtr conn,
+ const char *mdev_uuid)
+{
+ return nodeDeviceFindNewDevice(conn,
+ nodeDeviceFindNewMediatedDeviceFunc,
+ mdev_uuid);
+}
+
+
typedef struct _NewSCSIHostFuncData NewSCSIHostFuncData;
struct _NewSCSIHostFuncData
{
@@ -534,6 +579,158 @@ nodeDeviceHasCapability(virNodeDeviceDefPtr def, virNodeDevCapType
type)
}
+/* format a json string that provides configuration information about this mdev
+ * to the mdevctl utility */
+static int
+nodeDeviceDefToMdevctlConfig(virNodeDeviceDefPtr def, char **buf)
+{
+ size_t i;
+ virNodeDevCapMdevPtr mdev = &def->caps->data.mdev;
+ g_autoptr(virJSONValue) json = virJSONValueNewObject();
+
+ if (virJSONValueObjectAppendString(json, "mdev_type", mdev->type) <
0)
+ return -1;
here...
+ if (virJSONValueObjectAppendString(json, "start",
"manual") < 0)
+ return -1;
here...
+ if (mdev->attributes) {
+ g_autoptr(virJSONValue) attributes = virJSONValueNewArray();
here...
+ for (i = 0; i < mdev->nattributes; i++) {
+ virMediatedDeviceAttrPtr attr = mdev->attributes[i];
+ g_autoptr(virJSONValue) jsonattr = virJSONValueNewObject();
+
+ if (virJSONValueObjectAppendString(jsonattr, attr->name, attr->value)
< 0)
+ return -1;
here...
+ if (virJSONValueArrayAppend(attributes,
g_steal_pointer(&jsonattr)) < 0)
+ return -1;
+ }
and here...
+ if (virJSONValueObjectAppend(json, "attrs",
g_steal_pointer(&attributes)) < 0)
+ return -1;
+ }
+
+ *buf = virJSONValueToString(json, false);
+ if (!*buf)
+ return -1;
+
+ return 0;
+}
+
+
+static char *
+nodeDeviceFindAddressByName(const char *name)
+{
+ virNodeDeviceDefPtr def = NULL;
+ virNodeDevCapsDefPtr caps = NULL;
+ char *pci_addr = NULL;
+ virNodeDeviceObjPtr dev = virNodeDeviceObjListFindByName(driver->devs, name);
+
+ if (!dev) {
+ virReportError(VIR_ERR_NO_NODE_DEVICE,
+ _("could not find device '%s'"), name);
+ return NULL;
+ }
+
+ def = virNodeDeviceObjGetDef(dev);
+ for (caps = def->caps; caps != NULL; caps = caps->next) {
+ if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
+ virPCIDeviceAddress addr = {
+ .domain = caps->data.pci_dev.domain,
+ .bus = caps->data.pci_dev.bus,
+ .slot = caps->data.pci_dev.slot,
+ .function = caps->data.pci_dev.function
+ };
+
+ pci_addr = virPCIDeviceAddressAsString(&addr);
+ break;
+ }
+ }
+
+ virNodeDeviceObjEndAPI(&dev);
+
+ return pci_addr;
+}
+
+
+virCommandPtr
+nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
+ bool persist,
+ char **uuid_out)
+{
+ virCommandPtr cmd;
+ const char *subcommand;
+ g_autofree char *json = NULL;
+ g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
+
+ 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";
Like I mentioned in v2 of this patch, "define" doesn't start anything
without
--auto added to the cmdline. Still, it feels like the function is doing
a tiny bit more than it should IMO. You also noted that you're preparing for when we
start defining mdevs, I'd say let's leave 'persist' out until we actually
introduce support for persisting mdevs.
The rest of the patch looks good to me.
Regards,
Erik