On 09/13/12 08:59, Osier Yang wrote:
tools/virsh-nodedev.c:
* vshNodeDeviceSorter to sort node devices by name
* vshNodeDeviceListFree to free the node device objects list.
* vshNodeDeviceListCollect to collect the node device objects, trying
to use new API first, fall back to older APIs if it's not supported.
* Change option --cap to accept multiple capability types.
tools/virsh.pod
* Update document for --cap
v4 - v5:
* Desert the change which cause the subtree is listed for --tree.
* Error out if both --tree and --cap are specified.
* No second error reset when fallback to old APIs.
* Version number fix.
* Some code styles fix.
---
src/libvirt_private.syms | 1 +
tools/virsh-nodedev.c | 302 ++++++++++++++++++++++++++++++++++++++++------
tools/virsh.pod | 8 +-
3 files changed, 271 insertions(+), 40 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8e0fd47..b25a451 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -852,6 +852,7 @@ virPortGroupFindByName;
# node_device_conf.h
+virNodeDevCapTypeFromString;
OK, now I understand this change.
virNodeDevCapTypeToString;
virNodeDevCapsDefFree;
virNodeDeviceAssignDef;
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index e784af1..963464f 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -36,6 +36,7 @@
#include "memory.h"
#include "util.h"
#include "xml.h"
+#include "conf/node_device_conf.h"
/*
* "nodedev-create" command
@@ -138,6 +139,187 @@ vshNodeListLookup(int devid, bool parent, void *opaque)
return arrays->names[devid];
}
+static int
+vshNodeDeviceSorter(const void *a, const void *b)
+{
+ virNodeDevicePtr *na = (virNodeDevicePtr *) a;
+ virNodeDevicePtr *nb = (virNodeDevicePtr *) b;
+
+ if (*na && !*nb)
+ return -1;
+
+ if (!*na)
+ return *nb != NULL;
+
+ return vshStrcasecmp(virNodeDeviceGetName(*na),
+ virNodeDeviceGetName(*nb));
Still missaligned.
+}
+
+struct vshNodeDeviceList {
+ virNodeDevicePtr *devices;
+ size_t ndevices;
+};
+typedef struct vshNodeDeviceList *vshNodeDeviceListPtr;
+
+static void
+vshNodeDeviceListFree(vshNodeDeviceListPtr list)
+{
+ int i;
+
+ if (list && list->ndevices) {
+ for (i = 0; i < list->ndevices; i++) {
+ if (list->devices[i])
+ virNodeDeviceFree(list->devices[i]);
+ }
+ VIR_FREE(list->devices);
+ }
+ VIR_FREE(list);
+}
+
+static vshNodeDeviceListPtr
+vshNodeDeviceListCollect(vshControl *ctl,
+ char **capnames,
+ int ncapnames,
+ unsigned int flags)
+{
+ vshNodeDeviceListPtr list = vshMalloc(ctl, sizeof(*list));
+ int i;
+ int ret;
+ virNodeDevicePtr device;
+ bool success = false;
+ size_t deleted = 0;
+ int ndevices = 0;
+ char **names = NULL;
+
+ /* try the list with flags support (0.10.2 and later) */
+ if ((ret = virConnectListAllNodeDevices(ctl->conn,
+ &list->devices,
+ flags)) >= 0) {
+ list->ndevices = ret;
+ goto finished;
+ }
+
+ /* check if the command is actually supported */
+ if (last_error && last_error->code == VIR_ERR_NO_SUPPORT)
+ goto fallback;
+
+ /* there was an error during the call */
+ vshError(ctl, "%s", _("Failed to list node devices"));
+ goto cleanup;
+
+
+fallback:
+ /* fall back to old method (0.10.1 and older) */
+ vshResetLibvirtError();
+
+ ndevices = virNodeNumOfDevices(ctl->conn, NULL, 0);
+ if (ndevices < 0) {
+ vshError(ctl, "%s", _("Failed to count node devices"));
+ goto cleanup;
+ }
+
+ if (ndevices == 0)
+ return list;
+
+ names = vshMalloc(ctl, sizeof(char *) * ndevices);
+
+ ndevices = virNodeListDevices(ctl->conn, NULL, names, ndevices, 0);
+ if (ndevices < 0) {
+ vshError(ctl, "%s", _("Failed to list node devices"));
+ goto cleanup;
+ }
+
+ list->devices = vshMalloc(ctl, sizeof(virNodeDevicePtr) * (ndevices));
+ list->ndevices = 0;
+
+ /* get the node devices */
+ for (i = 0; i < ndevices ; i++) {
+ if (!(device = virNodeDeviceLookupByName(ctl->conn, names[i])))
+ continue;
+ list->devices[list->ndevices++] = device;
+ }
+
+ /* truncate domains that weren't found */
+ deleted = ndevices - list->ndevices;
+
+ if (!capnames)
+ goto finished;
+
+ /* filter the list if the list was acquired by fallback means */
+ for (i = 0; i < list->ndevices; i++) {
+ char **caps = NULL;
+ int ncaps = 0;
+ bool match = false;
+
+ device = list->devices[i];
+
+ if ((ncaps = virNodeDeviceNumOfCaps(device)) < 0) {
+ vshError(ctl, "%s", _("Failed to get capability numbers "
+ "of the device"));
+ goto cleanup;
+ }
+
+ caps = vshMalloc(ctl, sizeof(char *) * ncaps);
+
+ if ((ncaps = virNodeDeviceListCaps(device, caps, ncaps)) < 0) {
+ vshError(ctl, "%s", _("Failed to get capability names of the device"));
+ VIR_FREE(caps);
+ goto cleanup;
+ }
+
+ /* Check if the device's capability matches with provied
+ * capabilities.
+ */
+ int j, k;
+ for (j = 0; j < ncaps; j++) {
+ for (k = 0; k < ncapnames; k++) {
+ if (STREQ(caps[j], capnames[k])) {
+ match = true;
+ break;
+ }
+ }
+ }
+
+ VIR_FREE(caps);
+
+ if (!match)
+ goto remove_entry;
+
+ /* the device matched all filters, it may stay */
+ continue;
+
+remove_entry:
+ /* the device has to be removed as it failed one of the filters */
+ virNodeDeviceFree(list->devices[i]);
+ list->devices[i] = NULL;
+ deleted++;
+ }
+
+finished:
+ /* sort the list */
+ if (list->devices && list->ndevices)
+ qsort(list->devices, list->ndevices,
+ sizeof(*list->devices), vshNodeDeviceSorter);
+
+ /* truncate the list if filter simulation deleted entries */
+ if (deleted)
+ VIR_SHRINK_N(list->devices, list->ndevices, deleted);
+
+ success = true;
+
+cleanup:
+ for (i = 0; i < ndevices; i++)
+ VIR_FREE(names[i]);
+ VIR_FREE(names);
+
+ if (!success) {
+ vshNodeDeviceListFree(list);
+ list = NULL;
+ }
+
+ return list;
+}
+
/*
* "nodedev-list" command
*/
@@ -149,71 +331,117 @@ static const vshCmdInfo info_node_list_devices[] = {
static const vshCmdOptDef opts_node_list_devices[] = {
{"tree", VSH_OT_BOOL, 0, N_("list devices in a tree")},
- {"cap", VSH_OT_STRING, VSH_OFLAG_NONE, N_("capability name")},
+ {"cap", VSH_OT_STRING, VSH_OFLAG_NONE, N_("capability names, separated by comma")},
{NULL, 0, 0, NULL}
};
static bool
cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
{
- const char *cap = NULL;
- char **devices;
- int num_devices, i;
+ const char *cap_str = NULL;
+ int i;
bool tree = vshCommandOptBool(cmd, "tree");
bool ret = true;
+ unsigned int flags = 0;
+ char **caps = NULL;
+ int ncaps = 0;
+ vshNodeDeviceListPtr list = NULL;
+ int cap_type = -1;
+
+ ignore_value(vshCommandOptString(cmd, "cap", &cap_str));
+
+ if (cap_str) {
+ if (tree) {
+ vshError(ctl, "%s", _("Options --tree and --cap are incompatible"));
+ return -1;
The function has type bool, so you should "return false" here.
+ }
+ ncaps = vshStringToArray(cap_str, &caps);
+ }
- if (vshCommandOptString(cmd, "cap", &cap) <= 0)
- cap = NULL;
+ for (i = 0; i < ncaps; i++) {
This loop can be included into the if (caps_str) block above as ncaps
will never be non-zero if that block didn't fire.
+ if ((cap_type = virNodeDevCapTypeFromString(caps[i])) < 0) {
+ vshError(ctl, "%s", _("Invalid capability type"));
+ VIR_FREE(caps);
+ return false;
+ }
- num_devices = virNodeNumOfDevices(ctl->conn, cap, 0);
- if (num_devices < 0) {
- vshError(ctl, "%s", _("Failed to count node devices"));
- return false;
- } else if (num_devices == 0) {
- return true;
+ switch(cap_type) {
+ case VIR_NODE_DEV_CAP_SYSTEM:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM;
+ break;
+ case VIR_NODE_DEV_CAP_PCI_DEV:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV;
+ break;
+ case VIR_NODE_DEV_CAP_USB_DEV:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV;
+ break;
+ case VIR_NODE_DEV_CAP_USB_INTERFACE:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_INTERFACE;
+ break;
+ case VIR_NODE_DEV_CAP_NET:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET;
+ break;
+ case VIR_NODE_DEV_CAP_SCSI_HOST:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST;
+ break;
+ case VIR_NODE_DEV_CAP_SCSI_TARGET:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET;
+ break;
+ case VIR_NODE_DEV_CAP_SCSI:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI;
+ break;
+ case VIR_NODE_DEV_CAP_STORAGE:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE;
+ break;
+ default:
+ break;
+ }
}
- devices = vshMalloc(ctl, sizeof(char *) * num_devices);
- num_devices =
- virNodeListDevices(ctl->conn, cap, devices, num_devices, 0);
- if (num_devices < 0) {
- vshError(ctl, "%s", _("Failed to list node devices"));
- VIR_FREE(devices);
- return false;
+ if (!(list = vshNodeDeviceListCollect(ctl, caps, ncaps, flags))) {
+ ret = false;
+ goto cleanup;
}
- qsort(&devices[0], num_devices, sizeof(char*), vshNameSorter);
+
if (tree) {
- char **parents = vshMalloc(ctl, sizeof(char *) * num_devices);
- struct vshNodeList arrays = { devices, parents };
+ char **parents = vshMalloc(ctl, sizeof(char *) * list->ndevices);
+ char **names = vshMalloc(ctl, sizeof(char *) * list->ndevices);
+ struct vshNodeList arrays = { names, parents };
- for (i = 0; i < num_devices; i++) {
- virNodeDevicePtr dev = virNodeDeviceLookupByName(ctl->conn, devices[i]);
- if (dev && STRNEQ(devices[i], "computer")) {
+ for (i = 0; i < list->ndevices; i++)
+ names[i] = vshStrdup(ctl, virNodeDeviceGetName(list->devices[i]));
+
+ for (i = 0; i < list->ndevices; i++) {
+ virNodeDevicePtr dev = list->devices[i];
+ if (STRNEQ(names[i], "computer")) {
const char *parent = virNodeDeviceGetParent(dev);
parents[i] = parent ? vshStrdup(ctl, parent) : NULL;
} else {
parents[i] = NULL;
}
- virNodeDeviceFree(dev);
}
- for (i = 0 ; i < num_devices ; i++) {
+
+ for (i = 0 ; i < list->ndevices; i++) {
if (parents[i] == NULL &&
- vshTreePrint(ctl, vshNodeListLookup, &arrays, num_devices,
- i) < 0)
+ vshTreePrint(ctl, vshNodeListLookup, &arrays,
+ list->ndevices, i) < 0)
ret = false;
}
- for (i = 0 ; i < num_devices ; i++) {
- VIR_FREE(devices[i]);
+
+ for (i = 0 ; i < list->ndevices; i++)
VIR_FREE(parents[i]);
- }
VIR_FREE(parents);
+ for (i = 0; i < list->ndevices; i++)
+ VIR_FREE(names[i]);
+ VIR_FREE(names);
} else {
- for (i = 0; i < num_devices; i++) {
- vshPrint(ctl, "%s\n", devices[i]);
- VIR_FREE(devices[i]);
- }
+ for (i = 0; i < list->ndevices; i++)
+ vshPrint(ctl, "%s\n", virNodeDeviceGetName(list->devices[i]));
}
- VIR_FREE(devices);
+
+cleanup:
+ VIR_FREE(caps);
You'll need to free the "caps" string list here.
+ vshNodeDeviceListFree(list);
return ret;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 559e64d..a6390c2 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1856,9 +1856,11 @@ libvirt (such as whether device reset is supported).
=item B<nodedev-list> I<cap> I<--tree>
List all of the devices available on the node that are known by libvirt.
-If I<cap> is used, the list is filtered to show only the nodes that
-include the given capability. If I<--tree> is used, the output is
-formatted in a tree representing parents of each node.
+I<cap> is used to filter the list by capability types, the types must be
+separated by comma, e.g. --cap pci,scsi, valid capability types include
+'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target',
+'scsi', 'storage'. If I<--tree> is used, the output is formatted in a tree
+representing parents of each node.
Nice addition. You also could state that --tree and --cap are mutually
exclusive.
=item B<nodedev-reattach> I<nodedev>
ACK with the nits addressed.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list