[libvirt PATCH v2 12/16] api: add virNodeDeviceUndefine()
Ján Tomko
jtomko at redhat.com
Tue Aug 25 13:12:44 UTC 2020
On a Tuesday in 2020, Jonathon Jongsma wrote:
>This interface allows you to undefine a persistently defined (but
>inactive) mediated devices. It is implemented via 'mdevctl'
>
>Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
>---
> include/libvirt/libvirt-nodedev.h | 2 +
> src/driver-nodedev.h | 4 ++
> src/libvirt-nodedev.c | 36 ++++++++++
> src/libvirt_public.syms | 1 +
> src/node_device/node_device_driver.c | 68 +++++++++++++++++++
> src/node_device/node_device_driver.h | 6 ++
> src/node_device/node_device_udev.c | 1 +
> src/remote/remote_driver.c | 1 +
> src/remote/remote_protocol.x | 14 +++-
> src/remote_protocol-structs | 4 ++
> .../nodedevmdevctldata/mdevctl-undefine.argv | 1 +
> tests/nodedevmdevctltest.c | 56 ++++++++++++---
> 12 files changed, 185 insertions(+), 9 deletions(-)
> create mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv
>
>diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h
>index 02aa9d9750..f7957499ae 100644
>--- a/include/libvirt/libvirt-nodedev.h
>+++ b/include/libvirt/libvirt-nodedev.h
>@@ -131,6 +131,8 @@ virNodeDevicePtr virNodeDeviceDefineXML (virConnectPtr conn,
> const char *xmlDesc,
> unsigned int flags);
>
>+int virNodeDeviceUndefine (virNodeDevicePtr dev);
>+
> /**
> * VIR_NODE_DEVICE_EVENT_CALLBACK:
> *
>diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h
>index 16d787f3fc..e18029ab48 100644
>--- a/src/driver-nodedev.h
>+++ b/src/driver-nodedev.h
>@@ -79,6 +79,9 @@ typedef virNodeDevicePtr
> const char *xmlDesc,
> unsigned int flags);
>
>+typedef int
>+(*virDrvNodeDeviceUndefine)(virNodeDevicePtr dev);
>+
> typedef int
> (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn,
> virNodeDevicePtr dev,
>@@ -119,4 +122,5 @@ struct _virNodeDeviceDriver {
> virDrvNodeDeviceCreateXML nodeDeviceCreateXML;
> virDrvNodeDeviceDestroy nodeDeviceDestroy;
> virDrvNodeDeviceDefineXML nodeDeviceDefineXML;
>+ virDrvNodeDeviceUndefine nodeDeviceUndefine;
> };
>diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
>index 1704de929e..eb01ceb8b0 100644
>--- a/src/libvirt-nodedev.c
>+++ b/src/libvirt-nodedev.c
>@@ -803,6 +803,42 @@ virNodeDeviceDefineXML(virConnectPtr conn,
> }
>
>
>+/**
>+ * virNodeDeviceUndefine:
>+ * @dev: a device object
>+ *
>+ * Undefine the device object. The virtual device is removed from the host
>+ * operating system. This function may require privileged access.
>+ *
>+ * Returns 0 in case of success and -1 in case of failure.
>+ */
>+int
>+virNodeDeviceUndefine(virNodeDevicePtr dev)
>+{
>+ VIR_DEBUG("dev=%p", dev);
>+
>+ virResetLastError();
>+
>+ virCheckNodeDeviceReturn(dev, -1);
>+ virCheckReadOnlyGoto(dev->conn->flags, error);
>+
>+ if (dev->conn->nodeDeviceDriver &&
>+ dev->conn->nodeDeviceDriver->nodeDeviceUndefine) {
>+ int retval = dev->conn->nodeDeviceDriver->nodeDeviceUndefine(dev);
>+ if (retval < 0)
>+ goto error;
>+
>+ return 0;
>+ }
>+
>+ virReportUnsupportedError();
>+
>+ error:
>+ virDispatchError(dev->conn);
>+ return -1;
>+}
>+
>+
> /**
> * virConnectNodeDeviceEventRegisterAny:
> * @conn: pointer to the connection
>diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
>index 27c637e37c..35c88ec4b7 100644
>--- a/src/libvirt_public.syms
>+++ b/src/libvirt_public.syms
>@@ -876,5 +876,6 @@ LIBVIRT_6.0.0 {
> LIBVIRT_6.5.0 {
> global:
> virNodeDeviceDefineXML;
>+ virNodeDeviceUndefine;
> } LIBVIRT_6.0.0;
> # .... define new API here using predicted next version number ....
>diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
>index 16f3537776..60a360e4b5 100644
>--- a/src/node_device/node_device_driver.c
>+++ b/src/node_device/node_device_driver.c
>@@ -833,6 +833,17 @@ nodeDeviceGetMdevctlStopCommand(const char *uuid)
>
> }
>
>+virCommandPtr
>+nodeDeviceGetMdevctlUndefineCommand(const char *uuid)
>+{
>+ return virCommandNewArgList(MDEVCTL,
>+ "undefine",
>+ "-u",
>+ uuid,
>+ NULL);
>+
>+}
>+
> static int
> virMdevctlStop(virNodeDeviceDefPtr def)
> {
>@@ -848,6 +859,21 @@ virMdevctlStop(virNodeDeviceDefPtr def)
> }
>
>
>+static int
>+virMdevctlUndefine(virNodeDeviceDefPtr def)
>+{
>+ int status;
>+ g_autoptr(virCommand) cmd = NULL;
>+
>+ cmd = nodeDeviceGetMdevctlUndefineCommand(def->caps->data.mdev.uuid);
>+
>+ if (virCommandRun(cmd, &status) < 0 || status != 0)
>+ return -1;
>+
>+ return 0;
>+}
>+
>+
> virCommandPtr
> nodeDeviceGetMdevctlListCommand(bool defined,
> char **output)
>@@ -1093,6 +1119,48 @@ nodeDeviceDefineXML(virConnectPtr conn,
> }
>
>
>+int
>+nodeDeviceUndefine(virNodeDevicePtr device)
>+{
>+ int ret = -1;
>+ virNodeDeviceObjPtr obj = NULL;
>+ virNodeDeviceDefPtr def;
>+ g_autofree char *parent = NULL;
>+
../src/node_device/node_device_driver.c:1128:22: error: unused variable 'parent' [-Werror,-Wunused-variable]
g_autofree char *parent = NULL;
^
>+ if (nodeDeviceWaitInit() < 0)
>+ return -1;
>+
>+ if (!(obj = nodeDeviceObjFindByName(device->name)))
>+ return -1;
>+ def = virNodeDeviceObjGetDef(obj);
>+
>+ if (virNodeDeviceUndefineEnsureACL(device->conn, def) < 0)
>+ goto cleanup;
>+
An incremental build fails for me here:
../src/node_device/node_device_driver.c:1137:9: error: implicit declaration of function 'virNodeDeviceUndefineEnsureACL' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
if (virNodeDeviceUndefineEnsureACL(device->conn, def) < 0)
^
../src/node_device/node_device_driver.c:1137:9: note: did you mean 'virNodeDeviceDefineXMLEnsureACL'?
src/access/viraccessapicheck.h:340:12: note: 'virNodeDeviceDefineXMLEnsureACL' declared here
extern int virNodeDeviceDefineXMLEnsureACL(virConnectPtr conn, virNodeDeviceDefPtr device);
^
Maybe there's a missing dependency in meson.build?
>+ if (virNodeDeviceObjIsActive(obj)) {
>+ virReportError(VIR_ERR_OPERATION_INVALID,
>+ _("node device '%s' is still active"),
>+ def->name);
>+ goto cleanup;
>+ }
>+
>+ if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
>+ if (virMdevctlUndefine(def) < 0) {
>+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+ _("Unable to undefine mediated device"));
>+ goto cleanup;
>+ }
>+ ret = 0;
>+ } else {
>+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>+ _("Unsupported device type"));
>+ }
>+
>+ cleanup:
>+ virNodeDeviceObjEndAPI(&obj);
>+ return ret;
>+}
>+
>
> int
> nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn,
>diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
>index cf03e0b3cf..2adb3a8073 100644
>--- a/src/node_device/node_device_driver.h
>+++ b/src/node_device/node_device_driver.h
>@@ -112,6 +112,9 @@ nodeDeviceDefineXML(virConnectPtr conn,
> const char *xmlDesc,
> unsigned int flags);
>
>+int
>+nodeDeviceUndefine(virNodeDevicePtr dev);
>+
> int
> nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn,
> virNodeDevicePtr dev,
>@@ -134,6 +137,9 @@ nodeDeviceGetMdevctlDefineCommand(virNodeDeviceDefPtr def,
> virCommandPtr
> nodeDeviceGetMdevctlStopCommand(const char *uuid);
>
>+virCommandPtr
>+nodeDeviceGetMdevctlUndefineCommand(const char *uuid);
>+
> virCommandPtr
> nodeDeviceGetMdevctlListCommand(bool defined, char **output);
>
>diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>index 5101fff146..19d6642b21 100644
>--- a/src/node_device/node_device_udev.c
>+++ b/src/node_device/node_device_udev.c
>@@ -2199,6 +2199,7 @@ static virNodeDeviceDriver udevNodeDeviceDriver = {
> .nodeDeviceCreateXML = nodeDeviceCreateXML, /* 0.7.3 */
> .nodeDeviceDestroy = nodeDeviceDestroy, /* 0.7.3 */
> .nodeDeviceDefineXML = nodeDeviceDefineXML, /* 6.5.0 */
>+ .nodeDeviceUndefine = nodeDeviceUndefine, /* 6.5.0 */
> };
>
>
>diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>index 64d0d0c49b..fc27278c4d 100644
>--- a/src/remote/remote_driver.c
>+++ b/src/remote/remote_driver.c
>@@ -8815,6 +8815,7 @@ static virNodeDeviceDriver node_device_driver = {
> .nodeDeviceListCaps = remoteNodeDeviceListCaps, /* 0.5.0 */
> .nodeDeviceCreateXML = remoteNodeDeviceCreateXML, /* 0.6.3 */
> .nodeDeviceDefineXML = remoteNodeDeviceDefineXML, /* 6.5.0 */
>+ .nodeDeviceUndefine = remoteNodeDeviceUndefine, /* 6.5.0 */
> .nodeDeviceDestroy = remoteNodeDeviceDestroy /* 0.6.3 */
> };
>
>diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
>index ddfdb9c084..d668074204 100644
>--- a/src/remote/remote_protocol.x
>+++ b/src/remote/remote_protocol.x
>@@ -2148,6 +2148,10 @@ struct remote_node_device_define_xml_ret {
> remote_nonnull_node_device dev;
> };
>
>+struct remote_node_device_undefine_args {
>+ remote_nonnull_string name;
>+};
>+
>
> /*
> * Events Register/Deregister:
>@@ -6680,5 +6684,13 @@ enum remote_procedure {
> * @acl: node_device:write
> * @acl: node_device:start
> */
>- REMOTE_PROC_NODE_DEVICE_DEFINE_XML = 423
>+ REMOTE_PROC_NODE_DEVICE_DEFINE_XML = 423,
>+
>+ /**
>+ * @generate: both
>+ * @priority: high
>+ * @acl: node_device:stop
For domains, we use domain:delete as the acl for undefine
>+ */
>+ REMOTE_PROC_NODE_DEVICE_UNDEFINE = 424
>+
> };
>diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
>index c325923eaf..3d450a6de9 100644
>--- a/src/remote_protocol-structs
>+++ b/src/remote_protocol-structs
>@@ -1607,6 +1607,9 @@ struct remote_node_device_define_xml_args {
> struct remote_node_device_define_xml_ret {
> remote_nonnull_node_device dev;
> };
>+struct remote_node_device_undefine_args {
>+ remote_nonnull_string name;
>+};
> struct remote_connect_domain_event_register_ret {
> int cb_registered;
> };
>@@ -3566,4 +3569,5 @@ enum remote_procedure {
> REMOTE_PROC_DOMAIN_BACKUP_BEGIN = 421,
> REMOTE_PROC_DOMAIN_BACKUP_GET_XML_DESC = 422,
> REMOTE_PROC_NODE_DEVICE_DEFINE_XML = 423,
>+ REMOTE_PROC_NODE_DEVICE_UNDEFINE = 424,
> };
>diff --git a/tests/nodedevmdevctldata/mdevctl-undefine.argv b/tests/nodedevmdevctldata/mdevctl-undefine.argv
>new file mode 100644
>index 0000000000..54717455f7
>--- /dev/null
>+++ b/tests/nodedevmdevctldata/mdevctl-undefine.argv
>@@ -0,0 +1 @@
>+$MDEVCTL_BINARY$ undefine -u d76a6b78-45ed-4149-a325-005f9abc5281
>diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
>index f821622ff6..216c70f1b8 100644
>--- a/tests/nodedevmdevctltest.c
>+++ b/tests/nodedevmdevctltest.c
>@@ -12,7 +12,9 @@
>
> typedef enum {
> MDEVCTL_CMD_START,
>+ MDEVCTL_CMD_STOP,
> MDEVCTL_CMD_DEFINE,
>+ MDEVCTL_CMD_UNDEFINE
> } MdevctlCmd;
>
> struct startTestInfo {
>@@ -134,19 +136,21 @@ testMdevctlStartHelper(const void *data)
> mdevxml, cmdlinefile, jsonfile);
> }
>
>+typedef virCommandPtr (*GetStopUndefineCmdFunc)(const char *uuid);
>+struct UuidCommandTestInfo {
>+ const char *uuid;
>+ MdevctlCmd command;
>+};
>+
> static int
>-testMdevctlStop(const void *data)
>+testMdevctlUuidCommand(const char *uuid, GetStopUndefineCmdFunc func, const char *outfile)
> {
>- const char *uuid = data;
> g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> const char *actualCmdline = NULL;
> int ret = -1;
> g_autoptr(virCommand) cmd = NULL;
>- g_autofree char *cmdlinefile =
>- g_strdup_printf("%s/nodedevmdevctldata/mdevctl-stop.argv",
>- abs_srcdir);
>
>- cmd = nodeDeviceGetMdevctlStopCommand(uuid);
>+ cmd = func(uuid);
>
> if (!cmd)
> goto cleanup;
>@@ -158,7 +162,7 @@ testMdevctlStop(const void *data)
> if (!(actualCmdline = virBufferCurrentContent(&buf)))
> goto cleanup;
>
>- if (nodedevCompareToFile(actualCmdline, cmdlinefile) < 0)
>+ if (nodedevCompareToFile(actualCmdline, outfile) < 0)
> goto cleanup;
>
> ret = 0;
>@@ -168,6 +172,30 @@ testMdevctlStop(const void *data)
> return ret;
> }
>
>+static int
>+testMdevctlUuidCommandHelper(const void *data)
Please separate the test refactors from the API addition.
Jano
>+{
>+ const struct UuidCommandTestInfo *info = data;
>+ GetStopUndefineCmdFunc func;
>+ const char *cmd;
>+ g_autofree char *cmdlinefile = NULL;
>+
>+ if (info->command == MDEVCTL_CMD_STOP) {
>+ cmd = "stop";
>+ func = nodeDeviceGetMdevctlStopCommand;
>+ }else if (info->command == MDEVCTL_CMD_UNDEFINE) {
>+ cmd = "undefine";
>+ func = nodeDeviceGetMdevctlUndefineCommand;
>+ } else {
>+ return -1;
>+ }
>+
>+ cmdlinefile = g_strdup_printf("%s/nodedevmdevctldata/mdevctl-%s.argv",
>+ abs_srcdir, cmd);
>+
>+ return testMdevctlUuidCommand(info->uuid, func, cmdlinefile);
>+}
>+
> static int
> testMdevctlListDefined(const void *data G_GNUC_UNUSED)
> {
>@@ -391,8 +419,18 @@ mymain(void)
> #define DO_TEST_DEFINE(filename) \
> DO_TEST_START_FULL("mdevctl define " filename, "QEMU", CREATE_DEVICE, filename, MDEVCTL_CMD_DEFINE)
>
>+#define DO_TEST_UUID_COMMAND_FULL(desc, uuid, command) \
>+ do { \
>+ struct UuidCommandTestInfo info = { uuid, command }; \
>+ DO_TEST_FULL(desc, testMdevctlUuidCommandHelper, info); \
>+ } \
>+ while (0)
>+
> #define DO_TEST_STOP(uuid) \
>- DO_TEST_FULL("mdevctl stop " uuid, testMdevctlStop, uuid)
>+ DO_TEST_UUID_COMMAND_FULL("mdevctl stop " uuid, uuid, MDEVCTL_CMD_STOP)
>+
>+#define DO_TEST_UNDEFINE(uuid) \
>+ DO_TEST_UUID_COMMAND_FULL("mdevctl undefine " uuid, uuid, MDEVCTL_CMD_UNDEFINE)
>
> #define DO_TEST_LIST_DEFINED() \
> do { \
>@@ -422,6 +460,8 @@ mymain(void)
> DO_TEST_DEFINE("mdev_fedc4916_1ca8_49ac_b176_871d16c13076");
> DO_TEST_DEFINE("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9");
>
>+ DO_TEST_UNDEFINE("d76a6b78-45ed-4149-a325-005f9abc5281");
>+
> done:
> nodedevTestDriverFree(driver);
>
>--
>2.26.2
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200825/85759b0e/attachment-0001.sig>
More information about the libvir-list
mailing list