[libvirt PATCH v3 0/7] Enable autostarting mediated devices

This first version of this series was reviewed quite a while ago and all patches were ACKed except the second one. I posted a second series with changes noted below but it was never ACKed and I dropped the ball for a little while. Subsequently there were questions about whether physical devices (e.g. pci, usb, etc) should return 'true' or 'false' for the GetAutostart()/IsPersistent() calls. I had initially marked them as persistent=true and autostart=true because they superficially act a bit like persistently-defined devices. But Boris convinced me that this is not a very accurate classification since if the physical device is unplugged, there will be no record of it left behind. So I've switched all non-mdev devices to be persistent=false and autostart=false. This is also imperfect, but it seems relatively harmless. Comments welcome. A reminder of what is included in this series: - new API consistent with existing libvirt objects: - virNodeDeviceGetAutostart() - virNodeDeviceSetAutostart() - virNodeDeviceIsPersistent() - virNodeDeviceIsActive - new virsh commands - nodedev-info - nodedev-autostart Changes in version 2: - Parse the autostart property from mdevctl output. Changes in version 3: - switch physical devices to autostart=false, persistent=false - rebase to upstream - update version numbers for new API, etc - fix accidental copy-paste error in virsh command descriptions Jonathon Jongsma (7): api: add virNodeDevice(Get|Set)Autostart() nodedev: implement virNodeDevice(Get|Set)Autostart() nodedev: Add tests for mdevctl autostart command virsh: add nodedev-autostart api: add virNodeDeviceIsPersistent()/IsActive() nodedev: Implement virNodeDeviceIsPersistent()/IsActive() virsh: add nodedev-info docs/manpages/virsh.rst | 27 +++ include/libvirt/libvirt-nodedev.h | 10 + src/conf/node_device_conf.h | 1 + src/conf/virnodedeviceobj.c | 16 ++ src/conf/virnodedeviceobj.h | 6 + src/driver-nodedev.h | 18 ++ src/libvirt-nodedev.c | 141 +++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 4 + src/node_device/node_device_driver.c | 189 +++++++++++++++++- src/node_device/node_device_driver.h | 19 ++ src/node_device/node_device_udev.c | 22 +- src/remote/remote_driver.c | 6 +- src/remote/remote_protocol.x | 60 +++++- src/remote_protocol-structs | 26 +++ .../nodedevmdevctldata/mdevctl-autostart.argv | 8 + tests/nodedevmdevctltest.c | 55 +++++ tools/virsh-nodedev.c | 139 +++++++++++++ 18 files changed, 740 insertions(+), 9 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv -- 2.31.1

