Re: [PATCH v2 10/11] nodedev: Implement virNodeDeviceUpdateXML

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

 



On 2/13/24 3:04 AM, Boris Fiuczynski wrote:
On 2/9/24 23:45, Jonathon Jongsma wrote:
On 2/7/24 7:39 AM, Boris Fiuczynski wrote:
Implement the API functions in the node device driver by using
mdevctl modify with the options defined and live.
Instead of increasing the minimum mdevctl version to 1.3.0 in spec file
to ensure support exists in mdevctl the support is dynamically checked
before using mdevctl.

Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
---
  NEWS.rst                                      |   7 +
  libvirt.spec.in                               |   1 +
  src/node_device/node_device_driver.c          | 256 +++++++++++++++++-
  src/node_device/node_device_driver.h          |  11 +
  src/node_device/node_device_udev.c            |   1 +
  ...60c_c60c_c60c_c60c_c60cc60cc60c_update.xml |  16 ++
  tests/nodedevmdevctldata/mdevctl-modify.argv  |  25 ++
  tests/nodedevmdevctldata/mdevctl-modify.json  |   1 +
  tests/nodedevmdevctltest.c                    |  90 +++++-
  9 files changed, 405 insertions(+), 3 deletions(-)
  create mode 100644 tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml
  create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.argv
  create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.json

diff --git a/NEWS.rst b/NEWS.rst
index e2796fd8b2..aa2fee3533 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -17,6 +17,13 @@ v10.1.0 (unreleased)
  * **New features**
+  * nodedev: Support updating mdevs
+
+    The node device driver has been extended to allow updating mediated node +    devices. Options are available to target the update against the persisted,
+    active or both configurations of an mediated device.
+    **Note:** The support is only available with at least mdevctl v1.3.0 installed.
+
    * qemu: Support clusters in CPU topology
      It is now possible to configure the guest CPU topology to use clusters.
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8413e3c19a..a8dff4a32e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -613,6 +613,7 @@ Requires: libvirt-libs = %{version}-%{release}
  # needed for device enumeration
  Requires: systemd >= 185
  # For managing persistent mediated devices
+# Note: for nodedev-update support at least mdevctl v1.3.0 is required
  Requires: mdevctl
  # for modprobe of pci devices
  Requires: module-init-tools
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index baff49a0ae..5bece1a30a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -54,7 +54,7 @@ virNodeDeviceDriverState *driver;
  VIR_ENUM_IMPL(virMdevctlCommand,
                MDEVCTL_CMD_LAST,
-              "start", "stop", "define", "undefine", "create"
+              "start", "stop", "define", "undefine", "create", "modify"
  );
@@ -754,6 +754,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
      case MDEVCTL_CMD_START:
      case MDEVCTL_CMD_DEFINE:
      case MDEVCTL_CMD_UNDEFINE:
+    case MDEVCTL_CMD_MODIFY:
          cmd = virCommandNewArgList(MDEVCTL, subcommand, NULL);
          break;
      case MDEVCTL_CMD_LAST:
