[libvirt] [PATCH 0/6] Enumerate scsi generic device
The scsi generic device is listed as a child of the parent of the mapped disk device. E.g. +- pci_0000_00_1f_2 | | | +- scsi_host0 | | | | | +- scsi_target0_0_0 | | | | | +- scsi_0_0_0_0 | | | | | +- block_sda_TOSHIBA_MK5061GSY_9243PG8CT | | +- scsi_generic_sg0 This is consistent with the sysfs/udev. The XML produced by udev backend: <device> <name>scsi_generic_sg0</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>scsi_0_0_0_0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char> </capability> </device> The XML produced by HAL backend: <device> <name>pci_8086_2922_scsi_host_scsi_device_lun0_scsi_generic</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>pci_8086_2922_scsi_host_scsi_device_lun0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char> </capability> </device> Osier Yang (6): nodedev: Expose sysfs path of device nodedev_udev: Refactor udevGetDeviceType nodedev_udev: Enumerate scsi generic device nodedev_hal: Enumerate scsi generic device nodedev: Support SCSI_GENERIC cap flag for listAllNodeDevices virsh: Support SCSI_GENERIC cap flag for nodedev-list include/libvirt/libvirt.h.in | 1 + src/conf/node_device_conf.c | 11 ++- src/conf/node_device_conf.h | 8 ++- src/node_device/node_device_hal.c | 9 +++ src/node_device/node_device_udev.c | 137 ++++++++++++++++++++----------------- tools/virsh-nodedev.c | 3 + tools/virsh.pod | 6 +- 7 files changed, 105 insertions(+), 70 deletions(-) -- 1.8.1.4
The name format is constructed by libvirt, it's not that clear to get what the device's sysfs path should be. This exposes the device's sysfs path by a new tag <path>. Since the sysfspath is filled during enumerating the devices by either udev or HAL. It's an output-only tag. --- src/conf/node_device_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4eeb3b3..cc6f297 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -236,6 +236,7 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, "<device>\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); + virBufferEscapeString(&buf, " <path>%s</path>\n", def->sysfs_path); if (def->parent) { virBufferEscapeString(&buf, " <parent>%s</parent>\n", def->parent); } -- 1.8.1.4
On 06/03/2013 06:05 AM, Osier Yang wrote:
The name format is constructed by libvirt, it's not that clear to get what the device's sysfs path should be. This exposes the device's sysfs path by a new tag <path>.
Since the sysfspath is filled during enumerating the devices by either udev or HAL. It's an output-only tag. --- src/conf/node_device_conf.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4eeb3b3..cc6f297 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -236,6 +236,7 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def)
virBufferAddLit(&buf, "<device>\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); + virBufferEscapeString(&buf, " <path>%s</path>\n", def->sysfs_path);
Is this necessarily filled in? EG, do you need to do an "if (def->sysfs_path)" first? ACK John
if (def->parent) { virBufferEscapeString(&buf, " <parent>%s</parent>\n", def->parent); }
On 07/06/13 04:29, John Ferlan wrote:
On 06/03/2013 06:05 AM, Osier Yang wrote:
The name format is constructed by libvirt, it's not that clear to get what the device's sysfs path should be. This exposes the device's sysfs path by a new tag <path>.
Since the sysfspath is filled during enumerating the devices by either udev or HAL. It's an output-only tag. --- src/conf/node_device_conf.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4eeb3b3..cc6f297 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -236,6 +236,7 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def)
virBufferAddLit(&buf, "<device>\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); + virBufferEscapeString(&buf, " <path>%s</path>\n", def->sysfs_path); Is this necessarily filled in? EG, do you need to do an "if (def->sysfs_path)" first?
No. virBufferEscapeString has the checking inside, if the string is NULL, it does nothing for &buf.
ACK
John
if (def->parent) { virBufferEscapeString(&buf, " <parent>%s</parent>\n", def->parent); }
Checking if the "devtype" is NULL along with each "if" statements is bad. It wastes the performance, and also not good for reading. And also when the "devtype" is NULL, the logic is also not clear. This reorgnizes the logic of with "if...else" and a bunch of "else if". Other changes: * Change the function style. * Remove the useless debug statement. * Get rid of the goto * New helper udevDeviceHasProperty to simplify the logic for checking if a property is existing for the device. * Add comment to clarify "PCI devices don't set the DEVTYPE property" * s/sysfs path/sysfs name/, as udev_device_get_sysname returns the name instead of the full path. E.g. "sg0" * Refactor the comment for setting VIR_NODE_DEV_CAP_NET cap type a bit. --- src/node_device/node_device_udev.c | 119 ++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 67 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 620cd58..3c91298 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -106,7 +106,6 @@ static int udevStrToLong_i(char const *s, return ret; } - /* This function allocates memory from the heap for the property * value. That memory must be later freed by some other code. */ static int udevGetDeviceProperty(struct udev_device *udev_device, @@ -1100,78 +1099,64 @@ out: return ret; } - -static int udevGetDeviceType(struct udev_device *device, - enum virNodeDevCapType *type) +static bool +udevDeviceHasProperty(struct udev_device *dev, + const char *key) { - const char *devtype = NULL; - char *tmp_string = NULL; - unsigned int tmp = 0; - int ret = 0; + const char *value = NULL; + bool ret = false; - devtype = udev_device_get_devtype(device); - VIR_DEBUG("Found device type '%s' for device '%s'", - NULLSTR(devtype), udev_device_get_sysname(device)); - - if (devtype != NULL && STREQ(devtype, "usb_device")) { - *type = VIR_NODE_DEV_CAP_USB_DEV; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "usb_interface")) { - *type = VIR_NODE_DEV_CAP_USB_INTERFACE; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_host")) { - *type = VIR_NODE_DEV_CAP_SCSI_HOST; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_target")) { - *type = VIR_NODE_DEV_CAP_SCSI_TARGET; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_device")) { - *type = VIR_NODE_DEV_CAP_SCSI; - goto out; - } + if ((value = udev_device_get_property_value(dev, key))) + ret = true; - if (devtype != NULL && STREQ(devtype, "disk")) { - *type = VIR_NODE_DEV_CAP_STORAGE; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "wlan")) { - *type = VIR_NODE_DEV_CAP_NET; - goto out; - } - - if (udevGetUintProperty(device, "PCI_CLASS", &tmp, 16) == PROPERTY_FOUND) { - *type = VIR_NODE_DEV_CAP_PCI_DEV; - goto out; - } + return ret; +} - /* It does not appear that wired network interfaces set the - * DEVTYPE property. USB devices also have an INTERFACE property, - * but they do set DEVTYPE, so if devtype is NULL and the - * INTERFACE property exists, we have a network device. */ - if (devtype == NULL && - udevGetStringProperty(device, - "INTERFACE", - &tmp_string) == PROPERTY_FOUND) { - VIR_FREE(tmp_string); - *type = VIR_NODE_DEV_CAP_NET; - goto out; - } +static int +udevGetDeviceType(struct udev_device *device, + enum virNodeDevCapType *type) +{ + const char *devtype = NULL; + int ret = -1; - VIR_DEBUG("Could not determine device type for device " - "with sysfs path '%s'", - udev_device_get_sysname(device)); - ret = -1; + devtype = udev_device_get_devtype(device); + *type = 0; + + if (devtype) { + if (STREQ(devtype, "usb_device")) + *type = VIR_NODE_DEV_CAP_USB_DEV; + else if (STREQ(devtype, "usb_interface")) + *type = VIR_NODE_DEV_CAP_USB_INTERFACE; + else if (STREQ(devtype, "scsi_host")) + *type = VIR_NODE_DEV_CAP_SCSI_HOST; + else if (STREQ(devtype, "scsi_target")) + *type = VIR_NODE_DEV_CAP_SCSI_TARGET; + else if (STREQ(devtype, "scsi_device")) + *type = VIR_NODE_DEV_CAP_SCSI; + else if (STREQ(devtype, "disk")) + *type = VIR_NODE_DEV_CAP_STORAGE; + else if (STREQ(devtype, "wlan")) + *type = VIR_NODE_DEV_CAP_NET; + } else { + /* PCI devices don't set the DEVTYPE property. */ + if (udevDeviceHasProperty(device, "PCI_CLASS")) + *type = VIR_NODE_DEV_CAP_PCI_DEV; + + /* Wired network interfaces don't set the DEVTYPE property, + * USB devices also have an INTERFACE property, but they do + * set DEVTYPE, so if devtype is NULL and the INTERFACE + * property exists, we have a network device. */ + if (udevDeviceHasProperty(device, "INTERFACE")) + *type = VIR_NODE_DEV_CAP_NET; + } + + if (!*type) + VIR_DEBUG("Could not determine device type for device " + "with sysfs name '%s'", + udev_device_get_sysname(device)); + else + ret = 0; -out: return ret; } -- 1.8.1.4
On 06/03/2013 06:05 AM, Osier Yang wrote:
Checking if the "devtype" is NULL along with each "if" statements is bad. It wastes the performance, and also not good for reading. And also when the "devtype" is NULL, the logic is also not clear.
This reorgnizes the logic of with "if...else" and a bunch of "else if".
Other changes: * Change the function style. * Remove the useless debug statement. * Get rid of the goto * New helper udevDeviceHasProperty to simplify the logic for checking if a property is existing for the device. * Add comment to clarify "PCI devices don't set the DEVTYPE property" * s/sysfs path/sysfs name/, as udev_device_get_sysname returns the name instead of the full path. E.g. "sg0" * Refactor the comment for setting VIR_NODE_DEV_CAP_NET cap type a bit. --- src/node_device/node_device_udev.c | 119 ++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 67 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 620cd58..3c91298 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -106,7 +106,6 @@ static int udevStrToLong_i(char const *s, return ret; }
-
I have a recollection to other recent changes where the extra line was specifically added. I personally don't care, but I figured I'd point it out in case someone did...
/* This function allocates memory from the heap for the property * value. That memory must be later freed by some other code. */ static int udevGetDeviceProperty(struct udev_device *udev_device, @@ -1100,78 +1099,64 @@ out: return ret; }
- -static int udevGetDeviceType(struct udev_device *device, - enum virNodeDevCapType *type) +static bool +udevDeviceHasProperty(struct udev_device *dev, + const char *key)
Ditto with the extra line thing... Also 'udevDevice' seems to be redundant doesn't it? Perhaps udevHasDeviceProperty but it's not that important...
{ - const char *devtype = NULL; - char *tmp_string = NULL; - unsigned int tmp = 0; - int ret = 0; + const char *value = NULL; + bool ret = false;
- devtype = udev_device_get_devtype(device); - VIR_DEBUG("Found device type '%s' for device '%s'", - NULLSTR(devtype), udev_device_get_sysname(device)); - - if (devtype != NULL && STREQ(devtype, "usb_device")) { - *type = VIR_NODE_DEV_CAP_USB_DEV; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "usb_interface")) { - *type = VIR_NODE_DEV_CAP_USB_INTERFACE; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_host")) { - *type = VIR_NODE_DEV_CAP_SCSI_HOST; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_target")) { - *type = VIR_NODE_DEV_CAP_SCSI_TARGET; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_device")) { - *type = VIR_NODE_DEV_CAP_SCSI; - goto out; - } + if ((value = udev_device_get_property_value(dev, key))) + ret = true;
Coverity complains (UNUSED_VALUE): 1123 (1) Event returned_pointer: Pointer "value" returned by "udev_device_get_property_value(dev, key)" is never used. 1124 if ((value = udev_device_get_property_value(dev, key))) Essentially it's pointing out that value really isn't necessary...
- if (devtype != NULL && STREQ(devtype, "disk")) { - *type = VIR_NODE_DEV_CAP_STORAGE; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "wlan")) { - *type = VIR_NODE_DEV_CAP_NET; - goto out; - } - - if (udevGetUintProperty(device, "PCI_CLASS", &tmp, 16) == PROPERTY_FOUND) { - *type = VIR_NODE_DEV_CAP_PCI_DEV; - goto out; - } + return ret; +}
- /* It does not appear that wired network interfaces set the - * DEVTYPE property. USB devices also have an INTERFACE property, - * but they do set DEVTYPE, so if devtype is NULL and the - * INTERFACE property exists, we have a network device. */ - if (devtype == NULL && - udevGetStringProperty(device, - "INTERFACE", - &tmp_string) == PROPERTY_FOUND) { - VIR_FREE(tmp_string); - *type = VIR_NODE_DEV_CAP_NET; - goto out; - } +static int +udevGetDeviceType(struct udev_device *device, + enum virNodeDevCapType *type) +{ + const char *devtype = NULL; + int ret = -1;
- VIR_DEBUG("Could not determine device type for device " - "with sysfs path '%s'", - udev_device_get_sysname(device)); - ret = -1; + devtype = udev_device_get_devtype(device); + *type = 0; + + if (devtype) { + if (STREQ(devtype, "usb_device")) + *type = VIR_NODE_DEV_CAP_USB_DEV; + else if (STREQ(devtype, "usb_interface")) + *type = VIR_NODE_DEV_CAP_USB_INTERFACE; + else if (STREQ(devtype, "scsi_host")) + *type = VIR_NODE_DEV_CAP_SCSI_HOST; + else if (STREQ(devtype, "scsi_target")) + *type = VIR_NODE_DEV_CAP_SCSI_TARGET; + else if (STREQ(devtype, "scsi_device")) + *type = VIR_NODE_DEV_CAP_SCSI; + else if (STREQ(devtype, "disk")) + *type = VIR_NODE_DEV_CAP_STORAGE; + else if (STREQ(devtype, "wlan")) + *type = VIR_NODE_DEV_CAP_NET; + } else { + /* PCI devices don't set the DEVTYPE property. */ + if (udevDeviceHasProperty(device, "PCI_CLASS")) + *type = VIR_NODE_DEV_CAP_PCI_DEV; + + /* Wired network interfaces don't set the DEVTYPE property, + * USB devices also have an INTERFACE property, but they do + * set DEVTYPE, so if devtype is NULL and the INTERFACE + * property exists, we have a network device. */ + if (udevDeviceHasProperty(device, "INTERFACE")) + *type = VIR_NODE_DEV_CAP_NET; + } + + if (!*type) + VIR_DEBUG("Could not determine device type for device " + "with sysfs name '%s'", + udev_device_get_sysname(device)); + else + ret = 0;
-out: return ret; }
This mechanism seems better. Fix the Coverity complaint for an ACK. John
On 07/06/13 04:33, John Ferlan wrote:
On 06/03/2013 06:05 AM, Osier Yang wrote:
Checking if the "devtype" is NULL along with each "if" statements is bad. It wastes the performance, and also not good for reading. And also when the "devtype" is NULL, the logic is also not clear.
This reorgnizes the logic of with "if...else" and a bunch of "else if".
Other changes: * Change the function style. * Remove the useless debug statement. * Get rid of the goto * New helper udevDeviceHasProperty to simplify the logic for checking if a property is existing for the device. * Add comment to clarify "PCI devices don't set the DEVTYPE property" * s/sysfs path/sysfs name/, as udev_device_get_sysname returns the name instead of the full path. E.g. "sg0" * Refactor the comment for setting VIR_NODE_DEV_CAP_NET cap type a bit. --- src/node_device/node_device_udev.c | 119 ++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 67 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 620cd58..3c91298 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -106,7 +106,6 @@ static int udevStrToLong_i(char const *s, return ret; }
- I have a recollection to other recent changes where the extra line was specifically added. I personally don't care, but I figured I'd point it out in case someone did...
Just curious, why one want to add more than 1 blank lines among stuffs? Does it really make the code more readable? I think most of the cases we still use 1 blank line in the source.
/* This function allocates memory from the heap for the property * value. That memory must be later freed by some other code. */ static int udevGetDeviceProperty(struct udev_device *udev_device, @@ -1100,78 +1099,64 @@ out: return ret; }
- -static int udevGetDeviceType(struct udev_device *device, - enum virNodeDevCapType *type) +static bool +udevDeviceHasProperty(struct udev_device *dev, + const char *key) Ditto with the extra line thing...
Also 'udevDevice' seems to be redundant doesn't it? Perhaps udevHasDeviceProperty but it's not that important...
Ack'ed udevHasDeviceProperty is better, to be consistent with the names in current udev backend, though it needs thorough cleanup. But it will be later patches.
{ - const char *devtype = NULL; - char *tmp_string = NULL; - unsigned int tmp = 0; - int ret = 0; + const char *value = NULL; + bool ret = false;
- devtype = udev_device_get_devtype(device); - VIR_DEBUG("Found device type '%s' for device '%s'", - NULLSTR(devtype), udev_device_get_sysname(device)); - - if (devtype != NULL && STREQ(devtype, "usb_device")) { - *type = VIR_NODE_DEV_CAP_USB_DEV; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "usb_interface")) { - *type = VIR_NODE_DEV_CAP_USB_INTERFACE; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_host")) { - *type = VIR_NODE_DEV_CAP_SCSI_HOST; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_target")) { - *type = VIR_NODE_DEV_CAP_SCSI_TARGET; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "scsi_device")) { - *type = VIR_NODE_DEV_CAP_SCSI; - goto out; - } + if ((value = udev_device_get_property_value(dev, key))) + ret = true; Coverity complains (UNUSED_VALUE): 1123
(1) Event returned_pointer: Pointer "value" returned by "udev_device_get_property_value(dev, key)" is never used.
1124 if ((value = udev_device_get_property_value(dev, key)))
Essentially it's pointing out that value really isn't necessary...
Okay.
- if (devtype != NULL && STREQ(devtype, "disk")) { - *type = VIR_NODE_DEV_CAP_STORAGE; - goto out; - } - - if (devtype != NULL && STREQ(devtype, "wlan")) { - *type = VIR_NODE_DEV_CAP_NET; - goto out; - } - - if (udevGetUintProperty(device, "PCI_CLASS", &tmp, 16) == PROPERTY_FOUND) { - *type = VIR_NODE_DEV_CAP_PCI_DEV; - goto out; - } + return ret; +}
- /* It does not appear that wired network interfaces set the - * DEVTYPE property. USB devices also have an INTERFACE property, - * but they do set DEVTYPE, so if devtype is NULL and the - * INTERFACE property exists, we have a network device. */ - if (devtype == NULL && - udevGetStringProperty(device, - "INTERFACE", - &tmp_string) == PROPERTY_FOUND) { - VIR_FREE(tmp_string); - *type = VIR_NODE_DEV_CAP_NET; - goto out; - } +static int +udevGetDeviceType(struct udev_device *device, + enum virNodeDevCapType *type) +{ + const char *devtype = NULL; + int ret = -1;
- VIR_DEBUG("Could not determine device type for device " - "with sysfs path '%s'", - udev_device_get_sysname(device)); - ret = -1; + devtype = udev_device_get_devtype(device); + *type = 0; + + if (devtype) { + if (STREQ(devtype, "usb_device")) + *type = VIR_NODE_DEV_CAP_USB_DEV; + else if (STREQ(devtype, "usb_interface")) + *type = VIR_NODE_DEV_CAP_USB_INTERFACE; + else if (STREQ(devtype, "scsi_host")) + *type = VIR_NODE_DEV_CAP_SCSI_HOST; + else if (STREQ(devtype, "scsi_target")) + *type = VIR_NODE_DEV_CAP_SCSI_TARGET; + else if (STREQ(devtype, "scsi_device")) + *type = VIR_NODE_DEV_CAP_SCSI; + else if (STREQ(devtype, "disk")) + *type = VIR_NODE_DEV_CAP_STORAGE; + else if (STREQ(devtype, "wlan")) + *type = VIR_NODE_DEV_CAP_NET; + } else { + /* PCI devices don't set the DEVTYPE property. */ + if (udevDeviceHasProperty(device, "PCI_CLASS")) + *type = VIR_NODE_DEV_CAP_PCI_DEV; + + /* Wired network interfaces don't set the DEVTYPE property, + * USB devices also have an INTERFACE property, but they do + * set DEVTYPE, so if devtype is NULL and the INTERFACE + * property exists, we have a network device. */ + if (udevDeviceHasProperty(device, "INTERFACE")) + *type = VIR_NODE_DEV_CAP_NET; + } + + if (!*type) + VIR_DEBUG("Could not determine device type for device " + "with sysfs name '%s'", + udev_device_get_sysname(device)); + else + ret = 0;
-out: return ret; }
This mechanism seems better. Fix the Coverity complaint for an ACK.
John
Since scsi generic device doesn't have DEVTYPE property set, the only way to we have to get it's a scsi generic device is from the "SUBSYSTEM" property. The XML of the scsi generic device will be like: <device> <name>scsi_generic_sg0</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>scsi_0_0_0_0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char> </capability> </device> --- src/conf/node_device_conf.c | 6 +++++- src/conf/node_device_conf.h | 5 +++++ src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index cc6f297..a0b6338 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -49,7 +49,8 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "scsi", "storage", "fc_host", - "vports") + "vports", + "scsi_generic") VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", @@ -472,6 +473,9 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, " <capability type='hotpluggable' />\n"); break; + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + virBufferEscapeString(&buf, " <char>%s</char>\n", + data->sg.path); case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_LAST: diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 1c5855c..e326e82 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -48,6 +48,8 @@ enum virNodeDevCapType { VIR_NODE_DEV_CAP_STORAGE, /* Storage device */ VIR_NODE_DEV_CAP_FC_HOST, /* FC Host Bus Adapter */ VIR_NODE_DEV_CAP_VPORTS, /* HBA which is capable of vports */ + VIR_NODE_DEV_CAP_SCSI_GENERIC, /* SCSI generic device */ + VIR_NODE_DEV_CAP_LAST }; @@ -165,6 +167,9 @@ struct _virNodeDevCapsDef { char *media_label; unsigned int flags; /* virNodeDevStorageCapFlags bits */ } storage; + struct { + char *path; + } sg; /* SCSI generic device */ } data; virNodeDevCapsDefPtr next; /* next capability */ }; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 3c91298..27a48cf 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1099,6 +1099,21 @@ out: return ret; } +static int +udevProcessScsiGeneric(struct udev_device *dev, + virNodeDeviceDefPtr def) +{ + if (udevGetStringProperty(dev, + "DEVNAME", + &def->caps->data.sg.path) != PROPERTY_FOUND) + return -1; + + if (udevGenerateDeviceName(dev, def, NULL) != 0) + return -1; + + return 0; +} + static bool udevDeviceHasProperty(struct udev_device *dev, const char *key) @@ -1117,6 +1132,7 @@ udevGetDeviceType(struct udev_device *device, enum virNodeDevCapType *type) { const char *devtype = NULL; + char *subsystem = NULL; int ret = -1; devtype = udev_device_get_devtype(device); @@ -1148,6 +1164,11 @@ udevGetDeviceType(struct udev_device *device, * property exists, we have a network device. */ if (udevDeviceHasProperty(device, "INTERFACE")) *type = VIR_NODE_DEV_CAP_NET; + + if (udevGetStringProperty(device, "SUBSYSTEM", &subsystem) == + PROPERTY_FOUND && + STREQ(subsystem, "scsi_generic")) + *type = VIR_NODE_DEV_CAP_SCSI_GENERIC; } if (!*type) @@ -1194,6 +1215,9 @@ static int udevGetDeviceDetails(struct udev_device *device, case VIR_NODE_DEV_CAP_STORAGE: ret = udevProcessStorage(device, def); break; + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + ret = udevProcessScsiGeneric(device, def); + break; default: VIR_ERROR(_("Unknown device type %d"), def->caps->type); ret = -1; -- 1.8.1.4
On 06/03/2013 06:05 AM, Osier Yang wrote:
Since scsi generic device doesn't have DEVTYPE property set, the only way to we have to get it's a scsi generic device is from the "SUBSYSTEM" property.
I think you meant possessive case not the conjunction of "it is", e.g. s/it's a/its/ and perhaps s/generic device is/generic device data is/
The XML of the scsi generic device will be like:
<device> <name>scsi_generic_sg0</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>scsi_0_0_0_0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char>6d8528e3e104c34d477d478e3adf608a6e1bf7e2 </capability> </device> --- src/conf/node_device_conf.c | 6 +++++- src/conf/node_device_conf.h | 5 +++++ src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index cc6f297..a0b6338 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -49,7 +49,8 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "scsi", "storage", "fc_host", - "vports") + "vports", + "scsi_generic")
VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", @@ -472,6 +473,9 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, " <capability type='hotpluggable' />\n"); break; + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + virBufferEscapeString(&buf, " <char>%s</char>\n", + data->sg.path);
break; Just so no one inadvertently changes any of the next cases..
case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_LAST: diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 1c5855c..e326e82 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -48,6 +48,8 @@ enum virNodeDevCapType { VIR_NODE_DEV_CAP_STORAGE, /* Storage device */ VIR_NODE_DEV_CAP_FC_HOST, /* FC Host Bus Adapter */ VIR_NODE_DEV_CAP_VPORTS, /* HBA which is capable of vports */ + VIR_NODE_DEV_CAP_SCSI_GENERIC, /* SCSI generic device */ + VIR_NODE_DEV_CAP_LAST };
@@ -165,6 +167,9 @@ struct _virNodeDevCapsDef { char *media_label; unsigned int flags; /* virNodeDevStorageCapFlags bits */ } storage; + struct { + char *path; + } sg; /* SCSI generic device */ } data; virNodeDevCapsDefPtr next; /* next capability */ }; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 3c91298..27a48cf 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1099,6 +1099,21 @@ out: return ret; }
+static int +udevProcessScsiGeneric(struct udev_device *dev, + virNodeDeviceDefPtr def) +{ + if (udevGetStringProperty(dev, + "DEVNAME", + &def->caps->data.sg.path) != PROPERTY_FOUND) + return -1; +
When is data.sg.path VIR_FREE()'d? I think you need a case in the switch in 'virNodeDevCapsDefFree()' for VIR_NODE_DEV_CAP_SCSI_GENERIC: VIR_FREE(data->sg.path); break;
+ if (udevGenerateDeviceName(dev, def, NULL) != 0) + return -1; + + return 0; +} + static bool udevDeviceHasProperty(struct udev_device *dev, const char *key) @@ -1117,6 +1132,7 @@ udevGetDeviceType(struct udev_device *device, enum virNodeDevCapType *type) { const char *devtype = NULL; + char *subsystem = NULL; int ret = -1;
devtype = udev_device_get_devtype(device); @@ -1148,6 +1164,11 @@ udevGetDevi6d8528e3e104c34d477d478e3adf608a6e1bf7e2ceType(struct udev_device *device, * property exists, we have a network device. */ if (udevDeviceHasProperty(device, "INTERFACE")) *type = VIR_NODE_DEV_CAP_NET;
How about a comment prior to the call like the other two in this else {}
+ + if (udevGetStringProperty(device, "SUBSYSTEM", &subsystem) == + PROPERTY_FOUND && + STREQ(subsystem, "scsi_generic")) + *type = VIR_NODE_DEV_CAP_SCSI_GENERIC;
VIR_FREE(subsystem); (memory leak)
}
if (!*type) @@ -1194,6 +1215,9 @@ static int udevGetDeviceDetails(struct udev_device *device, case VIR_NODE_DEV_CAP_STORAGE: ret = udevProcessStorage(device, def); break; + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + ret = udevProcessScsiGeneric(device, def); + break; default: VIR_ERROR(_("Unknown device type %d"), def->caps->type); ret = -1;
If I understood what you said in the patch comments - this data isn't kept in the xml file and thus you wouldn't need a case in the switch in virNodeDevCapsDefParseXML(), correct? I searched for VIR_NODE_DEV_CAP_STORAGE and made sure corresponding entries were made for VIR_NODE_DEV_CAP_SCSI_GENERIC. ACK w/ issues addressed (added break;, case in virNodeDevCapsDefFree(), comments, and VIR_FREE(). I didn't see a case in the DefParseXML() for SCSI_GENERIC and wanted to be sure. John
On 06/03/2013 06:05 AM, Osier Yang wrote:
Since scsi generic device doesn't have DEVTYPE property set, the only way to we have to get it's a scsi generic device is from the "SUBSYSTEM" property. I think you meant possessive case not the conjunction of "it is", e.g.
s/it's a/its/
and perhaps
s/generic device is/generic device data is/
The XML of the scsi generic device will be like:
<device> <name>scsi_generic_sg0</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>scsi_0_0_0_0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char>6d8528e3e104c34d477d478e3adf608a6e1bf7e2 </capability> </device> --- src/conf/node_device_conf.c | 6 +++++- src/conf/node_device_conf.h | 5 +++++ src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index cc6f297..a0b6338 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -49,7 +49,8 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "scsi", "storage", "fc_host", - "vports") + "vports", + "scsi_generic")
VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", @@ -472,6 +473,9 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, " <capability type='hotpluggable' />\n"); break; + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + virBufferEscapeString(&buf, " <char>%s</char>\n", + data->sg.path); break;
Just so no one inadvertently changes any of the next cases..
case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_LAST: diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 1c5855c..e326e82 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -48,6 +48,8 @@ enum virNodeDevCapType { VIR_NODE_DEV_CAP_STORAGE, /* Storage device */ VIR_NODE_DEV_CAP_FC_HOST, /* FC Host Bus Adapter */ VIR_NODE_DEV_CAP_VPORTS, /* HBA which is capable of vports */ + VIR_NODE_DEV_CAP_SCSI_GENERIC, /* SCSI generic device */ + VIR_NODE_DEV_CAP_LAST };
@@ -165,6 +167,9 @@ struct _virNodeDevCapsDef { char *media_label; unsigned int flags; /* virNodeDevStorageCapFlags bits */ } storage; + struct { + char *path; + } sg; /* SCSI generic device */ } data; virNodeDevCapsDefPtr next; /* next capability */ }; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 3c91298..27a48cf 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1099,6 +1099,21 @@ out: return ret; }
+static int +udevProcessScsiGeneric(struct udev_device *dev, + virNodeDeviceDefPtr def) +{ + if (udevGetStringProperty(dev, + "DEVNAME", + &def->caps->data.sg.path) != PROPERTY_FOUND) + return -1; +
When is data.sg.path VIR_FREE()'d?
I think you need a case in the switch in 'virNodeDevCapsDefFree()' for VIR_NODE_DEV_CAP_SCSI_GENERIC: VIR_FREE(data->sg.path); break;
+ if (udevGenerateDeviceName(dev, def, NULL) != 0) + return -1; + + return 0; +} + static bool udevDeviceHasProperty(struct udev_device *dev, const char *key) @@ -1117,6 +1132,7 @@ udevGetDeviceType(struct udev_device *device, enum virNodeDevCapType *type) { const char *devtype = NULL; + char *subsystem = NULL; int ret = -1;
devtype = udev_device_get_devtype(device); @@ -1148,6 +1164,11 @@ udevGetDevi6d8528e3e104c34d477d478e3adf608a6e1bf7e2ceType(struct udev_device *device, * property exists, we have a network device. */ if (udevDeviceHasProperty(device, "INTERFACE")) *type = VIR_NODE_DEV_CAP_NET; How about a comment prior to the call like the other two in this else {}
+ + if (udevGetStringProperty(device, "SUBSYSTEM", &subsystem) == + PROPERTY_FOUND && + STREQ(subsystem, "scsi_generic")) + *type = VIR_NODE_DEV_CAP_SCSI_GENERIC; VIR_FREE(subsystem);
(memory leak)
}
if (!*type) @@ -1194,6 +1215,9 @@ static int udevGetDeviceDetails(struct udev_device *device, case VIR_NODE_DEV_CAP_STORAGE: ret = udevProcessStorage(device, def); break; + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + ret = udevProcessScsiGeneric(device, def); + break; default: VIR_ERROR(_("Unknown device type %d"), def->caps->type); ret = -1;
If I understood what you said in the patch comments - this data isn't kept in the xml file and thus you wouldn't need a case in the switch in virNodeDevCapsDefParseXML(), correct?
I searched for VIR_NODE_DEV_CAP_STORAGE and made sure corresponding entries were made for VIR_NODE_DEV_CAP_SCSI_GENERIC.
ACK w/ issues addressed (added break;, case in virNodeDevCapsDefFree(), comments, and VIR_FREE(). I didn't see a case in the DefParseXML() for SCSI_GENERIC and wanted to be sure. I don't see why we have to parse the XML of node device, except
On 07/06/13 04:38, John Ferlan wrote: the XML for vHBA, for which we provide API to create it, and PCI device, qemu and xen drivers needs to collect host PCI device addresses. So I didn't add parsing helper for scsi generic device. For other nits in this patch and other patches, I fixed them. Pushed the set. Thanks for the reviewing! Osier
The xml outputed by HAL backend for scsi generic device: <device> <name>pci_8086_2922_scsi_host_scsi_device_lun0_scsi_generic</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>pci_8086_2922_scsi_host_scsi_device_lun0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char> </capability> </device> --- src/node_device/node_device_hal.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 99b5044..794f923 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -303,6 +303,14 @@ gather_storage_cap(LibHalContext *ctx, const char *udi, return 0; } +static int +gather_scsi_generic_cap(LibHalContext *ctx, const char *udi, + union _virNodeDevCapData *d) +{ + (void)get_str_prop(ctx, udi, "scsi_generic.device", &d->sg.path); + return 0; +} + static int gather_system_cap(LibHalContext *ctx, const char *udi, @@ -350,6 +358,7 @@ static caps_tbl_entry caps_tbl[] = { { "scsi_host", VIR_NODE_DEV_CAP_SCSI_HOST, gather_scsi_host_cap }, { "scsi", VIR_NODE_DEV_CAP_SCSI, gather_scsi_cap }, { "storage", VIR_NODE_DEV_CAP_STORAGE, gather_storage_cap }, + { "scsi_generic", VIR_NODE_DEV_CAP_SCSI_GENERIC, gather_scsi_generic_cap }, }; -- 1.8.1.4
On 06/03/2013 06:05 AM, Osier Yang wrote:
The xml outputed by HAL backend for scsi generic device:
<device> <name>pci_8086_2922_scsi_host_scsi_device_lun0_scsi_generic</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>pci_8086_2922_scsi_host_scsi_device_lun0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char> </capability> </device> --- src/node_device/node_device_hal.c | 9 +++++++++ 1 file changed, 9 insertions(+)
ACK, Although I must say I'm not familiar with whether adding new entries to the caps_tbl_entry requires anything more than what you did. Seems correct. John
On 07/06/13 04:38, John Ferlan wrote:
On 06/03/2013 06:05 AM, Osier Yang wrote:
The xml outputed by HAL backend for scsi generic device:
<device> <name>pci_8086_2922_scsi_host_scsi_device_lun0_scsi_generic</name> <path>/sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0</path> <parent>pci_8086_2922_scsi_host_scsi_device_lun0</parent> <capability type='scsi_generic'> <char>/dev/sg0</char> </capability> </device> --- src/node_device/node_device_hal.c | 9 +++++++++ 1 file changed, 9 insertions(+)
ACK,
Although I must say I'm not familiar with whether adding new entries to the caps_tbl_entry requires anything more than what you did. Seems correct.
It's fine, I tested on old systems which have libhal, instead of the replacement project udisks... Osier
--- include/libvirt/libvirt.h.in | 1 + src/conf/node_device_conf.c | 4 +++- src/conf/node_device_conf.h | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..e7e003c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3258,6 +3258,7 @@ typedef enum { VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE = 1 << 8, /* Storage device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST = 1 << 9, /* FC Host Bus Adapter */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS = 1 << 10, /* Capable of vport */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC = 1 << 11, /* Capable of scsi_generic */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index a0b6338..b764cb8 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1492,7 +1492,9 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj, (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST) && virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_FC_HOST)) || (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS) && - virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_VPORTS)))) + virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_VPORTS)) || + (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC) && + virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_SCSI_GENERIC)))) return false; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index e326e82..17240be 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -274,7 +274,8 @@ void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj); VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST | \ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS) + VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC) int virNodeDeviceList(virConnectPtr conn, virNodeDeviceObjList devobjs, -- 1.8.1.4
On 06/03/2013 06:05 AM, Osier Yang wrote:
--- include/libvirt/libvirt.h.in | 1 + src/conf/node_device_conf.c | 4 +++- src/conf/node_device_conf.h | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-)
ACK.... Seems reasonable - although the last time you updated virConnectListAllNodeDeviceFlags you also changed libvirt.c to add VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST and VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS to the documented list of flags in virConnectListAllNodeDevices(). I would think you'd need to add VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC too. I used cscope search on VIR_CONNECT_LIST_NODE_DEVICES_CAP in order to find that. John
On 07/06/13 04:40, John Ferlan wrote:
On 06/03/2013 06:05 AM, Osier Yang wrote:
--- include/libvirt/libvirt.h.in | 1 + src/conf/node_device_conf.c | 4 +++- src/conf/node_device_conf.h | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-)
ACK....
Seems reasonable - although the last time you updated virConnectListAllNodeDeviceFlags you also changed libvirt.c to add VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST and VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS to the documented list of flags in virConnectListAllNodeDevices(). I would think you'd need to add VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC too.
Thanks for finding it out, will add . Osier
Document for nodedev-list is also updated. --- tools/virsh-nodedev.c | 3 +++ tools/virsh.pod | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 3657c5f..da93b8e 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -453,6 +453,9 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) case VIR_NODE_DEV_CAP_VPORTS: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS; break; + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC; + break; default: break; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 7c8ce18..f9e0287 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2086,9 +2086,9 @@ List all of the devices available on the node that are known by libvirt. 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', 'fc_host', 'vports'. If I<--tree> is used, the output -is formatted in a tree representing parents of each node. I<cap> and -I<--tree> are mutually exclusive. +'scsi', 'storage', 'fc_host', 'vports', 'scsi_generic'. If I<--tree> is +used, the output is formatted in a tree representing parents of each node. +I<cap> and I<--tree> are mutually exclusive. =item B<nodedev-reattach> I<nodedev> -- 1.8.1.4
On 06/03/2013 06:05 AM, Osier Yang wrote:
Document for nodedev-list is also updated. --- tools/virsh-nodedev.c | 3 +++ tools/virsh.pod | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-)
ACK John
participants (2)
-
John Ferlan -
Osier Yang