On 23/01/14 21:35, John Ferlan wrote:
On 01/23/2014 04:45 AM, Osier Yang wrote:
On 22/01/14 21:36, John Ferlan wrote:
On 01/07/2014 06:07 AM, Osier Yang wrote:
On 07/01/14 01:21, John Ferlan wrote:
On 01/06/2014 05:19 AM, Osier Yang wrote:
The "checkPool" is a bit different for pool with "fc_host"
type source adapter, since the vHBA it's based on might be
not created yet (it's created by "startPool", which is
involked after "checkPool" in storageDriverAutostart). So it
should not fail, otherwise the "autostart" of the pool will
fail either.
The problem is easy to reproduce:
* Enable "autostart" for the pool
* Restart libvirtd service
* Check the pool's state
---
src/storage/storage_backend_scsi.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
Not sure this is the right thing to do. With this change it doesn't
matter what getAdapterName() returns for fc_host's...
It matters with if "*isActive" is true or false in the end. We don't
need to try to start the pool if it's already active.
Fair enough; however, let's consider the failure case. On failure, a
message is reported and name == NULL. Do we need to clear that error now?
I think my original thoughts were along the lines of having
getAdapterName do more of the heavy lifting. Have both check and start
call it. It would call the createVport which would be mostly unchanged,
except for the virFileWaitForDevices() which could move to start...
Found your method *might* break one logic.
Assuming the pool is not marked as "autostart", and the vHBA was not
created yet. Since with your method "checkPool" will create the vHBA now,
then it's possible for "checkPool" to return "*isActive" as true, as long as
the device path for the created vHBA can show up in host very quickly
(like what we talked a lot in PATCH 2/2, how long it will show up is also
depended, the time can be long, but it also can be short), i.e. during the
"checkPool" process. And thus the pool will be marked as "Active". As
the result, the problem on one side of the coin is fixed, but it introduces
a similar problem on another side. :-)
Huh? Not sure I see what problem you're driving at.
Look back at the caller - isActive from _scsi translates to "started" in
the caller. The 'started' is used to decide whether to call 'startPool'
and 'refreshPool'. If autostart is disabled, then 'startPool' won't be
called.
Assuming pool->autostart if false, and "started" is happened to be true.
Calling refreshPool is likely to cause pool->active == 1.
<...>
if (started) {
if (backend->refreshPool(conn, pool) < 0) {
virErrorPtr err = virGetLastError();
if (backend->stopPool)
backend->stopPool(conn, pool);
VIR_ERROR(_("Failed to autostart storage pool '%s': %s"),
pool->def->name, err ? err->message :
_("no error message found"));
virStoragePoolObjUnlock(pool);
continue;
}
pool->active = 1;
}
</...>
Since the vHBA was already created in checkPool, then I see not much
reason why refreshPool can not be success.
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list