@@ -767,6 +768,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
      switch (cmd_type) {
      case MDEVCTL_CMD_CREATE:
      case MDEVCTL_CMD_DEFINE:
+    case MDEVCTL_CMD_MODIFY:
          if (!def->caps->data.mdev.parent_addr) {
              virReportError(VIR_ERR_INTERNAL_ERROR,
                             _("unable to find parent device '%1$s'"), def->parent);
@@ -783,7 +785,8 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
          virCommandAddArgPair(cmd, "--jsonfile", "/dev/stdin");
          virCommandSetInputBuffer(cmd, inbuf);
-        virCommandSetOutputBuffer(cmd, outbuf);
+        if (outbuf)
+            virCommandSetOutputBuffer(cmd, outbuf);
          break;
      case MDEVCTL_CMD_UNDEFINE:
@@ -868,6 +871,100 @@ virMdevctlDefine(virNodeDeviceDef *def, char **uuid)
  }
+/* gets a virCommand object that executes a mdevctl command to modify a
+ * a device to an updated version
+ */
+virCommand*
+nodeDeviceGetMdevctlModifyCommand(virNodeDeviceDef *def,
+                                  bool defined,
+                                  bool live,
+                                  char **errmsg)
+{
+    virCommand *cmd = nodeDeviceGetMdevctlCommand(def,
+                                                  MDEVCTL_CMD_MODIFY,
+                                                  NULL, errmsg);
+
+    if (!cmd)
+        return NULL;
+
+    if (defined)
+        virCommandAddArg(cmd, "--defined");
+
+    if (live)
+        virCommandAddArg(cmd, "--live");
+
+    return cmd;
+}
+
+
+/* checks if mdevctl supports on the command modify the options live, defined
+ * and jsonfile
+ */
+static int
+nodeDeviceGetMdevctlModifySupportCheck(void)
+{
+    int status;
+    g_autoptr(virCommand) cmd = NULL;
+    const char *subcommand = virMdevctlCommandTypeToString(MDEVCTL_CMD_MODIFY);
+
+    cmd = virCommandNewArgList(MDEVCTL,
+                               subcommand,
+                               "--defined",
+                               "--live",
+                               "--jsonfile",
+                               "b",
+                               "--help",
+                               NULL);
+
+    if (!cmd)
+        return -1;
+
+    if (virCommandRun(cmd, &status) < 0)
+        return -1;
+
+    if (status != 0) // update is unsupported
+        return -1;
+
+    return 0;
+}
+
+
+static int
+virMdevctlModify(virNodeDeviceDef *def,
+                 bool defined,
+                 bool live)
+{
+    int status;
+    g_autofree char *errmsg = NULL;
+    g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlModifyCommand(def,
+ defined,
+                                                                  live,
+ &errmsg);
+
+    if (!cmd)
+        return -1;
+
+    if (nodeDeviceGetMdevctlModifySupportCheck() < 0) {
+        VIR_WARN("Installed mdevctl version does not support modify with options jsonfile, defined and live");
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("Unable to modify mediated device: modify unsupported"));
+        return -1;
+    }
+
+    if (virCommandRun(cmd, &status) < 0)
+        return -1;
+
+    if (status != 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to modify mediated device: %1$s"),
+                       MDEVCTL_ERROR(errmsg));
+        return -1;
+    }
+
+    return 0;
+}
+
+
  static virNodeDevicePtr
  nodeDeviceCreateXMLMdev(virConnectPtr conn,
                          virNodeDeviceDef *def)
@@ -2106,3 +2203,158 @@ nodeDeviceIsActive(virNodeDevice *device)
      virNodeDeviceObjEndAPI(&obj);
      return ret;
  }
+
+
+static int
+nodeDeviceDefCloneMdevConfigs(virNodeDeviceDef *def_upd)
+{
+    virNodeDevCapsDef *caps = NULL;
+    virNodeDevCapMdev *cap_mdev_upd = NULL;
+    virMediatedDeviceConfig *src_mdev_config = NULL;
+    virMediatedDeviceConfig *dst_mdev_config = NULL;
+
+    for (caps = def_upd->caps; caps != NULL; caps = caps->next) {
+        if (caps->data.type == VIR_NODE_DEV_CAP_MDEV) {
+            cap_mdev_upd = &caps->data.mdev;
+        }
+    }
+    if (!cap_mdev_upd)
+        return -1;
+
+    src_mdev_config = &cap_mdev_upd->defined_config;
+    dst_mdev_config = &cap_mdev_upd->active_config;
+
+    if (STRNEQ_NULLABLE(dst_mdev_config->type, src_mdev_config->type)) {
+        g_free(dst_mdev_config->type);
+        dst_mdev_config->type = g_strdup(src_mdev_config->type);
+    }
+
+    virMediatedDeviceAttrsCopy(dst_mdev_config, src_mdev_config);
+
+    return 0;
+}
+
+

