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

Re: [libvirt] PATCH: 11/12: virsh support



On Thu, 2008-11-13 at 17:35 +0000, Daniel P. Berrange wrote:
> This patch adds two node virsh commands for the node device enumeration
> APIs. The only change here is to change the command names to have a shorter
> prefix,  nodedev-list and nodedev-dumpxml
> 
> Daniel
> 
> diff -r 0136f215fc06 src/virsh.c
> --- a/src/virsh.c	Thu Nov 13 13:06:59 2008 +0000
> +++ b/src/virsh.c	Thu Nov 13 13:07:59 2008 +0000
> @@ -4410,6 +4410,96 @@
>          vshPrint(ctl, _("Running hypervisor: %s %d.%d.%d\n"),
>                   hvType, major, minor, rel);
>      }
> +    return TRUE;
> +}
> +
> +/*
> + * "nodedev-list" command
> + */
> +static const vshCmdInfo info_node_list_devices[] = {
> +    {"syntax", "nodedev-list [--cap <capability>]"},

How about:

+    {"syntax", "nodedev-list [--cap net|block|storage|scsi|scsi_host|pci|usb]"},

so people easily know what capabilities are valid?

> +    {"help", gettext_noop("enumerate devices on this host")},
> +    {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_node_list_devices[] = {
> +    {"cap", VSH_OT_STRING, VSH_OFLAG_NONE, gettext_noop("capability name")},
> +    {NULL, 0, 0, NULL}
> +};
> +
> +static int
> +cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> +{
> +    char *cap;
> +    char **devices;
> +    int found, num_devices, i;
> +
> +    if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
> +        return FALSE;
> +
> +    cap = vshCommandOptString(cmd, "cap", &found);
> +    if (!found)
> +        cap = NULL;
> +
> +    num_devices = cap ? virNodeNumOfDevicesByCap(ctl->conn, cap, 0) :
> +        virNodeNumOfDevices(ctl->conn, 0);
> +    if (num_devices < 0) {
> +        vshError(ctl, FALSE, "%s", _("Failed to count node devices"));
> +        return FALSE;
> +    } else if (num_devices == 0) {
> +        return TRUE;
> +    }
> +
> +    devices = vshMalloc(ctl, sizeof(char *) * num_devices);
> +    num_devices = cap ?
> +        virNodeListDevicesByCap(ctl->conn, cap, devices, num_devices, 0) :
> +        virNodeListDevices(ctl->conn, devices, num_devices, 0);
> +    if (num_devices < 0) {
> +        vshError(ctl, FALSE, "%s", _("Failed to list node devices"));
> +        free(devices);
> +        return FALSE;
> +    }
> +    for (i = 0; i < num_devices; i++) {
> +        vshPrint(ctl, "%s\n", devices[i]);

Just printing the name makes the output seem a bit sparse - but I guess
the names are fairly descriptive.
 
> @@ -5565,6 +5655,9 @@
>      {"net-uuid", cmdNetworkUuid, opts_network_uuid, info_network_uuid},
>      {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo},
>  
> +    {"node-list-devices", cmdNodeListDevices, opts_node_list_devices, info_node_list_devices},
> +    {"node-device-dumpxml", cmdNodeDeviceDumpXML, opts_node_device_dumpxml, info_node_device_dumpxml},
> +

You never actually renamed them to nodedev-list and nodedev-dumpxml

By the way, trying dumpxml on a non-existent name is a bit noisy:

# virsh node-device-dumpxml foo
libvir: Device Monitor error : invalid node device pointer in no node device with matching name
error: Could not find matching device 'foo'

Cheers,
Mark.


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