Commit Graph

108 Commits

Author SHA1 Message Date
John Ferlan
08c0ea16fc conf: Return the vHBA name from virNodeDeviceCreateVport
Rather than returning true/false and having the caller check if the
vHBA was actually created, let's do that check within the CreateVport
function. That way the caller can faithfully assume success based
on a name start the thread looking for the LUNs. Prior to this change
it's possible that the vHBA wasn't really created (e.g if the call to
virVHBAGetHostByWWN returned NULL), we'd claim success, but in reality
there'd be no vHBA for the pool. This also fixes a second yet seen
issue that if the nodedev was present, but the parent by name wasn't
provided (perhaps parent by wwnn/wwpn or by fabric_name), then a failure
would be returned. For this path it shouldn't be an error - we should
just be happy that something else is managing the device and we don't
have to create/delete it.

The end result is that the createVport code can now just start the
refresh thread once it gets a non NULL name back.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-03-15 21:17:47 -04:00
John Ferlan
106930aaa7 conf: Move/Rename createVport and deleteVport
Move the bulk of createVport and rename to virNodeDeviceCreateVport.

Remove the deleteVport entirely and replace with virNodeDeviceDeleteVport

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-03-15 21:17:47 -04:00
John Ferlan
97e0d3c3c9 util: Rename virFileWaitForDevices
The function is actually in virutil.c, but prototyped in virfile.h.
This patch fixes that by renaming the function to virWaitForDevices,
adding the prototype in virutil.h and libvirt_private.syms, and then
changing the callers to use the new name.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-03-15 21:17:47 -04:00
John Ferlan
a7ce03c736 conf: Convert virStoragePoolSourceAdapter to virStorageAdapter
Move the virStoragePoolSourceAdapter from storage_conf.h and rename
to virStorageAdapter.

Continue with code realignment for brevity and flow.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-03-15 21:17:47 -04:00
John Ferlan
ed22190094 storage: Rework createVport and deleteVport
Rework the code to use the new FCHost specific adapter structures.

Also rework the parameters to only pass what's need and leave logic in
the caller for the adapter type and the need to call the helpers.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-03-15 21:17:47 -04:00
John Ferlan
b129ac6aad storage: Rework getAdapterName to use adapter specific typedefs
Use the FCHost and SCSIHost adapter specific typedefs

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-03-15 21:17:47 -04:00
Peter Krempa
f813fe810f storage: backend: Refactor registration of the backend drivers
Add APIs that allow to dynamically register driver backends so that the
list of available drivers does not need to be known during compile time.

This will allow us to modularize the storage driver on runtime.
2017-02-21 09:34:30 +01:00
John Ferlan
03346def06 util: Move scsi_host specific functions from virutil
Create a virscsihost.c and place the functions there. That removes the
last #ifdef __linux__ from virutil.c.

Take the opporunity to also change the function names and in one case
the parameters slightly
2017-02-19 06:45:09 -05:00
John Ferlan
d2d74a986d util: Replace virStoragePoolGetVhbaSCSIHostParent
Use the new virNodeDeviceGetParentName instead. Modify the callers to
build the node device scsi_host# name string in order to call the new
function so that proper lookup occurs.
2017-02-19 06:45:09 -05:00
John Ferlan
16416816c1 util: Create a new virvhba module and move/rename API's
Rather than have them mixed in with the virutil apis, create a separate
virvhba.c module and move the vHBA related calls into there. Soon there
will be more added.

Also modify the names of the functions and some arguments to be more
indicative of what is really happening. Adjust the callers respectively.