I'm making a couple additional suggestions here relevant to our discussion below.


+static int
+nodeDeviceDefCompareMdevs(virNodeDeviceDef *def_cur,
+                          virNodeDeviceDef *def_upd,
+                          bool live)

I think this function is poorly named. It implies that you are comparing two different mdevs for equality, but it's really a function that validates whether it is valid to update from one device def to a new def. So it would be much more understandable if it was named something like nodeDeviceDefValidateUpdate().

In addition, I'd personally appreciate less cryptic variable names. For example, `def_cur` could just be `def` or `current_def`. `def_upd` could be `new_def`. Similar for the local variables below.

+{
+    virNodeDevCapsDef *caps = NULL;
+    virNodeDevCapMdev *cap_mdev_cur = NULL;
+    virNodeDevCapMdev *cap_mdev_upd = NULL;
+
+    for (caps = def_cur->caps; caps != NULL; caps = caps->next) {
+        if (caps->data.type == VIR_NODE_DEV_CAP_MDEV) {
+            cap_mdev_cur = &caps->data.mdev;
+        }
+    }
+    if (!cap_mdev_cur)
+        return -1;
+
+    for (caps = def_upd->caps; caps != NULL; caps = caps->next) {
+        if (caps->data.type == VIR_NODE_DEV_CAP_MDEV) {
+            cap_mdev_upd = &caps->data.mdev;
+        }
+    }
+    if (!cap_mdev_upd)
+        return -1;
+
+    if (STRNEQ_NULLABLE(cap_mdev_cur->uuid, cap_mdev_upd->uuid)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Cannot update device '%1$s, uuid mismatch (current uuid '%2$s')"),
+                       def_cur->name,
+                       cap_mdev_cur->uuid);
+        return -1;
+    }
+    // for a live update the types of the active configs must match!

Related to my suggestion below, you could change the comment here to something like:

/* A live update cannot change the mdev type. Since the new config is
 * stored in defined_config, compare that to the mdev type of the
 * current live config to make sure they match
 */

Then it would be clear why you're comparing the current active config to the new defined config.