This will allow persistent mediated devices to be configured to be restarted automatically when the host reboots. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-nodedev.h | 6 +++ src/driver-nodedev.h | 10 ++++ src/libvirt-nodedev.c | 76 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/remote/remote_driver.c | 4 +- src/remote/remote_protocol.x | 29 +++++++++++- src/remote_protocol-structs | 12 +++++ 7 files changed, 137 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index 067d914d37..0e841ada8a 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -141,6 +141,12 @@ int virNodeDeviceUndefine(virNodeDevicePtr dev, int virNodeDeviceCreate(virNodeDevicePtr dev, unsigned int flags); +int virNodeDeviceSetAutostart(virNodeDevicePtr dev, + int autostart); + +int virNodeDeviceGetAutostart(virNodeDevicePtr dev, + int *autostart); + /** * VIR_NODE_DEVICE_EVENT_CALLBACK: * diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h index dd56421a54..125f4cfd9e 100644 --- a/src/driver-nodedev.h +++ b/src/driver-nodedev.h @@ -87,6 +87,14 @@ typedef int (*virDrvNodeDeviceCreate)(virNodeDevicePtr dev, unsigned int flags); +typedef int +(*virDrvNodeDeviceSetAutostart)(virNodeDevicePtr dev, + int autostart); + +typedef int +(*virDrvNodeDeviceGetAutostart)(virNodeDevicePtr dev, + int *autostart); + typedef int (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn, virNodeDevicePtr dev, @@ -128,4 +136,6 @@ struct _virNodeDeviceDriver { virDrvNodeDeviceDefineXML nodeDeviceDefineXML; virDrvNodeDeviceUndefine nodeDeviceUndefine; virDrvNodeDeviceCreate nodeDeviceCreate; + virDrvNodeDeviceSetAutostart nodeDeviceSetAutostart; + virDrvNodeDeviceGetAutostart nodeDeviceGetAutostart; }; diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index e416c12534..68fc83203d 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -979,3 +979,79 @@ virConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, virDispatchError(conn); return -1; } + +/** + * virNodeDeviceSetAutostart: + * @dev: the device object + * @autostart: whether the device should be automatically started + * + * Configure the node device to be automatically started when the host machine + * boots or the parent device becomes available. + * + * Returns -1 in case of error, 0 in case of success + */ +int +virNodeDeviceSetAutostart(virNodeDevicePtr dev, + int autostart) +{ + VIR_DEBUG("dev=%p", dev); + + virResetLastError(); + + virCheckNodeDeviceReturn(dev, -1); + virCheckReadOnlyGoto(dev->conn->flags, error); + + if (dev->conn->nodeDeviceDriver && + dev->conn->nodeDeviceDriver->nodeDeviceSetAutostart) { + int retval = dev->conn->nodeDeviceDriver->nodeDeviceSetAutostart(dev, autostart); + if (retval < 0) + goto error; + + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dev->conn); + return -1; +} + + +/** + * virNodeDeviceGetAutostart: + * @dev: the device object + * @autostart: the value returned + * + * Provides a boolean value indicating whether the node device is configured to + * be automatically started when the host machine boots or the parent device + * becomes available. + * + * Returns -1 in case of error, 0 in case of success + */ +int +virNodeDeviceGetAutostart(virNodeDevicePtr dev, + int *autostart) +{ + VIR_DEBUG("dev=%p", dev); + + virResetLastError(); + + virCheckNodeDeviceReturn(dev, -1); + virCheckReadOnlyGoto(dev->conn->flags, error); + + if (dev->conn->nodeDeviceDriver && + dev->conn->nodeDeviceDriver->nodeDeviceGetAutostart) { + int retval = dev->conn->nodeDeviceDriver->nodeDeviceGetAutostart(dev, autostart); + if (retval < 0) + goto error; + + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dev->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 68f5e9c900..206b20f773 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -899,6 +899,8 @@ LIBVIRT_7.3.0 { LIBVIRT_7.7.0 { global: virNWFilterDefineXMLFlags; + virNodeDeviceSetAutostart; + virNodeDeviceGetAutostart; } LIBVIRT_7.3.0; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9ee22e7e15..d8febcf46f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8672,7 +8672,9 @@ static virNodeDeviceDriver node_device_driver = { .nodeDeviceCreate = remoteNodeDeviceCreate, /* 7.3.0 */ .nodeDeviceDefineXML = remoteNodeDeviceDefineXML, /* 7.3.0 */ .nodeDeviceUndefine = remoteNodeDeviceUndefine, /* 7.3.0 */ - .nodeDeviceDestroy = remoteNodeDeviceDestroy /* 0.6.3 */ + .nodeDeviceDestroy = remoteNodeDeviceDestroy, /* 0.6.3 */ + .nodeDeviceGetAutostart = remoteNodeDeviceGetAutostart, /* 7.7.0 */ + .nodeDeviceSetAutostart = remoteNodeDeviceSetAutostart, /* 7.7.0 */ }; static virNWFilterDriver nwfilter_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 56f610839e..c4f26ecb4c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2173,6 +2173,19 @@ struct remote_node_device_create_args { unsigned int flags; }; +struct remote_node_device_get_autostart_args { + remote_nonnull_string name; +}; + +struct remote_node_device_get_autostart_ret { + int autostart; +}; + +struct remote_node_device_set_autostart_args { + remote_nonnull_string name; + int autostart; +}; + /* * Events Register/Deregister: @@ -6801,5 +6814,19 @@ enum remote_procedure { * @acl: nwfilter:write * @acl: nwfilter:save */ - REMOTE_PROC_NWFILTER_DEFINE_XML_FLAGS = 431 + REMOTE_PROC_NWFILTER_DEFINE_XML_FLAGS = 431, + + /** + * @generate: both + * @priority: high + * @acl: node_device:read + */ + REMOTE_PROC_NODE_DEVICE_GET_AUTOSTART = 432, + + /** + * @generate: both + * @priority: high + * @acl: node_device:write + */ + REMOTE_PROC_NODE_DEVICE_SET_AUTOSTART = 433 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index d51f12f781..dad3a418cb 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1622,6 +1622,16 @@ struct remote_node_device_create_args { remote_nonnull_string name; u_int flags; }; +struct remote_node_device_get_autostart_args { + remote_nonnull_string name; +}; +struct remote_node_device_get_autostart_ret { + int autostart; +}; +struct remote_node_device_set_autostart_args { + remote_nonnull_string name; + int autostart; +}; struct remote_connect_domain_event_register_ret { int cb_registered; }; @@ -3631,4 +3641,6 @@ enum remote_procedure { REMOTE_PROC_NODE_DEVICE_UNDEFINE = 429, REMOTE_PROC_NODE_DEVICE_CREATE = 430, REMOTE_PROC_NWFILTER_DEFINE_XML_FLAGS = 431, + REMOTE_PROC_NODE_DEVICE_GET_AUTOSTART = 432, + REMOTE_PROC_NODE_DEVICE_SET_AUTOSTART = 433, }; -- 2.31.1

Implement autostart functionality for mediated devices. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/conf/node_device_conf.h | 1 + src/conf/virnodedeviceobj.c | 16 +++ src/conf/virnodedeviceobj.h | 6 ++ src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 139 ++++++++++++++++++++++++++- src/node_device/node_device_driver.h | 13 +++ src/node_device/node_device_udev.c | 12 ++- 7 files changed, 185 insertions(+), 4 deletions(-) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 786de85060..5a4d9c7a55 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -154,6 +154,7 @@ struct _virNodeDevCapMdev { virMediatedDeviceAttr **attributes; size_t nattributes; char *parent_addr; + bool autostart; }; typedef struct _virNodeDevCapPCIDev virNodeDevCapPCIDev; diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 87e180f3ef..9a9841576a 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -41,6 +41,7 @@ struct _virNodeDeviceObj { used by testdriver */ bool active; bool persistent; + bool autostart; }; struct _virNodeDeviceObjList { @@ -1034,6 +1035,21 @@ virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj, } +bool +virNodeDeviceObjIsAutostart(virNodeDeviceObj *obj) +{ + return obj->autostart; +} + + +void +virNodeDeviceObjSetAutostart(virNodeDeviceObj *obj, + bool autostart) +{ + obj->autostart = autostart; +} + + typedef struct _PredicateHelperData PredicateHelperData; struct _PredicateHelperData { virNodeDeviceObjListPredicate predicate; diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h index 068c7c6f34..5f6d22e1f6 100644 --- a/src/conf/virnodedeviceobj.h +++ b/src/conf/virnodedeviceobj.h @@ -137,6 +137,12 @@ virNodeDeviceObjIsPersistent(virNodeDeviceObj *obj); void virNodeDeviceObjSetPersistent(virNodeDeviceObj *obj, bool persistent); +bool +virNodeDeviceObjIsAutostart(virNodeDeviceObj *obj); + +void +virNodeDeviceObjSetAutostart(virNodeDeviceObj *obj, + bool autostart); typedef bool (*virNodeDeviceObjListPredicate)(virNodeDeviceObj *obj, const void *opaque); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 090ac80691..e78a8c8631 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1287,6 +1287,7 @@ virNetworkPortDefSaveStatus; virNodeDeviceObjEndAPI; virNodeDeviceObjGetDef; virNodeDeviceObjIsActive; +virNodeDeviceObjIsAutostart; virNodeDeviceObjIsPersistent; virNodeDeviceObjListAssignDef; virNodeDeviceObjListExport; @@ -1304,6 +1305,7 @@ virNodeDeviceObjListNumOfDevices; virNodeDeviceObjListRemove; virNodeDeviceObjListRemoveLocked; virNodeDeviceObjSetActive; +virNodeDeviceObjSetAutostart; virNodeDeviceObjSetPersistent; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index db619aa2d8..bb18b24e53 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -621,11 +621,12 @@ nodeDeviceDefToMdevctlConfig(virNodeDeviceDef *def, char **buf) size_t i; virNodeDevCapMdev *mdev = &def->caps->data.mdev; g_autoptr(virJSONValue) json = virJSONValueNewObject(); + const char *startval = mdev->autostart ? "auto" : "manual"; if (virJSONValueObjectAppendString(json, "mdev_type", mdev->type) < 0) return -1; - if (virJSONValueObjectAppendString(json, "start", "manual") < 0) + if (virJSONValueObjectAppendString(json, "start", startval) < 0) return -1; if (mdev->attributes) { @@ -1014,6 +1015,46 @@ virMdevctlStart(virNodeDeviceDef *def) } +/* gets a virCommand object that executes a mdevctl command to set the + * 'autostart' property of the device to the specified value + */ +virCommand* +nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def, + bool autostart, + char **errmsg) +{ + virCommand *cmd = virCommandNewArgList(MDEVCTL, + "modify", + "--uuid", + def->caps->data.mdev.uuid, + NULL); + + if (autostart) + virCommandAddArg(cmd, "--auto"); + else + virCommandAddArg(cmd, "--manual"); + + virCommandSetErrorBuffer(cmd, errmsg); + + return cmd; +} + + +static int +virMdevctlSetAutostart(virNodeDeviceDef *def, bool autostart, char **errmsg) +{ + int status; + g_autoptr(virCommand) cmd = NULL; + + cmd = nodeDeviceGetMdevctlSetAutostartCommand(def, autostart, errmsg); + + if (virCommandRun(cmd, &status) < 0 || status != 0) + return -1; + + return 0; +} + + virCommand* nodeDeviceGetMdevctlListCommand(bool defined, char **output, @@ -1066,6 +1107,7 @@ nodeDeviceParseMdevctlChildDevice(const char *parent, virJSONValue *attrs; g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1); virNodeDeviceObj *parent_obj; + const char *start = NULL; /* the child object should have a single key equal to its uuid. * The value is an object describing the properties of the mdev */ @@ -1094,6 +1136,8 @@ nodeDeviceParseMdevctlChildDevice(const char *parent, mdev->parent_addr = g_strdup(parent); mdev->type = g_strdup(virJSONValueObjectGetString(props, "mdev_type")); + start = virJSONValueObjectGetString(props, "start"); + mdev->autostart = STREQ_NULLABLE(start, "auto"); attrs = virJSONValueObjectGet(props, "attrs"); @@ -1346,6 +1390,7 @@ nodeDeviceUpdateMediatedDevice(virNodeDeviceDef *def) /* all devices returned by virMdevctlListDefined() are persistent */ virNodeDeviceObjSetPersistent(obj, true); + virNodeDeviceObjSetAutostart(obj, def->caps->data.mdev.autostart); if (!defined) event = virNodeDeviceEventLifecycleNew(name, @@ -1649,10 +1694,12 @@ removeMissingPersistentMdev(virNodeDeviceObj *obj, /* The device is active, but no longer defined by mdevctl. Keep the device * in the list, but mark it as non-persistent */ - if (virNodeDeviceObjIsActive(obj)) + if (virNodeDeviceObjIsActive(obj)) { + virNodeDeviceObjSetAutostart(obj, false); virNodeDeviceObjSetPersistent(obj, false); - else + } else { remove = true; + } virObjectEventStateQueue(driver->nodeDeviceEventState, event); @@ -1762,6 +1809,92 @@ nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst, if (virMediatedDeviceAttrsCopy(dstmdev, srcmdev)) ret = true; + if (dstmdev->autostart != srcmdev->autostart) { + ret = true; + dstmdev->autostart = srcmdev->autostart; + } + + return ret; +} + + +int +nodeDeviceSetAutostart(virNodeDevice *device, + int autostart) +{ + int ret = -1; + virNodeDeviceObj *obj = NULL; + virNodeDeviceDef *def = NULL; + + if (nodeDeviceInitWait() < 0) + return -1; + + if (!(obj = nodeDeviceObjFindByName(device->name))) + return -1; + def = virNodeDeviceObjGetDef(obj); + + if (virNodeDeviceSetAutostartEnsureACL(device->conn, def) < 0) + goto cleanup; + + if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) { + if (!virNodeDeviceObjIsPersistent(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot set autostart for transient device")); + goto cleanup; + } + + if (autostart != virNodeDeviceObjIsAutostart(obj)) { + g_autofree char *errmsg = NULL; + + if (virMdevctlSetAutostart(def, autostart, &errmsg) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set autostart on '%s': %s"), + def->name, + errmsg && errmsg[0] != '\0' ? errmsg : _("Unknown Error")); + goto cleanup; + } + /* Due to mdevctl performance issues, it may take several seconds + * to re-query mdevctl for the defined devices. Because the mdevctl + * command returned without an error status, assume it was + * successful and set the object status directly here rather than + * waiting for the next query */ + virNodeDeviceObjSetAutostart(obj, autostart); + } + ret = 0; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Unsupported device type")); + } + + cleanup: + virNodeDeviceObjEndAPI(&obj); + return ret; +} + + +int +nodeDeviceGetAutostart(virNodeDevice *device, + int *autostart) +{ + virNodeDeviceObj *obj = NULL; + virNodeDeviceDef *def = NULL; + int ret = -1; + + if (nodeDeviceInitWait() < 0) + return -1; + + if (!(obj = nodeDeviceObjFindByName(device->name))) + return -1; + def = virNodeDeviceObjGetDef(obj); + + if (virNodeDeviceGetAutostartEnsureACL(device->conn, def) < 0) + goto cleanup; + + *autostart = virNodeDeviceObjIsAutostart(obj); + ret = 0; + + cleanup: + virNodeDeviceObjEndAPI(&obj); return ret; } diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index fdc92b8aef..17c7473d85 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -177,3 +177,16 @@ int nodeDeviceDefPostParse(virNodeDeviceDef *def, int nodeDeviceDefValidate(virNodeDeviceDef *def, void *opaque); + +int +nodeDeviceSetAutostart(virNodeDevice *dev, + int autostart); + +int +nodeDeviceGetAutostart(virNodeDevice *dev, + int *autostart); + +virCommand* +nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def, + bool autostart, + char **errmsg); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 90a64f16b0..73334c1cc7 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1504,6 +1504,8 @@ udevAddOneDevice(struct udev_device *device) bool new_device = true; int ret = -1; bool was_persistent = false; + bool autostart = false; + bool is_mdev; def = g_new0(virNodeDeviceDef, 1); @@ -1525,12 +1527,16 @@ udevAddOneDevice(struct udev_device *device) if (udevSetParent(device, def) != 0) goto cleanup; + is_mdev = def->caps->data.type == VIR_NODE_DEV_CAP_MDEV; + if ((obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) { objdef = virNodeDeviceObjGetDef(obj); - if (def->caps->data.type == VIR_NODE_DEV_CAP_MDEV) + if (is_mdev) nodeDeviceDefCopyFromMdevctl(def, objdef); was_persistent = virNodeDeviceObjIsPersistent(obj); + autostart = virNodeDeviceObjIsAutostart(obj); + /* If the device was defined by mdevctl and was never instantiated, it * won't have a sysfs path. We need to emit a CREATED event... */ new_device = (objdef->sysfs_path == NULL); @@ -1543,6 +1549,7 @@ udevAddOneDevice(struct udev_device *device) if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def))) goto cleanup; virNodeDeviceObjSetPersistent(obj, was_persistent); + virNodeDeviceObjSetAutostart(obj, autostart); objdef = virNodeDeviceObjGetDef(obj); if (new_device) @@ -1946,6 +1953,7 @@ udevSetupSystemDev(void) goto cleanup; virNodeDeviceObjSetActive(obj, true); + virNodeDeviceObjSetAutostart(obj, true); virNodeDeviceObjEndAPI(&obj); @@ -2350,6 +2358,8 @@ static virNodeDeviceDriver udevNodeDeviceDriver = { .nodeDeviceDefineXML = nodeDeviceDefineXML, /* 7.3.0 */ .nodeDeviceUndefine = nodeDeviceUndefine, /* 7.3.0 */ .nodeDeviceCreate = nodeDeviceCreate, /* 7.3.0 */ + .nodeDeviceSetAutostart = nodeDeviceSetAutostart, /* 7.7.0 */ + .nodeDeviceGetAutostart = nodeDeviceGetAutostart, /* 7.7.0 */ }; -- 2.31.1

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- .../nodedevmdevctldata/mdevctl-autostart.argv | 8 +++ tests/nodedevmdevctltest.c | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv diff --git a/tests/nodedevmdevctldata/mdevctl-autostart.argv b/tests/nodedevmdevctldata/mdevctl-autostart.argv new file mode 100644 index 0000000000..9b441e9466 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-autostart.argv @@ -0,0 +1,8 @@ +mdevctl \ +modify \ +--uuid d069d019-36ea-4111-8f0a-8c9a70e21366 \ +--auto +mdevctl \ +modify \ +--uuid d069d019-36ea-4111-8f0a-8c9a70e21366 \ +--manual diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 3d1a7e4b6c..92d3c75766 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -124,6 +124,56 @@ testMdevctlHelper(const void *data) } +static int +testMdevctlAutostart(const void *data G_GNUC_UNUSED) +{ + g_autoptr(virNodeDeviceDef) def = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *actualCmdline = NULL; + int ret = -1; + g_autoptr(virCommand) enablecmd = NULL; + g_autoptr(virCommand) disablecmd = NULL; + g_autofree char *errmsg = NULL; + /* just concatenate both calls into the same output file */ + g_autofree char *cmdlinefile = + g_strdup_printf("%s/nodedevmdevctldata/mdevctl-autostart.argv", + abs_srcdir); + g_autofree char *mdevxml = + g_strdup_printf("%s/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml", + abs_srcdir); + g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); + + if (!(def = virNodeDeviceDefParseFile(mdevxml, CREATE_DEVICE, VIRT_TYPE, + &parser_callbacks, NULL))) + return -1; + + virCommandSetDryRun(dryRunToken, &buf, true, true, NULL, NULL); + + if (!(enablecmd = nodeDeviceGetMdevctlSetAutostartCommand(def, true, &errmsg))) + goto cleanup; + + if (virCommandRun(enablecmd, NULL) < 0) + goto cleanup; + + if (!(disablecmd = nodeDeviceGetMdevctlSetAutostartCommand(def, false, &errmsg))) + goto cleanup; + + if (virCommandRun(disablecmd, NULL) < 0) + goto cleanup; + + if (!(actualCmdline = virBufferCurrentContent(&buf))) + goto cleanup; + + if (virTestCompareToFileFull(actualCmdline, cmdlinefile, false) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virBufferFreeAndReset(&buf); + return ret; +} + static int testMdevctlListDefined(const void *data G_GNUC_UNUSED) { @@ -378,6 +428,9 @@ mymain(void) #define DO_TEST_LIST_DEFINED() \ DO_TEST_FULL("list defined mdevs", testMdevctlListDefined, NULL) +#define DO_TEST_AUTOSTART() \ + DO_TEST_FULL("autostart mdevs", testMdevctlAutostart, NULL) + #define DO_TEST_PARSE_JSON(filename) \ DO_TEST_FULL("parse mdevctl json " filename, testMdevctlParse, filename) @@ -401,6 +454,8 @@ mymain(void) DO_TEST_START("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); + DO_TEST_AUTOSTART(); + done: nodedevTestDriverFree(driver); -- 2.31.1

Add ability to set node devices to autostart on boot or parent device availability. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/manpages/virsh.rst | 15 +++++++++ tools/virsh-nodedev.c | 71 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index e0cdabf3aa..08097a45bf 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -5131,6 +5131,21 @@ When *--timestamp* is used, a human-readable timestamp will be printed before the event. +nodedev-autostart +----------------- + +**Syntax:** + +:: + + nodedev-autostart [--disable] device + +Configure a device to be automatically started when the host machine boots or +the parent device becomes available. With *--disable*, the device will be set +to manual mode and will no longer be automatically started by the host. This +command is only supported for persistently-defined mediated devices. + + VIRTUAL NETWORK COMMANDS ======================== diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 945ccc7f45..0a70029fc7 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -1153,6 +1153,71 @@ cmdNodeDeviceStart(vshControl *ctl, const vshCmd *cmd) } +/* + * "nodedev-autostart" command + */ +static const vshCmdInfo info_node_device_autostart[] = { + {.name = "help", + .data = N_("autostart a defined node device") + }, + {.name = "desc", + .data = N_("Configure a node device to be automatically started at boot.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_node_device_autostart[] = { + {.name = "device", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("device name or wwn pair in 'wwnn,wwpn' format"), + .completer = virshNodeDeviceNameCompleter, + }, + {.name = "disable", + .type = VSH_OT_BOOL, + .help = N_("disable autostarting") + }, + {.name = NULL} +}; + +static bool +cmdNodeDeviceAutostart(vshControl *ctl, const vshCmd *cmd) +{ + virNodeDevice *dev = NULL; + bool ret = false; + const char *name = NULL; + int autostart; + + if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0) + return false; + + dev = vshFindNodeDevice(ctl, name); + + if (!dev) goto cleanup; + + autostart = !vshCommandOptBool(cmd, "disable"); + + if (virNodeDeviceSetAutostart(dev, autostart) < 0) { + if (autostart) + vshError(ctl, _("failed to mark device %s as autostarted"), name); + else + vshError(ctl, _("failed to unmark device %s as autostarted"), name); + goto cleanup; + } + + if (autostart) + vshPrintExtra(ctl, _("Device %s marked as autostarted\n"), name); + else + vshPrintExtra(ctl, _("Device %s unmarked as autostarted\n"), name); + + ret = true; + cleanup: + if (dev) + virNodeDeviceFree(dev); + return ret; +} + + const vshCmdDef nodedevCmds[] = { {.name = "nodedev-create", .handler = cmdNodeDeviceCreate, @@ -1224,5 +1289,11 @@ const vshCmdDef nodedevCmds[] = { .info = info_node_device_start, .flags = 0 }, + {.name = "nodedev-autostart", + .handler = cmdNodeDeviceAutostart, + .opts = opts_node_device_autostart, + .info = info_node_device_autostart, + .flags = 0 + }, {.name = NULL} }; -- 2.31.1

These two public APIs are implemented for almost all other objects that have a concept of persistent definition and activatability. Now that we have node devices (mdevs) that can be defined and inactive, it will be useful to query the persistent/active state of node devices as well. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-nodedev.h | 4 ++ src/driver-nodedev.h | 8 ++++ src/libvirt-nodedev.c | 65 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 33 +++++++++++++++- src/remote_protocol-structs | 14 +++++++ 7 files changed, 127 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index 0e841ada8a..e492634217 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -147,6 +147,10 @@ int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart); +int virNodeDeviceIsPersistent(virNodeDevicePtr dev); + +int virNodeDeviceIsActive(virNodeDevicePtr dev); + /** * VIR_NODE_DEVICE_EVENT_CALLBACK: * diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h index 125f4cfd9e..167a8166dd 100644 --- a/src/driver-nodedev.h +++ b/src/driver-nodedev.h @@ -95,6 +95,12 @@ typedef int (*virDrvNodeDeviceGetAutostart)(virNodeDevicePtr dev, int *autostart); +typedef int +(*virDrvNodeDeviceIsPersistent)(virNodeDevicePtr dev); + +typedef int +(*virDrvNodeDeviceIsActive)(virNodeDevicePtr dev); + typedef int (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn, virNodeDevicePtr dev, @@ -138,4 +144,6 @@ struct _virNodeDeviceDriver { virDrvNodeDeviceCreate nodeDeviceCreate; virDrvNodeDeviceSetAutostart nodeDeviceSetAutostart; virDrvNodeDeviceGetAutostart nodeDeviceGetAutostart; + virDrvNodeDeviceIsPersistent nodeDeviceIsPersistent; + virDrvNodeDeviceIsActive nodeDeviceIsActive; }; diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index 68fc83203d..8ad1e9cb9e 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -1055,3 +1055,68 @@ virNodeDeviceGetAutostart(virNodeDevicePtr dev, virDispatchError(dev->conn); return -1; } + +/** + * virNodeDeviceIsPersistent: + * @dev: pointer to the nodedev object + * + * Determine if the node device has a persistent configuration + * which means it will still exist after shutting down + * + * Returns 1 if persistent, 0 if transient, -1 on error + */ +int +virNodeDeviceIsPersistent(virNodeDevicePtr dev) +{ + VIR_DEBUG("dev=%p", dev); + + virResetLastError(); + + virCheckNodeDeviceReturn(dev, -1); + + if (dev->conn->nodeDeviceDriver && + dev->conn->nodeDeviceDriver->nodeDeviceIsPersistent) { + int ret; + ret = dev->conn->nodeDeviceDriver->nodeDeviceIsPersistent(dev); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(dev->conn); + return -1; +} + + +/** + * virNodeDeviceIsActive: + * @dev: pointer to the node device object + * + * Determine if the node device is currently active + * + * Returns 1 if active, 0 if inactive, -1 on error + */ +int virNodeDeviceIsActive(virNodeDevicePtr dev) +{ + VIR_DEBUG("dev=%p", dev); + + virResetLastError(); + + virCheckNodeDeviceReturn(dev, -1); + + if (dev->conn->nodeDeviceDriver && + dev->conn->nodeDeviceDriver->nodeDeviceIsActive) { + int ret; + ret = dev->conn->nodeDeviceDriver->nodeDeviceIsActive(dev); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(dev->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 206b20f773..a4ae48a6c3 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -901,6 +901,8 @@ LIBVIRT_7.7.0 { virNWFilterDefineXMLFlags; virNodeDeviceSetAutostart; virNodeDeviceGetAutostart; + virNodeDeviceIsPersistent; + virNodeDeviceIsActive; } LIBVIRT_7.3.0; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d8febcf46f..ba1999f468 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8675,6 +8675,8 @@ static virNodeDeviceDriver node_device_driver = { .nodeDeviceDestroy = remoteNodeDeviceDestroy, /* 0.6.3 */ .nodeDeviceGetAutostart = remoteNodeDeviceGetAutostart, /* 7.7.0 */ .nodeDeviceSetAutostart = remoteNodeDeviceSetAutostart, /* 7.7.0 */ + .nodeDeviceIsPersistent = remoteNodeDeviceIsPersistent, /* 7.7.0 */ + .nodeDeviceIsActive = remoteNodeDeviceIsActive, /* 7.7.0 */ }; static virNWFilterDriver nwfilter_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c4f26ecb4c..1815867b12 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2186,6 +2186,22 @@ struct remote_node_device_set_autostart_args { int autostart; }; +struct remote_node_device_is_persistent_args { + remote_nonnull_string name; +}; + +struct remote_node_device_is_persistent_ret { + int persistent; +}; + +struct remote_node_device_is_active_args { + remote_nonnull_string name; +}; + +struct remote_node_device_is_active_ret { + int active; +}; + /* * Events Register/Deregister: @@ -6828,5 +6844,20 @@ enum remote_procedure { * @priority: high * @acl: node_device:write */ - REMOTE_PROC_NODE_DEVICE_SET_AUTOSTART = 433 + REMOTE_PROC_NODE_DEVICE_SET_AUTOSTART = 433, + + /** + * @generate: both + * @priority: high + * @acl: node_device:read + */ + REMOTE_PROC_NODE_DEVICE_IS_PERSISTENT = 434, + + /** + * @generate: both + * @priority: high + * @acl: node_device:read + */ + REMOTE_PROC_NODE_DEVICE_IS_ACTIVE = 435 + }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index dad3a418cb..86cccff048 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1632,6 +1632,18 @@ struct remote_node_device_set_autostart_args { remote_nonnull_string name; int autostart; }; +struct remote_node_device_is_persistent_args { + remote_nonnull_string name; +}; +struct remote_node_device_is_persistent_ret { + int persistent; +}; +struct remote_node_device_is_active_args { + remote_nonnull_string name; +}; +struct remote_node_device_is_active_ret { + int active; +}; struct remote_connect_domain_event_register_ret { int cb_registered; }; @@ -3643,4 +3655,6 @@ enum remote_procedure { REMOTE_PROC_NWFILTER_DEFINE_XML_FLAGS = 431, REMOTE_PROC_NODE_DEVICE_GET_AUTOSTART = 432, REMOTE_PROC_NODE_DEVICE_SET_AUTOSTART = 433, + REMOTE_PROC_NODE_DEVICE_IS_PERSISTENT = 434, + REMOTE_PROC_NODE_DEVICE_IS_ACTIVE = 435, }; -- 2.31.1

Implement these new API functions in the nodedev driver. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- src/node_device/node_device_driver.c | 50 ++++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 6 ++++ src/node_device/node_device_udev.c | 10 ++++-- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index bb18b24e53..3bc6eb1c11 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1989,3 +1989,53 @@ int nodeDeviceDefValidate(virNodeDeviceDef *def, } return 0; } + + +int +nodeDeviceIsPersistent(virNodeDevice *device) +{ + virNodeDeviceObj *obj = NULL; + virNodeDeviceDef *def = NULL; + int ret = -1; + + if (nodeDeviceInitWait() < 0) + return -1; + + if (!(obj = nodeDeviceObjFindByName(device->name))) + return -1; + def = virNodeDeviceObjGetDef(obj); + + if (virNodeDeviceIsPersistentEnsureACL(device->conn, def) < 0) + goto cleanup; + + ret = virNodeDeviceObjIsPersistent(obj); + + cleanup: + virNodeDeviceObjEndAPI(&obj); + return ret; +} + + +int +nodeDeviceIsActive(virNodeDevice *device) +{ + virNodeDeviceObj *obj = NULL; + virNodeDeviceDef *def = NULL; + int ret = -1; + + if (nodeDeviceInitWait() < 0) + return -1; + + if (!(obj = nodeDeviceObjFindByName(device->name))) + return -1; + def = virNodeDeviceObjGetDef(obj); + + if (virNodeDeviceIsActiveEnsureACL(device->conn, def) < 0) + goto cleanup; + + ret = virNodeDeviceObjIsActive(obj); + + cleanup: + virNodeDeviceObjEndAPI(&obj); + return ret; +} diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 17c7473d85..7311b603ac 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -186,6 +186,12 @@ int nodeDeviceGetAutostart(virNodeDevice *dev, int *autostart); +int +nodeDeviceIsPersistent(virNodeDevice *dev); + +int +nodeDeviceIsActive(virNodeDevice *dev); + virCommand* nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def, bool autostart, diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 73334c1cc7..0f1770f771 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1503,7 +1503,7 @@ udevAddOneDevice(struct udev_device *device) virObjectEvent *event = NULL; bool new_device = true; int ret = -1; - bool was_persistent = false; + bool persistent = false; bool autostart = false; bool is_mdev; @@ -1534,7 +1534,8 @@ udevAddOneDevice(struct udev_device *device) if (is_mdev) nodeDeviceDefCopyFromMdevctl(def, objdef); - was_persistent = virNodeDeviceObjIsPersistent(obj); + + persistent = virNodeDeviceObjIsPersistent(obj); autostart = virNodeDeviceObjIsAutostart(obj); /* If the device was defined by mdevctl and was never instantiated, it @@ -1548,7 +1549,7 @@ udevAddOneDevice(struct udev_device *device) * and the current definition will take its place. */ if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def))) goto cleanup; - virNodeDeviceObjSetPersistent(obj, was_persistent); + virNodeDeviceObjSetPersistent(obj, persistent); virNodeDeviceObjSetAutostart(obj, autostart); objdef = virNodeDeviceObjGetDef(obj); @@ -1954,6 +1955,7 @@ udevSetupSystemDev(void) virNodeDeviceObjSetActive(obj, true); virNodeDeviceObjSetAutostart(obj, true); + virNodeDeviceObjSetPersistent(obj, true); virNodeDeviceObjEndAPI(&obj); @@ -2360,6 +2362,8 @@ static virNodeDeviceDriver udevNodeDeviceDriver = { .nodeDeviceCreate = nodeDeviceCreate, /* 7.3.0 */ .nodeDeviceSetAutostart = nodeDeviceSetAutostart, /* 7.7.0 */ .nodeDeviceGetAutostart = nodeDeviceGetAutostart, /* 7.7.0 */ + .nodeDeviceIsPersistent = nodeDeviceIsPersistent, /* 7.7.0 */ + .nodeDeviceIsActive = nodeDeviceIsActive, /* 7.7.0 */ }; -- 2.31.1

This is currently the only way to view the 'autostart' property for a node device in virsh. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/manpages/virsh.rst | 12 ++++++++ tools/virsh-nodedev.c | 68 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 08097a45bf..af8c4cb9eb 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -5057,6 +5057,18 @@ be either device name or wwn pair in "wwnn,wwpn" format (only works for HBA). +nodedev-info +------------ + +**Syntax:** + +:: + + nodedev-info device + +Returns basic information about the *device* object. + + nodedev-list ------------ diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 0a70029fc7..f1b4eb94bf 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -1218,6 +1218,68 @@ cmdNodeDeviceAutostart(vshControl *ctl, const vshCmd *cmd) } +/* + * "nodedev-info" command + */ +static const vshCmdInfo info_node_device_info[] = { + {.name = "help", + .data = N_("node device information") + }, + {.name = "desc", + .data = N_("Returns basic information about the node device") + }, + {.name = NULL} +}; + + +static const vshCmdOptDef opts_node_device_info[] = { + {.name = "device", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("device name or wwn pair in 'wwnn,wwpn' format"), + .completer = virshNodeDeviceNameCompleter, + }, + {.name = NULL} +}; + +static bool +cmdNodeDeviceInfo(vshControl *ctl, const vshCmd *cmd) +{ + virNodeDevicePtr device = NULL; + const char *device_value = NULL; + bool ret = false; + int autostart; + const char *parent = NULL; + + if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + return false; + + device = vshFindNodeDevice(ctl, device_value); + + if (!device) + goto cleanup; + + parent = virNodeDeviceGetParent(device); + vshPrint(ctl, "%-15s %s\n", _("Name:"), virNodeDeviceGetName(device)); + vshPrint(ctl, "%-15s %s\n", _("Parent:"), parent ? parent : ""); + vshPrint(ctl, "%-15s %s\n", _("Active:"), virNodeDeviceIsActive(device) ? + _("yes") : _("no")); + vshPrint(ctl, "%-15s %s\n", _("Persistent:"), + virNodeDeviceIsPersistent(device) ? _("yes") : _("no")); + if (virNodeDeviceGetAutostart(device, &autostart) < 0) + vshPrint(ctl, "%-15s %s\n", _("Autostart:"), _("no autostart")); + else + vshPrint(ctl, "%-15s %s\n", _("Autostart:"), autostart ? _("yes") : _("no")); + + ret = true; + cleanup: + if (device) + virNodeDeviceFree(device); + return ret; +} + + + const vshCmdDef nodedevCmds[] = { {.name = "nodedev-create", .handler = cmdNodeDeviceCreate, @@ -1295,5 +1357,11 @@ const vshCmdDef nodedevCmds[] = { .info = info_node_device_autostart, .flags = 0 }, + {.name = "nodedev-info", + .handler = cmdNodeDeviceInfo, + .opts = opts_node_device_info, + .info = info_node_device_info, + .flags = 0 + }, {.name = NULL} }; -- 2.31.1

Ping Can I get a review on this? As I mentioned in the cover letter, the only patch that was not initially ACKed in the last version was the second one. However, the non-mdev devices were also switched to to persistent=false and autostart=false. On Fri, Aug 20, 2021 at 5:36 PM Jonathon Jongsma <jjongsma@redhat.com> wrote:
This first version of this series was reviewed quite a while ago and all patches were ACKed except the second one. I posted a second series with changes noted below but it was never ACKed and I dropped the ball for a little while.
Subsequently there were questions about whether physical devices (e.g. pci, usb, etc) should return 'true' or 'false' for the GetAutostart()/IsPersistent() calls. I had initially marked them as persistent=true and autostart=true because they superficially act a bit like persistently-defined devices. But Boris convinced me that this is not a very accurate classification since if the physical device is unplugged, there will be no record of it left behind. So I've switched all non-mdev devices to be persistent=false and autostart=false. This is also imperfect, but it seems relatively harmless. Comments welcome.
A reminder of what is included in this series: - new API consistent with existing libvirt objects: - virNodeDeviceGetAutostart() - virNodeDeviceSetAutostart() - virNodeDeviceIsPersistent() - virNodeDeviceIsActive - new virsh commands - nodedev-info - nodedev-autostart
Changes in version 2: - Parse the autostart property from mdevctl output.
Changes in version 3: - switch physical devices to autostart=false, persistent=false - rebase to upstream - update version numbers for new API, etc - fix accidental copy-paste error in virsh command descriptions
Jonathon Jongsma (7): api: add virNodeDevice(Get|Set)Autostart() nodedev: implement virNodeDevice(Get|Set)Autostart() nodedev: Add tests for mdevctl autostart command virsh: add nodedev-autostart api: add virNodeDeviceIsPersistent()/IsActive() nodedev: Implement virNodeDeviceIsPersistent()/IsActive() virsh: add nodedev-info
docs/manpages/virsh.rst | 27 +++ include/libvirt/libvirt-nodedev.h | 10 + src/conf/node_device_conf.h | 1 + src/conf/virnodedeviceobj.c | 16 ++ src/conf/virnodedeviceobj.h | 6 + src/driver-nodedev.h | 18 ++ src/libvirt-nodedev.c | 141 +++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 4 + src/node_device/node_device_driver.c | 189 +++++++++++++++++- src/node_device/node_device_driver.h | 19 ++ src/node_device/node_device_udev.c | 22 +- src/remote/remote_driver.c | 6 +- src/remote/remote_protocol.x | 60 +++++- src/remote_protocol-structs | 26 +++ .../nodedevmdevctldata/mdevctl-autostart.argv | 8 + tests/nodedevmdevctltest.c | 55 +++++ tools/virsh-nodedev.c | 139 +++++++++++++ 18 files changed, 740 insertions(+), 9 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv
-- 2.31.1

On 8/21/21 12:29 AM, Jonathon Jongsma wrote:
This first version of this series was reviewed quite a while ago and all patches were ACKed except the second one. I posted a second series with changes noted below but it was never ACKed and I dropped the ball for a little while.
Subsequently there were questions about whether physical devices (e.g. pci, usb, etc) should return 'true' or 'false' for the GetAutostart()/IsPersistent() calls. I had initially marked them as persistent=true and autostart=true because they superficially act a bit like persistently-defined devices. But Boris convinced me that this is not a very accurate classification since if the physical device is unplugged, there will be no record of it left behind. So I've switched all non-mdev devices to be persistent=false and autostart=false. This is also imperfect, but it seems relatively harmless. Comments welcome.
A reminder of what is included in this series: - new API consistent with existing libvirt objects: - virNodeDeviceGetAutostart() - virNodeDeviceSetAutostart() - virNodeDeviceIsPersistent() - virNodeDeviceIsActive - new virsh commands - nodedev-info - nodedev-autostart
Changes in version 2: - Parse the autostart property from mdevctl output.
Changes in version 3: - switch physical devices to autostart=false, persistent=false - rebase to upstream - update version numbers for new API, etc - fix accidental copy-paste error in virsh command descriptions
Jonathon Jongsma (7): api: add virNodeDevice(Get|Set)Autostart() nodedev: implement virNodeDevice(Get|Set)Autostart() nodedev: Add tests for mdevctl autostart command virsh: add nodedev-autostart api: add virNodeDeviceIsPersistent()/IsActive() nodedev: Implement virNodeDeviceIsPersistent()/IsActive() virsh: add nodedev-info
docs/manpages/virsh.rst | 27 +++ include/libvirt/libvirt-nodedev.h | 10 + src/conf/node_device_conf.h | 1 + src/conf/virnodedeviceobj.c | 16 ++ src/conf/virnodedeviceobj.h | 6 + src/driver-nodedev.h | 18 ++ src/libvirt-nodedev.c | 141 +++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 4 + src/node_device/node_device_driver.c | 189 +++++++++++++++++- src/node_device/node_device_driver.h | 19 ++ src/node_device/node_device_udev.c | 22 +- src/remote/remote_driver.c | 6 +- src/remote/remote_protocol.x | 60 +++++- src/remote_protocol-structs | 26 +++ .../nodedevmdevctldata/mdevctl-autostart.argv | 8 + tests/nodedevmdevctltest.c | 55 +++++ tools/virsh-nodedev.c | 139 +++++++++++++ 18 files changed, 740 insertions(+), 9 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv
Sorry for the delay. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Jonathon Jongsma
-
Michal Prívozník