While I was changing fchosttest, rather than the non-descriptive names
test1...test6, rename them to match what the test is doing.
2017-02-19 06:45:09 -05:00
Peter Krempa
ebc8564c1a storage: scsi: Remove private constants from header
They are used only in the SCSI backend driver so there's no need to
pollute the headers.
2017-01-19 09:25:51 +01:00
Peter Krempa
0de123c84e storage: scsi: Fix build if SCSI backend is disabled but iSCSI is enabled
The iSCSI backend driver was using stuff from the SCSI driver without
making sure that it's compiled in. Move the common code into the
storage_util.c since it does not contain any specific code.
2017-01-19 09:25:51 +01:00
Peter Krempa
46e8049c15 storage: Split utility functions from storage_backend.(ch)
The file became a garbage dump for all kinds of utility functions over
time. Move them to a separate file so that the files can become a clean
interface for the storage backends.
2017-01-19 09:25:51 +01:00
John Ferlan
78be2e8b74 iscsi: Add parent wwnn/wwpn or fabric capability for createVport
https://bugzilla.redhat.com/show_bug.cgi?id=1349696

As it turns out using only the 'parent' to achieve the goal of a
consistent vHBA parent has issues with reboots where the scsi_hostX
parent could change to scsi_hostY causing either failure to create
the vHBA or usage of the wrong HBA for our vHBA.

Thus add the ability to search for the "parent" by the parent wwnn/
wwpn values or just a fabric_name if someone only cares to ensure
usage of the same SAN for the vHBA.
2017-01-06 17:15:34 -05:00
John Ferlan
9fdc8c4269 scsi: Converge more createVport checks
Remove duplicated code - make one simple path through

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-01-04 17:09:59 -05:00
John Ferlan
476ecf2a2a scsi: Change order of checks in createVport
Move the check for an already existing vHBA to the top of the function.
No sense in first decoding a provided parent if the next thing we're going
to do is fail if a provided wwnn/wwpn already exists.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-01-04 17:09:59 -05:00
John Ferlan
79ab093518 scsi: Clean up createVport exit paths
Use the ret = -1, goto cleanup, etc. rather than current hodgepodge.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-01-04 17:09:59 -05:00
Ján Tomko
0f79480b9f Replace some uses STREQLEN with STRPREFIX
Do not call it with a magic constant matching the length
of the pattern.
2016-06-24 16:30:55 +02:00
Ján Tomko
e81de04c10 Use virDirOpen
Switch from opendir to virDirOpen everywhere we need to report an error.
2016-06-24 14:20:57 +02:00
Ján Tomko
70a033ab42 Do not ignore hidden files in /sys and /proc
The directories we iterate over are unlikely to contain any entries
starting with a dot, other than '.' and '..' which is already skipped
by virDirRead.
2016-06-23 21:58:38 +02:00
Ján Tomko
a4e6f1eb9c Introduce VIR_DIR_CLOSE
Introduce a helper that only calls closedir if DIR* is non-NULL
and sets it to NULL afterwards.
2016-06-23 21:58:33 +02:00
Martin Kletzander
c36b1f7b6a Change virDevicePCIAddress to virPCIDeviceAddress
We had both and the only difference was that the latter also included
information about multifunction setting.  The problem with that was that
we couldn't use functions made for only one of the structs (e.g.
parsing).  To consolidate those two structs, use the one in virpci.h,
include that in domain_conf.h and add the multifunction member in it.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2016-05-02 15:46:23 +02:00
Nitesh Konkar
3e19b5d53d storage: Initialize pool size parameters for refresh thread
If the pool creation thread happens to detect the luns in
the scsi target, the size parameters will be calculated as
part of the refreshPool called from storagePoolCreate().

This means the virStoragePoolFCRefreshThread (commit id
'512b874') waiting to run and "refresh" the pool will
essentially double the allocation and capacity values.
A separate refresh would correct the values.

To avoid this, the FCRefreshThread needs to reinitialize
the pool size values prior to calling virStorageBackendSCSIFindLUs
which eventually calls virStorageBackendSCSINewLun and
updates the size values for each volume found.
2016-03-29 07:28:47 -04:00
John Ferlan
a523770c32 storage: Ignore block devices that fail format detection
https://bugzilla.redhat.com/show_bug.cgi?id=1276198

Prior to commit id '98322052' failure to saferead the block device would
cause an error to be logged and the device to be skipped while attempting
to discover/create a stable target path for a new LUN (NPIV).

This was because virStorageBackendSCSIFindLUs ignored errors from
processLU and virStorageBackendSCSINewLun.

Ignoring the failure allowed a multipath device with an "active" and
"ghost" to be present on the host with the "ghost" block device being
ignored. This patch will return a -2 to the caller indicating the desire
to ignore the block device since it cannot be used directly rather than
fail the pool startup.
2015-12-09 16:31:15 -05:00
John Ferlan
22346003dc storage: Add readflags for backend error processing
Similar to the openflags which allow VIR_STORAGE_VOL_OPEN_NOERROR to be
passed to avoid open errors, add a 'readflags' variable so that in the
future read failures could also be ignored.
2015-12-09 16:31:14 -05:00
John Ferlan
d3fa510a75 storage: Don't assume storage pool exists for FC/SCSI refresh thread
https://bugzilla.redhat.com/show_bug.cgi?id=1277781

The virStoragePoolFCRefreshThread had passed a pointer to the pool obj
in the virStoragePoolFCRefreshInfoPtr; however, we cannot assume that
the pool exists still since we don't keep the pool lock throughout
the duration of the thread.

Therefore, instead of passing the pool obj pointer, pass the UUID of
the pool and perform a lookup.  If found, then we can perform the
refresh using the locked pool obj pointer; otherwise, we just exit
the thread since the pool is now gone.
2015-11-12 06:30:33 -05:00
John Ferlan
27432ba70c storage: Change cbdata scsi refresh thread field name
Change the field name from 'name' to 'fchost_name' to better id it.
2015-11-12 06:30:32 -05:00
John Ferlan
1b695be173 scsi: Force error for SCSI pools on virStorageBackendSCSIFindLUs failure
Related to :

https://bugzilla.redhat.com/show_bug.cgi?id=1171933

Rather than ignore the return status from virStorageBackendSCSIFindLUs,
cause a failure to start the pool if a -1 is returned. Issue was noted
during testing of the bz for iscsi that 'scsi' and 'fc' pools don't fail.
2015-06-24 09:18:59 -04:00
John Ferlan
5c9fc1c11f scsi: Adjust return status from getBlockDevice
https://bugzilla.redhat.com/show_bug.cgi?id=1224233

Currently it's not possible to determine the difference between a
fatal memory allocation or failure to open/read the directory error
with a perhaps less fatal, I didn't find the "block" device in the
directory (which may be a disk entry without a block device).

In the case of the latter, we shouldn't cause failure to continue
searching in the caller (virStorageBackendSCSIFindLUs), rather we
should allow trying reading the next directory entry.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-06-18 17:14:00 +02:00
Michal Privoznik
c4bdfafcbb getOldStyleBlockDevice: Adjust formatting
Instead of initializing return value to zero (success) and overwriting
it on every failure just before the control jumps onto 'out' label,
let's initialize to an error value and set to zero only when we are
sure about the success. Just follow the pattern we have in the rest of
the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-06-15 14:13:39 +02:00
Michal Privoznik
71b66bec2b getNewStyleBlockDevice: Adjust formatting
Instead of initializing return value to zero (success) and overwriting
it on every failure just before the control jumps onto 'out' label,
let's initialize to an error value and set to zero only when we are
sure about the success. Just follow the pattern we have in the rest of
the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-06-15 14:13:34 +02:00
John Ferlan
0259426060 storage: Resolve Coverity UNINIT
commit id '1e13eff4' didn't init found when changed from a bool to
an int in virStoragePoolFCRefreshThread and Coverity...
2015-04-27 06:57:51 -04:00
John Ferlan
9832205263 scsi: Adjust return values from processLU
https://bugzilla.redhat.com/show_bug.cgi?id=1171933

Adjust the processLU error returns to be a bit more logical. Currently,
the calling code cannot determine the difference between a non disk/lun
volume and a processed/found disk/lun. It can also not differentiate
between perhaps real/fatal error and one that won't necessarily stop
the code from finding other volumes.

After this patch virStorageBackendSCSIFindLUsInternal will stop processing
as soon as a "fatal" message occurs rather than continuting processing
for no apparent reason. It will also only set the *found value when
at least one of the processLU's was successful.

With the failed return, if the reason for the stop was that the pool
target path did not exist, was /dev, was /dev/, or did not start with
/dev, then iSCSI pool startup and refresh will fail.
2015-04-21 10:20:17 -04:00
John Ferlan
1e13eff435 scsi: Change return values for virStorageBackendSCSIFindLUs
Rather than passing/returning a pointer to a boolean to indicate that
perhaps we should try again - adjust the return of the call to return
the count of LU's found during processing, then let the caller decide
what to do with that value.
2015-04-21 10:20:17 -04:00
John Ferlan
adb182fa2f scsi: Adjust return value for virStorageBackendSCSINewLun
Use virStorageBackendPoolUseDevPath API to determine whether creation of
stable target path is possible for the volume.

This will differentiate a failed virStorageBackendStablePath which won't
need to be fatal. Thus, we'll add a -2 return value to differentiate that
the failure was a result of either the inability to find the symlink for
the device or failure to open the target path directory
2015-04-21 10:20:13 -04:00
John Ferlan
0cf87b51d4 storage: Refactor virStorageBackendSCSINewLun
Invert the logical of retval and clean up code paths, especially goto's

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-04-16 12:28:04 -04:00
John Ferlan
f51fbdd19d scsi: Remove unused 'type_path' in processLU
Seems to be a remnant that was never cleaned up from original submit...

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-04-02 08:46:30 -04:00
Erik Skultety
cf7392a0d2 storage: Remove unused attribute conn from 'checkPool' callback
In order to be able to use 'checkPool' inside functions which do not
have any connection reference, 'conn' attribute needs to be discarded
from the checkPool's signature, since it's not used by any storage backend
anyway.
2015-04-02 11:57:07 +02:00
Ján Tomko
d3452a3f73 Revert "Restore skipping of setting capacity"
This reverts commit f1856eb622.

Now that we can update capacity from image metadata,
we don't need to skip the update.
2015-03-02 08:07:11 +01:00
John Ferlan
7b4cdb6eaa storage: Move and rename getVhbaSCSIHostParent
https://bugzilla.redhat.com/show_bug.cgi?id=1159180

Move the API from the backend to storage_conf and rename it to
virStoragePoolGetVhbaSCSIHostParent.  A future patch will need to
use this functionality from storage_conf
2014-12-01 10:04:19 -05:00
John Ferlan
512b8747d8 storage: Add thread to refresh for createVport
https://bugzilla.redhat.com/show_bug.cgi?id=1152382

When libvirt create's the vport (VPORT_CREATE) for the vHBA, there isn't
enough "time" between the creation and the running of the following
backend->refreshPool after a backend->startPool in order to find the LU's.
Population of LU's happens asynchronously when udevEventHandleCallback
discovers the "new" vHBA port.  Creation of the infrastructure by udev
is an iterative process creating and discovering actual storage devices and
adjusting the environment.

Because of the time it takes to discover and set things up, the backend
refreshPool call done after the startPool call will generally fail to
find any devices. This leaves the newly started pool appear empty when
querying via 'vol-list' after startup. The "workaround" has always been
to run pool-refresh after startup (or any time thereafter) in order to
find the LU's. Depending on how quickly run after startup, this too may
not find any LUs in the pool. Eventually though given enough time and
retries it will find something if LU's exist for the vHBA.

This patch adds a thread to be executed after the VPORT_CREATE which will
attempt to find the LU's without requiring the external run of refresh-pool.
It does this by waiting for 5 seconds and searching for the LU's. If any
are found, then the thread completes; otherwise, it will retry once more
in another 5 seconds.  If none are found in that second pass, the thread
gives up.

Things learned while investigating this... No need to try and fill the
pool too quickly or too many times. Over the course of creation, the udev
code may 'add', 'change', and 'delete' the same device. So if the refresh
code runs and finds something, it may display it only to have a subsequent
refresh appear to "lose" the device. The udev processing doesn't seem to
have a way to indicate that it's all done with the creation processing of a
newly found vHBA. Only the Lone Ranger has silver bullets to fix everything.
2014-11-20 10:24:11 -05:00
John Ferlan
2087041732 storage: Fix issue finding LU's when block doesn't exist
Fix a problem in the getBlockDevice and processLU where retval initialized
to zero causing some failures to erroneously continue through to the
virStorageBackendSCSINewLun with an attempt to find a path for "/dev/(null)".
This would fail approximately 10 seconds later with debug message:

virStorageBackendSCSINewLun:203 :
     No stable path found for '/dev/(null)' in '/dev/disk/by-path'

The root cause of the issue is for many /sys/bus/scsi/devices/<lun path>
there is no "block*" device found for the vHBA's, where "<lun path>" are
the various paths created for the vHBA, such as "17:0:0:0", "17:0:1:0",
"17:0:2:0", "17:0:3:0", etc.  If the block device isn't found, then the
directory should just be ignored rather than attempting to process it.

The bug was that in getBlockDevice the assumption was "block" would exist
and either getNewStyleBlockDevice or getOldStyleBlockDevice would fill in
@block_device. However, if 'block*' doesn't exist, then the code returned
NULL for block_device *and* a good (zero) retval value.  This caused the
processLU code to attempt the virStorageBackendSCSINewLun which failed
"at some point in time" in the future.

After this change - on test system the refresh-pool didn't have a noticable
pause of about 20 seconds - it completed within a second since no longer
was there an attempt/need to find "/dev/(null)".

Additionally, the virStorageBackendSCSIFindLU's shouldn't be declaring
found unless the processLU actually returns success. This will be
important in the followup patch which relies on whether a LU was found.
2014-11-20 10:24:11 -05:00
Martin Kletzander
e7a1da8aeb Remove unnecessary curly brackets in src/storage/
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-11-14 17:13:01 +01:00
John Ferlan
5530f248db storage: Introduce 'managed' for the fchost parent
https://bugzilla.redhat.com/show_bug.cgi?id=1160926

Introduce a 'managed' attribute to allow libvirt to decide whether to
delete a vHBA vport created via external means such as nodedev-create.
The code currently decides whether to delete the vHBA based solely on
whether the parent was provided at creation time. However, that may not
be the desired action, so rather than delete and force someone to create
another vHBA via an additional nodedev-create allow the configuration of
the storage pool to decide the desired action.

During createVport when libvirt does the VPORT_CREATE, set the managed
value to YES if not already set to indicate to the deleteVport code that
it should delete the vHBA when the pool is destroyed.

If libvirtd is restarted all the memory only state was lost, so for a
persistent storage pool, use the virStoragePoolSaveConfig in order to
write out the managed value.

Because we're now saving the current configuration, we need to be sure
to not save the parent in the output XML if it was undefined at start.
Saving the name would cause future starts to always use the same parent
which is not the expected result when not providing a parent. By not
providing a parent, libvirt is expected to find the best available
vHBA port for each subsequent (re)start.

At deleteVport, use the new managed value to decide whether to execute
the VPORT_DELETE.  Since we no longer save the parent in memory or in
XML when provided, if it was not provided, then we have to look it up.
2014-11-12 10:18:28 -05:00
John Ferlan
5b226fcdc6 storage: Don't use a stack copy of the adapter
https://bugzilla.redhat.com/show_bug.cgi?id=1160926

Passing a copy of the storage pool adapter to a function just changes the
copy of the fields in the particular function and then when returning to
the caller those changes are discarded.  While not yet biting us in the
storage clean-up case, it did cause an issue for the fchost storage pool
startup case, createVport.  The issue was at startup, if no parent is found
in the XML, the code will search for the 'best available' parent and then
store that in the in memory copy of the adapter.  Of course, in this case
it was a copy, so when returning to the virStorageBackendSCSIStartPool that
change was discarded (or lost) from the pool->def->source.adapter which
meant at shutdown (deleteVport), the code assumed no adapter was passed
and skipped the deletion, leaving the vHBA created by libvirt still defined
requiring an additional stop of a nodedev-destroy to remove.

