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.