+    if (live &&
+        STRNEQ_NULLABLE(cap_mdev_cur->active_config.type, cap_mdev_upd->active_config.type)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Cannot update device '%1$s', type mismatch (current type '%2$s')"),
+                       def_cur->name,
+                       cap_mdev_cur->active_config.type);
+        return -1;
+    }
+    if (STRNEQ_NULLABLE(cap_mdev_cur->parent_addr, cap_mdev_upd->parent_addr)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Cannot update device '%1$s', parent address mismatch (current parent address '%2$s')"),
+                       def_cur->name,
+                       cap_mdev_cur->parent_addr);
+        return -1;
+    }
+
+    return 0;
+}
+
+
+int
+nodeDeviceUpdateXML(virNodeDevice *device,
+                    const char *xmlDesc,
+                    unsigned int flags)
+{
+    int ret = -1;
+    virNodeDeviceObj *obj = NULL;
+    virNodeDeviceDef *def_cur;
+    g_autoptr(virNodeDeviceDef) def_upd = NULL

^ another case where less cryptic variable names would be nice.

+    bool updated = false;
+
+    virCheckFlags(VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE |
+                  VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG, -1);
+
+    if (nodeDeviceInitWait() < 0)
+        return -1;
+
+    if (!(obj = nodeDeviceObjFindByName(device->name)))
+        return -1;
+    def_cur = virNodeDeviceObjGetDef(obj);
+
+    virt_type = virConnectGetType(device->conn);
+
+    if (virNodeDeviceUpdateXMLEnsureACL(device->conn, def_cur, flags) < 0)
+        goto cleanup;
+
+    if (!(def_upd = virNodeDeviceDefParse(xmlDesc, NULL, EXISTING_DEVICE, virt_type, +                                          &driver->parserCallbacks, NULL, true)))
+        goto cleanup;
+
+    if (nodeDeviceHasCapability(def_cur, VIR_NODE_DEV_CAP_MDEV) &&
+        nodeDeviceHasCapability(def_upd, VIR_NODE_DEV_CAP_MDEV)) {
+        // Checks flags are valid for the state and sets flags for current if flags not set
+        if (virNodeDeviceObjUpdateModificationImpact(obj, &flags) < 0)
+            goto cleanup;
+
+        // Before live update clone defined to active config
+        if (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE &&
+            nodeDeviceDefCloneMdevConfigs(def_upd) < 0)
+            goto cleanup;

It's not clear to me why this is necessary. As far as I can tell, only the defined_config is used to generate the JSON to send to mdevctl. And this def is freed at the end of the function and then we re-query mdevctl to update our device list.

In case a live update is to be made there is a check of the mdev types also on the active config. Therefore I try to match the def used for the update to the current def by cloning the defined_config into the active_config. Another solution would have been to change the compare in
nodeDeviceDefCloneMdevConfigs from
STRNEQ_NULLABLE(cap_mdev_cur->active_config.type, cap_mdev_upd->active_config.type)
into
STRNEQ_NULLABLE(cap_mdev_cur->active_config.type, cap_mdev_upd->defined_config.type) which seems kind of wrong to me but I am willing to suffer if you disagree... :D

I assume you mean the function nodeDeviceDefCompareMdevs(). I do think that would be a better approach, actually. It does look a bit weird, but with a clear comment in the function (suggestion above), I think it's more understandable than the alternative of calling one function to copy the persistent config into the active config only for the purposes of checking it in another function.

Jonathon




+
+        // Compare def_cur and def_upd for compatibleness e.g. parent, type and uuid

compatibleness -> compatibility

Changed.


+        if (nodeDeviceDefCompareMdevs(def_cur, def_upd,
+                                      (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE)) < 0)
+            goto cleanup;
+
+        // Update now
+        VIR_DEBUG("Update node device '%s' with mdevctl", def_cur->name);
+        if (virMdevctlModify(def_upd,
+                             (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG), +                             (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE)) < 0) {
+            goto cleanup;
+        };
+        // Detect updates and also trigger events
+        updated = true;
+    } else {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Unsupported device type"));
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virNodeDeviceObjEndAPI(&obj);
+    if (updated)
+        nodeDeviceUpdateMediatedDevices();
+
+    return ret;
+}
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index 4dce7e6f17..5751625eda 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -39,6 +39,7 @@ typedef enum {
       * separation makes our code more readable in terms of knowing when we're        * starting a defined device and when we're creating a transient one */
      MDEVCTL_CMD_CREATE,
+    MDEVCTL_CMD_MODIFY,
      MDEVCTL_CMD_LAST,
  } virMdevctlCommand;
@@ -186,3 +187,13 @@ virCommand*
  nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def,
                                          bool autostart,
                                          char **errmsg);
+
+virCommand*
+nodeDeviceGetMdevctlModifyCommand(virNodeDeviceDef *def,
+                                  bool defined,
+                                  bool live,
+                                  char **errmsg);
+int
+nodeDeviceUpdateXML(virNodeDevice *dev,
+                    const char *xmlDesc,
+                    unsigned int flags);
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 57368a96c3..4cd9d285fa 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -2403,6 +2403,7 @@ static virNodeDeviceDriver udevNodeDeviceDriver = {
      .nodeDeviceGetAutostart = nodeDeviceGetAutostart, /* 7.8.0 */
      .nodeDeviceIsPersistent = nodeDeviceIsPersistent, /* 7.8.0 */
      .nodeDeviceIsActive = nodeDeviceIsActive, /* 7.8.0 */
+    .nodeDeviceUpdateXML = nodeDeviceUpdateXML, /* 10.1.0 */
  };
