Re: [libvirt] [PATCH v2 3/6] scsi: Adjust return value for virStorageBackendSCSINewLun

On Wed, Apr 01, 2015 at 01:29:08PM -0400, John Ferlan wrote: > Add a return -2 to differentiate that the failure was a result of a non > stable device path found or some other real error which would be messaged > in some manner. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/storage/storage_backend_scsi.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c > index 58e7e6d..6def373 100644 > --- a/src/storage/storage_backend_scsi.c > +++ b/src/storage/storage_backend_scsi.c > @@ -146,6 +146,16 @@ virStorageBackendSCSISerial(const char *dev) > } > > > +/* > + * Attempt to create a new LUN > + * > + * Returns: > + * > + * 0 => Success > + * -1 => Failure due to some sort of OOM or other fatal issue found when > + * attempting to get/update information about a found volume > + * -2 => Failure to find a stable path > + */ > static int > virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, > uint32_t host ATTRIBUTE_UNUSED, > @@ -202,7 +212,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, > VIR_DEBUG("No stable path found for '%s' in '%s'", > devpath, pool->def->target.path); > > - retval = -1; > + retval = -2; This mixes two different errors: * virStorageBackendStablePath short-circuited based on pool target path that's just as fatal as OOM and the attempt to find other volumes will also fail * virStorageBackendStablePath fell through to ret_strdup - the directory exists, but a symlink to the device did not show up This means the device could appear later, so it makes sense to retry later. And it doesn't handle the opendir failure in virStorageBackendStablePath, which could also mean that the device will appear later. Given that this check here has to negate the !STREQ "/dev" checks in virStorageBackendStablePath, maybe virStorageBackendStablePath should be split up to two functions - one that returns an error when the device path can't be stabilized and the other that would strdup the original path?