Re: [libvirt] [PATCH 17/25] utils: Add a helper to get the device name that sg device mapped to

On 07/05/13 22:00, John Ferlan wrote: > On 05/03/2013 02:07 PM, Osier Yang wrote: >> E.g. >> >> % sg_map >> /dev/sg0 /dev/sda >> /dev/sg1 /dev/sr0 >> >> What the helper gets for /dev/sg0 is /dev/sda, it will be used by >> later patch. >> --- >> src/libvirt_private.syms | 1 + >> src/util/virscsi.c | 47 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> src/util/virscsi.h | 4 ++++ >> 3 files changed, 52 insertions(+) >> >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index fa59c14..8a7105c 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -1692,6 +1692,7 @@ virSCSIDeviceFileIterate; >> virSCSIDeviceFree; >> virSCSIDeviceGetAdapter; >> virSCSIDeviceGetBus; >> +virSCSIDeviceGetDevName; >> virSCSIDeviceGetName; >> virSCSIDeviceGetReadonly; >> virSCSIDeviceGetSgName; >> diff --git a/src/util/virscsi.c b/src/util/virscsi.c >> index 7d3d40e..f337c75 100644 >> --- a/src/util/virscsi.c >> +++ b/src/util/virscsi.c >> @@ -143,6 +143,53 @@ cleanup: >> return sg; >> } >> +/* Returns device name (e.g. "sdc") on success, or NULL >> + * on failure. >> + */ >> +char * >> +virSCSIDeviceGetDevName(const char *adapter, >> + unsigned int bus, >> + unsigned int target, >> + unsigned int unit) >> +{ >> + DIR *dir = NULL; >> + struct dirent *entry; >> + char *path = NULL; >> + char *name = NULL; >> + unsigned int adapter_id; >> + >> + if (virSCSIDeviceGetAdapterId(adapter, &adapter_id) < 0) >> + return NULL; >> + >> + if (virAsprintf(&path, >> + SYSFS_SCSI_DEVICES "/%d:%d:%d:%d/block", >> + adapter_id, bus, target, unit) < 0) { >> + virReportOOMError(); >> + return NULL; >> + } >> + >> + if (!(dir = opendir(path))) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("Failed to open %s"), path); >> + goto cleanup; >> + } >> + >> + while ((entry = readdir(dir))) { >> + if (entry->d_name[0] == '.') >> + continue; >> + >> + if (!(name = strdup(entry->d_name))) { >> + virReportOOMError(); >> + goto cleanup; >> + } > If 'name' then what - we go back to the top of the loop and if there's > more entries then we'll strdup() another name.... Although I have to > wonder if there can be more than non hidden entry in the list... Makes > me wonder about my earlier reviews which didn't think about the loop > exit (that is virSCSIDeviceGetSgName()) It's the only one entry (except the hidden entries), same for virSCSIDeviceGetSgName. But adding the "break" is better, to avoid it loops 1 more. Will change when pushing.