diff --git a/tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml b/tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml
new file mode 100644
index 0000000000..17e3611bf4
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml
@@ -0,0 +1,16 @@
+<device>
+  <name>mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0052</name>
+ <path>/sys/devices/css0/0.0.0052/c60cc60c-c60c-c60c-c60c-c60cc60cc60c</path>
+  <parent>css_0_0_0052</parent>
+  <driver>
+    <name>vfio_ccw_mdev</name>
+  </driver>
+  <capability type='mdev'>
+    <type id='vfio_ccw-io'/>
+    <uuid>c60cc60c-c60c-c60c-c60c-c60cc60cc60c</uuid>
+    <parent_addr>0.0.0052</parent_addr>
+    <attr name='add_attr_1' value='val1'/>
+    <attr name='add_attr_2' value='val2'/>
+    <iommuGroup number='4'/>
+  </capability>
+</device>
diff --git a/tests/nodedevmdevctldata/mdevctl-modify.argv b/tests/nodedevmdevctldata/mdevctl-modify.argv
new file mode 100644
index 0000000000..341c5e7fd0
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdevctl-modify.argv
@@ -0,0 +1,25 @@
+mdevctl \
+modify \
+--parent=0.0.0052 \
+--jsonfile=/dev/stdin \
+--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \
+--defined
+mdevctl \
+modify \
+--parent=0.0.0052 \
+--jsonfile=/dev/stdin \
+--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \
+--live
+mdevctl \
+modify \
+--parent=0.0.0052 \
+--jsonfile=/dev/stdin \
+--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \
+--live
+mdevctl \
+modify \
+--parent=0.0.0052 \
+--jsonfile=/dev/stdin \
+--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \
+--defined \
+--live
diff --git a/tests/nodedevmdevctldata/mdevctl-modify.json b/tests/nodedevmdevctldata/mdevctl-modify.json
new file mode 100644
index 0000000000..7ae8c73ce3
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdevctl-modify.json
@@ -0,0 +1 @@
+{"mdev_type":"vfio_ccw-io","start":"manual"}{"mdev_type":"vfio_ccw-io","start":"manual","attrs":[{"add_attr_1":"val1"},{"add_attr_2":"val2"}]}{"mdev_type":"vfio_ccw-io","start":"manual"}{"mdev_type":"vfio_ccw-io","start":"manual","attrs":[{"add_attr_1":"val1"},{"add_attr_2":"val2"}]}
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index f49d668461..11273e3049 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -33,7 +33,10 @@ testCommandDryRunCallback(const char *const*args G_GNUC_UNUSED,
  {
      char **stdinbuf = opaque;
-    *stdinbuf = g_strdup(input);
+    if (*stdinbuf)
+        *stdinbuf = g_strconcat(*stdinbuf, input, NULL);

I think g_strjoin() would be nicer so we can add a newline separator between invocations.

There is no problem adding a newline seperator with the g_strconcat as well. Using g_strconcat kind of looks more natural to me

*stdinbuf = g_strconcat(*stdinbuf, "\n", input, NULL);

versus

*stdinbuf = g_strjoin("\n", *stdinbuf, input, NULL);


+    else
+        *stdinbuf = g_strdup(input);
  }
  typedef virCommand * (*MdevctlCmdFunc)(virNodeDeviceDef *, char **, char **);
@@ -63,6 +66,7 @@ testMdevctlCmd(virMdevctlCommand cmd_type,
          case MDEVCTL_CMD_START:
          case MDEVCTL_CMD_STOP:
          case MDEVCTL_CMD_UNDEFINE:
+        case MDEVCTL_CMD_MODIFY:
              create = EXISTING_DEVICE;
              break;
          case MDEVCTL_CMD_LAST:
@@ -173,6 +177,85 @@ testMdevctlAutostart(const void *data G_GNUC_UNUSED)
      return ret;
  }
+
+static int
+testMdevctlModify(const void *data G_GNUC_UNUSED)
+{
+    g_autoptr(virNodeDeviceDef) def = NULL;
+    g_autoptr(virNodeDeviceDef) def_update = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    const char *actualCmdline = NULL;
+    int ret = -1;
+    g_autoptr(virCommand) definedcmd = NULL;
+    g_autoptr(virCommand) livecmd = NULL;
+    g_autoptr(virCommand) bothcmd = NULL;
+    g_autofree char *errmsg = NULL;
+    g_autofree char *stdinbuf = NULL;
+    g_autofree char *mdevxml =
+ g_strdup_printf("%s/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml",
+                        abs_srcdir);
+    g_autofree char *mdevxml_update =
+ g_strdup_printf("%s/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml",
+                        abs_srcdir);
+    /* just concatenate both calls into the same output files */
+    g_autofree char *cmdlinefile =
+        g_strdup_printf("%s/nodedevmdevctldata/mdevctl-modify.argv",
+                        abs_srcdir);
+    g_autofree char *jsonfile =
+        g_strdup_printf("%s/nodedevmdevctldata/mdevctl-modify.json",
+                        abs_srcdir);
+    g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew();
+
+    if (!(def = virNodeDeviceDefParse(NULL, mdevxml, CREATE_DEVICE, VIRT_TYPE,
+                                      &parser_callbacks, NULL, false)))
+        return -1;
+
+    virCommandSetDryRun(dryRunToken, &buf, true, true, testCommandDryRunCallback, &stdinbuf);
+
+    if (!(definedcmd = nodeDeviceGetMdevctlModifyCommand(def, true, false, &errmsg)))
+        goto cleanup;
+
+    if (virCommandRun(definedcmd, NULL) < 0)
+        goto cleanup;
+
+    if (!(def_update = virNodeDeviceDefParse(NULL, mdevxml_update, EXISTING_DEVICE, VIRT_TYPE, +                                             &parser_callbacks, NULL, false)))
+        goto cleanup;
+
+    if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def_update, false, true, &errmsg)))
+        goto cleanup;
+
+    if (virCommandRun(livecmd, NULL) < 0)
+        goto cleanup;
+
+    if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def, false, true, &errmsg)))
+        goto cleanup;
+
+    if (virCommandRun(livecmd, NULL) < 0)
+        goto cleanup;
+
+    if (!(bothcmd = nodeDeviceGetMdevctlModifyCommand(def_update, true, true, &errmsg)))
+        goto cleanup;
+
+    if (virCommandRun(bothcmd, NULL) < 0)
+        goto cleanup;
+
+    if (!(actualCmdline = virBufferCurrentContent(&buf)))
+        goto cleanup;
+
+    if (virTestCompareToFileFull(actualCmdline, cmdlinefile, false) < 0)
+        goto cleanup;
+
+    if (virTestCompareToFile(stdinbuf, jsonfile) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    virBufferFreeAndReset(&buf);
+    return ret;
+}
+
  static int
  testMdevctlListDefined(const void *data G_GNUC_UNUSED)
  {
@@ -457,6 +540,9 @@ mymain(void)
  #define DO_TEST_AUTOSTART() \
      DO_TEST_FULL("autostart mdevs", testMdevctlAutostart, NULL)
+#define DO_TEST_MODIFY() \
+    DO_TEST_FULL("modify mdevs", testMdevctlModify, NULL)
+
  #define DO_TEST_PARSE_JSON(filename) \
      DO_TEST_FULL("parse mdevctl json " filename, testMdevctlParse, filename)
@@ -485,6 +571,8 @@ mymain(void)
      DO_TEST_AUTOSTART();
+    DO_TEST_MODIFY();
+
   done:
      nodedevTestDriverFree(driver);

Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>

Thanks.

_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx

_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux