[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt PATCH v2 12/16] api: add virNodeDeviceUndefine()



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 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

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]