Re: [libvirt] [PATCH 3/6] nodedev_udev: Enumerate scsi generic device
On 07/06/13 04:38, John Ferlan wrote: 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: scsi_generic_sg0 /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 scsi_0_0_0_0 /dev/sg06d8528e3e104c34d477d478e3adf608a6e1bf7e2 --- 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, "\n"); break; +case VIR_NODE_DEV_CAP_SCSI_GENERIC: +virBufferEscapeString(&buf, "%s\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 co
Re: [libvirt] [PATCH 3/6] nodedev_udev: Enumerate scsi generic device
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: > > > scsi_generic_sg0 > > /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 > scsi_0_0_0_0 > > /dev/sg06d8528e3e104c34d477d478e3adf608a6e1bf7e2 > > > --- > 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, > "\n"); > break; > +case VIR_NODE_DEV_CAP_SCSI_GENERIC: > +virBufferEscapeString(&buf, "%s\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); > +b
[libvirt] [PATCH 3/6] nodedev_udev: Enumerate scsi generic device
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: scsi_generic_sg0 /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 scsi_0_0_0_0 /dev/sg0 --- 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, "\n"); break; +case VIR_NODE_DEV_CAP_SCSI_GENERIC: +virBufferEscapeString(&buf, "%s\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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list