Adjusted the createVport to take virStoragePoolDefPtr instead of the
adapter copy. Then use the virStoragePoolSourceAdapterPtr when processing.
A future patch will need the 'def' anyway, so this just sets up for that.
2014-11-12 10:18:28 -05:00
John Ferlan
42a021c120 storage: Ensure fc_host parent matches wwnn/wwpn
https://bugzilla.redhat.com/show_bug.cgi?id=1160565

The existing code assumed that the configuration of a 'parent' attribute
was correct for the createVport path. As it turns out, that may not be
the case which leads errors during the deleteVport path because the
wwnn/wwpn isn't associated with the parent.

With this change the following is reported:

error: Failed to start pool fc_pool_host3
error: XML error: Parent attribute 'scsi_host4' does not match parent 'scsi_host3' determined for the 'scsi_host16' wwnn/wwpn lookup.

for XML as follows:

  <pool type='scsi'>
    <name>fc_pool</name>
    <source>
      <adapter type='fc_host' parent='scsi_host4' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>
    </source>

Where 'nodedev-dumpxml scsi_host16' provides:

  <device>
    <name>scsi_host16</name>
    <path>/sys/devices/pci0000:00/0000:00:04.0/0000:10:00.0/host3/vport-3:0-11/host16</path>
    <parent>scsi_host3</parent>
    <capability type='scsi_host'>
      <host>16</host>
      <unique_id>13</unique_id>
      <capability type='fc_host'>
        <wwnn>5001a4aaf3ca174b</wwnn>
        <wwpn>5001a4a77192b864</wwpn>
...

The patch also adjusts the description of the storage pool to describe the
restrictions.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2014-11-12 10:18:28 -05:00
John Ferlan
844c1d7e32 storage: Check for valid fc_host parent at startup
https://bugzilla.redhat.com/show_bug.cgi?id=1160565

If a 'parent' attribute is provided for the fchost, then at startup
time check to ensure it is a vport capable scsi_host. If the parent
is not vport capable, then disallow the startup. The following is the
expected results:

error: Failed to start pool fc_pool
error: XML error: parent 'scsi_host2' specified for vHBA is not vport capable

where the XML for the fc_pool is:

    <pool type='scsi'>
      <name>fc_pool</name>
      <source>
        <adapter type='fc_host' parent='scsi_host2' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>
      </source>
...

and 'scsi_host2' is not vport capable.

Providing an incorrect parent and a correct wwnn/wwpn could lead to
failures at shutdown (deleteVport) where the assumption is the parent
is for the fchost.

NOTE: If the provided wwnn/wwpn doesn't resolve to an existing scsi_host,
      then we will be creating one with code (virManageVport) which
      assumes the parent is vport capable.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2014-11-12 10:18:27 -05:00
John Ferlan
beff5d4e1b virutil: Introduce virGetSCSIHostNameByParentaddr
Create the function from the code in getAdapterName() in order to return
the "host#" name for the provided parentaddr values.
2014-10-28 21:25:32 -04:00
John Ferlan
55f439599c virutil: Introduce virGetSCSIHostNumber
Create/use virGetSCSIHostNumber to replace the static getHostNumber

Removed the "if (result &&" since result is now required to be non NULL
on input.
2014-10-28 21:25:26 -04:00
John Ferlan
ea37fb34a9 getAdapterName: Lookup stable scsi_host
If a parentaddr was provided in the XML, have getAdapterName lookup
the stable address.  This allows virStorageBackendSCSICheckPool() and
virStorageBackendSCSIRefreshPool() to automagically find the scsi_host
by its PCI address and unique_id
2014-07-21 12:55:11 -04:00