Re: [libvirt] [PATCH 2/2 v2] storage: Add document for possible problem on volume detection

On 22/01/14 23:15, John Ferlan wrote: > > On 01/22/2014 09:39 AM, Osier Yang wrote: >> For pool which relies on remote resources, such as a "iscsi" type >> pool, since how long it takes to export the corresponding devices >> to host's sysfs is really depended, it could depend on the network >> connection, it also could depend on the host's udev procedures. So >> it's likely that the volumes are not able to be detected during pool >> starting process, polling the sysfs doesn't work, since we don't >> know how much time is best for the polling, and even worse, the >> volumes could still be not detected or partly not detected even after >> the polling. So we end up with a documentation to prompt the fact, >> in virsh manual. > Probably some of the above is overkill since the virsh.pod repeats much > of it - although having the intro comment specifically target udev > procedures (or "udevadm settle") as the culprit are good. Also see my > change to your text below. > >> And as a small improvement, let's explicitly say no LUNs found in >> the debug log in that case. >> --- >> src/storage/storage_backend_scsi.c | 5 +++++ >> tools/virsh.pod | 8 ++++++++ >> 2 files changed, 13 insertions(+) >> >> diff --git a/src/storage/storage_backend_scsi.c >> b/src/storage/storage_backend_scsi.c >> index 93039c1..fbcb102 100644 >> --- a/src/storage/storage_backend_scsi.c >> +++ b/src/storage/storage_backend_scsi.c >> @@ -495,6 +495,7 @@ >> virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, >> DIR *devicedir = NULL; >> struct dirent *lun_dirent = NULL; >> char devicepattern[64]; >> + bool found = false; >> VIR_DEBUG("Discovering LUs on host %u", scanhost); >> @@ -516,11 +517,15 @@ >> virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, >> continue; >> } >> + found = true; >> VIR_DEBUG("Found LU '%s'", lun_dirent->d_name); >> processLU(pool, scanhost, bus, target, lun); >> } >> + if (!found) >> + VIR_DEBUG("No LU found for pool %s", pool->def->name); >> + >> closedir(devicedir); >> return retval; >> diff --git a/tools/virsh.pod b/tools/virsh.pod >> index 77f9383..073465e 100644 >> --- a/tools/virsh.pod >> +++ b/tools/virsh.pod >> @@ -2650,6 +2650,14 @@ Refresh the list of volumes contained in >> I<pool>. >> Start the storage I<pool>, which is previously defined but >> inactive. >> +B<Note>: For pool which relies on remote resources, e.g. a >> "iscsi" type >> +pool, or a "scsi" type pool based on a (v)HBA, since the corresponding >> +devices for the volumes may not show up on the host's sysfs yet during >> +the pool starting process, it may need to refresh the pool (see >> +B<pool-refresh>) to get the volumes correctly detected. How many times >> +needed for the refreshing is depended on the network connection and >> the >> +time the host takes to export the corresponding devices to sysfs. >> + >> =item B<pool-undefine> I<pool-or-uuid> >> Undefine the configuration for an inactive I<pool>. >> > > B<Note>: A storage pool that relies on remote resources such as an > "iscsi" or a vHBA backed "scsi" pool may need to be refreshed multiple > times in order to have all the volumes detected (see B<pool-refresh>). > This is because the corresponding volume devices may not be present in > the host's filesystem during the initial pool startup or the current > refresh attempt. The number of refresh retries is dependant upon the > network connection and the time the host takes to export the > corresponding devices. I like it. :-) > > > (sic) note that formatstorage.html.in has both vHBA and (v)HBA, while We have to use (v)HBA here, since it's the same case for HBA.