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

Re: [libvirt] [PATCH 4/4 v5] virsh: Use virNodeDeviceLookupSCSIHostByWWN



On 2013年02月11日 21:33, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 10:16:44PM +0800, Osier Yang wrote:
Only nodedev-destroy and nodedev-dumpxml can benifit from the
new API, other commands like nodedev-detach only works for
PCI devices, WWN makes no sense for them.

---
Rebased on Peter's virsh cleanup patches.
---
  tools/virsh-nodedev.c |   84 +++++++++++++++++++++++++++++++++++++++---------
  tools/virsh.pod       |   15 +++++---
  2 files changed, 77 insertions(+), 22 deletions(-)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index f85bded..7c51a56 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -101,9 +101,14 @@ static const vshCmdInfo info_node_device_destroy[] = {

  static const vshCmdOptDef opts_node_device_destroy[] = {
      {.name = "name",
+     .type = VSH_OT_ALIAS,
+     .flags = 0,
+     .help = "device"
+    },
+    {.name = "device",
       .type = VSH_OT_DATA,
       .flags = VSH_OFLAG_REQ,
-     .help = N_("name of the device to be destroyed")
+     .help = N_("device name or wwn pair in 'wwnn,wwpn' format")
      },
      {.name = NULL}
  };
@@ -112,21 +117,47 @@ static bool
  cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
  {
      virNodeDevicePtr dev = NULL;
-    bool ret = true;
-    const char *name = NULL;
+    bool ret = false;
+    const char *device_value = NULL;
+    char **arr = NULL;
+    int narr;

-    if (vshCommandOptStringReq(ctl, cmd, "name",&name)<  0)
+    if (vshCommandOptStringReq(ctl, cmd, "device",&device_value)<  0)
          return false;

-    dev = virNodeDeviceLookupByName(ctl->conn, name);
+    if (strchr(device_value, ',')) {
+        narr = vshStringToArray(device_value,&arr);
+        if (narr != 2) {
+            vshError(ctl, _("Malformed device value '%s'"), device_value);
+            goto cleanup;
+        }
+
+        if (!virValidateWWN(arr[0]) || !virValidateWWN(arr[1]))
+            goto cleanup;
+
+        dev = virNodeDeviceLookupSCSIHostByWWN(ctl->conn, arr[0], arr[1], 0);
+    } else {
+        dev = virNodeDeviceLookupByName(ctl->conn, device_value);
+    }
+
+    if (!dev) {
+        vshError(ctl, "%s '%s'", _("Could not find matching device"), device_value);
+        goto cleanup;
+    }

      if (virNodeDeviceDestroy(dev) == 0) {
-        vshPrint(ctl, _("Destroyed node device '%s'\n"), name);
+        vshPrint(ctl, _("Destroyed node device '%s'\n"), device_value);
      } else {
-        vshError(ctl, _("Failed to destroy node device '%s'"), name);
-        ret = false;
+        vshError(ctl, _("Failed to destroy node device '%s'"), device_value);
+        goto cleanup;
      }

+    ret = true;
+cleanup:
+    if (arr) {
+        VIR_FREE(*arr);

Hmm, IIUC, that is equiv to VIR_FREE(arr[0]), so what is
free'ing arr[1] ?

vshStringToArray substitutes the delimiter ',' with '\0',
and the elements simply point to the pieces. So freeing
the first element frees all the memory of the array elements.


+        VIR_FREE(arr);
+    }


ACK if that leak is fixed, or otherwise explained.


Thanks for the reviewing, I pushed this set.

Osier


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