Split out the worker loop into a separate function and export it.
Reviewed-by: NEric Blake <eblake@redhat.com>

Allow getting the raw data from query-blockstats, so that we can use it
to detect the backing chain later on.
Reviewed-by: NEric Blake <eblake@redhat.com>

Disallow providing the wwnn/wwpn of the HBA in the adapter XML:

  <adapter type='fc_host' [parent='scsi_hostN'] wwnn='HBA_wwnn'
    wwpn='HBA_wwpn'/>

This should be considered a configuration error since a vHBA
would not be created. In order to use the HBA as the backing the
following XML should be used:

  <adapter type='scsi_host' name='scsi_hostN'/>

So add a check prior to the checkParent call to validate that
the provided wwnn/wwpn resolves to a vHBA and not an HBA.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Since commit 2e6ecba1, the pointer to the qemu driver is saved in
domain object's private data and hence does not have to be passed as
yet another parameter if domain object is already one of them.

This is a first (example) patch of this kind of clean up, others will
hopefully follow.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

A virNetworkObjPtr will be an 'obj'.

A virNetworkPtr will be a 'net'.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

The switch contains considerable amount of changes:

  virQEMUCapsRememberCached() is removed because this is now handled
  by virFileCacheSave().

  virQEMUCapsInitCached() is removed because this is now handled by
  virFileCacheLoad().

  virQEMUCapsNewForBinary() is split into two functions,
  virQEMUCapsNewData() which creates new data if there is nothing
  cached and virQEMUCapsLoadFile() which loads the cached data.
  This is now handled by virFileCacheNewData().

  virQEMUCapsCacheValidate() is removed because this is now handled by
  virFileCacheValidate().

  virQEMUCapsCacheFree() is removed because it's no longer required.

  Add virCapsPtr into virQEMUCapsCachePriv because for each call of
  virFileCacheLookup*() we need to use current virCapsPtr.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

This is a preparation for following patches where we switch to
virFileCache for QEMU capabilities cache

The host arch will always remain the same but virCaps may change.  Now
the host arch is stored while creating new qemu capabilities cache.
It removes the need to pass virCaps into virQEMUCapsCache*() functions.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

This will store private data that will be used by following patches
when switching to virFileCache.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

The new virFileCache will nicely handle the caching logic for any data
that we would like to cache.  For each type of data we will just need
to implement few handlers that will take care of creating, validating,
loading and saving the cached data.

The cached data must be an instance of virObject.

Currently we cache QEMU capabilities which will start using
virFileCache.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

Introduced by commit 'a7bc2c8c'.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

It's possible to have more than one unnamed virtio-serial unix channel.
We need to generate a unique name for each channel. Currently, we use
".../unknown.sock" for all of them. Better practice would be to specify
an explicit target path name; however, in the absence of that, we need
uniqueness in the names we generate internally.

Before the changes we'd get /var/lib/libvirt/qemu/channel/target/unknown.sock
for each instance of
    <channel type='unix'>
        <source mode='bind'/>
        <target type='virtio'/>
    </channel>

Now, we get vioser-00-00-01.sock, vioser-00-00-02.sock, etc.
Signed-off-by: NScott Garfinkle <seg@us.ibm.com>

It is more related to a domain as we might use it even when there is
no systemd and it does not use any dbus/systemd functions.  In order
not to use code from conf/ in util/ pass machineName in cgroups code
as a parameter.  That also fixes a leak of machineName in the lxc
driver and cleans up and de-duplicates some code.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This way the function can work as a central point of clean-up code and
we don't have to duplicate code.  And it works similarly to the qemu
driver.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This way we can finally make it static and not use any externs anywhere.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This will help us to get to some data more easily.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Rather than rely on virSecretObjEndAPI to make the final virObjectUnref
after the call to virSecretObjListRemove, be more explicit by calling
virObjectUnref and setting @obj to NULL for secretUndefine and in
the error path of secretDefineXML. Calling EndAPI will end up calling
Unlock on an already unlocked object which has indeteriminate results
(usually an ignored error).

The virSecretObjEndAPI will both Unref and Unlock the object; however,
the virSecretObjListRemove would have already Unlock'd the object so
calling Unlock again is incorrect. Once the virSecretObjListRemove
is called all that's left is to Unref our interest since that's the
corrollary to the virSecretObjListAdd which returned our ref interest
plus references for each hash table in which the object resides. In math
terms, after an Add there's 2 refs on the object (1 for the object and
1 for the list). After calling Remove there's just 1 ref on the object.
For the Add callers, calling EndAPI removes the ref for the object and
unlocks it, but since it's in a list the other 1 remains.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

If the virSecretLoadValue fails, the code jumped to cleanup without
setting @ret = obj, thus calling virSecretObjListRemove which only
accounts for the object reference related to adding the object to
the list during virSecretObjListAdd, but does not account for the
reference to the object itself as the return of @ret would be NULL
so the caller wouldn't call virSecretObjEndAPI on the object recently
added thus reducing the refcnt to zero.

This patch will perform the ObjListRemove in the failure path of
virSecretLoadValue and Unref @obj in order to perform clean up
and return @obj as NULL. The @def will be freed as part of the
virObjectUnref.

Since the virSecretObjListAdd technically consumes @def on success,
the secretDefineXML should set @def = NULL immediately and process
the remaining calls using a new @objDef variable. We can use use
VIR_STEAL_PTR since we know the Add function just stores @def in
obj->def.

Because we steal @def into @objDef, if we jump to restore_backup:
and @backup is set, then we need to ensure the @def would be
free'd properly, so we'll steal it back from @objDef. For the other
condition this fixes a double free of @def if the code had jumped to
@backup == NULL thus calling virSecretObjListRemove without setting
@def = NULL. In this case, the subsequent call to DefFree would
succeed and free @def; however, the call to EndAPI would also
call DefFree because the Unref done would be the last one for
the @obj meaning the obj->def would be used to call DefFree,
but it's already been free'd because @def wasn't managed right
within this error path.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Rather than assign to a local variable, let's just assign directly to the
object using the error path for cleanup.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

This reverts commit 328bd244.

As it turns out, this is not portable and very Linux & glibc
specific. Worse, this may lead to not starving writers on Linux
but everywhere else. Revert this and if the starvation occurs
resolve it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrange <berrange@redhat.com>

The original name didn't hint at the fact that PHBs are
a pSeries-specific concept.
Suggested-by: NPeter Krempa <pkrempa@redhat.com>
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

Recent commits made it so that pci-root controllers for
pSeries guests are automatically assigned the
spapr-pci-host-bridge model name; however, that prevents
guests to migrate to older versions of libvirt which don't
know about that model name at all, which at the moment is
all of them :)

To avoid the issue, just strip the model name from PHBs
when formatting the migratable XML; guests that use more
than one PHB are not going to be migratable anyway.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

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

If the parent provided for the storage pool adapter is not vHBA
capable, then issue a configuration error even though the provided
wwnn/wwpn were found.

It is a configuration error to provide a mismatched parent to
the wwnn/wwpn. The @parent is optional and is used as a means to
perform duplicate pool source checks.

It's no longer needed since the checkParent code moved back to
storage_backend_scsi.c
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

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

Commit id '106930aa' altered the order of checking for an existing
vHBA (e.g something created via nodedev-create functionality outside
of the storage pool logic) which inadvertantly broke the code to
decide whether to alter/force the fchost->managed field to be 'yes'
because the storage pool will be managing the created vHBA in order
to ensure when the storage pool is destroyed that the vHBA is also
destroyed.

This patch moves the check (and checkParent helper) for an existing
vHBA back into the createVport in storage_backend_scsi. It also
adjusts the checkParent logic to more closely follow the intentions
prior to commit id '79ab0935'. The changes made by commit id '08c0ea16'
are only necessary to run the virStoragePoolFCRefreshThread when
a vHBA was really created because there's a timing lag such that
the refreshPool call made after a startPool from storagePoolCreate*
wouldn't necessarily find LUNs, but the thread would. For an already
existing vHBA, using the thread is unnecessary since the vHBA already
exists and the lag to configure the LUNs wouldn't exist.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Since virnodedeviceobj now has a self-lockable hash table, there's no
need to lock the table from the driver for processing. Thus remove the
locks from the driver for NodeDeviceObjList mgmt.

This includes the test driver as well.

Rather than use a forward linked list of elements, it'll be much more
efficient to use a hash table to reference the elements by unique name
and to perform hash searches.

This patch does all the heavy lifting of converting the list object to
use a self locking list that contains the hash table. Each of the FindBy
functions that do not involve finding the object by it's key (name) is
converted to use virHashSearch in order to find the specific object.
When searching for the key (name), it's possible to use virHashLookup.
For any of the list perusal functions that are required to evaluate
each object, the virHashForEach function is used.

The only callers to this function are from CreateXML paths now, so
let's just remove the unnecessary parameter.

Alter the node device deletion logic to make use of the parent field
from the obj->def rather than call virNodeDeviceObjListGetParentHost.
As it turns out the saved @def won't have parent_wwnn/wwpn or
parent_fabric_wwn, so the only logical path would be to call
virNodeDeviceObjListGetParentHostByParent which we can accomplish
directly via virNodeDeviceObjListFindByName.

There is no reason why two threads trying to look up two domains
should mutually exclude each other. Utilize new
virObjectRWLockable that was just introduced.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

Up until now we only had virObjectLockable which uses mutexes for
mutually excluding each other in critical section. Well, this is
not enough. Future work will require RW locks so we might as well
have virObjectRWLockable which is introduced here.

Moreover, polymorphism is introduced to our code for the first
time. Yay! More specifically, virObjectLock will grab a write
lock, virObjectLockRead will grab a read lock then (what a
surprise right?). This has great advantage that an object can be
made derived from virObjectRWLockable in a single line and still
continue functioning properly (mutexes can be viewed as grabbing
write locks only). Then just those critical sections that can
grab a read lock need fixing. Therefore the resulting change is
going to be way smaller.

In order to avoid writer starvation, the object initializes RW
lock that prefers writers.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

We already have virRWLockInit. But this uses pthread defaults
which prefer reader to initialize the RW lock. This may lead to
writer starvation. Therefore we need to have the counterpart that
prefers writers. Now, according to the
pthread_rwlockattr_setkind_np() man page setting
PTHREAD_RWLOCK_PREFER_WRITER_NP attribute is no-op. Therefore we
need to use PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
attribute. So much for good enum value names.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

It was observed while adding new property that there should be a space
before closing a curly brace in intel-iommu object property definition.
Fixing it as a separate patch.
Signed-off-by: NShivaprasad G Bhat <sbhat@linux.vnet.ibm.com>

Currently, @port is type of string. Well, that's overkill and
waste of memory. Port is always an integer. Use it as such.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Split out parsing of one host into a separate function and add a new
function to loop through all the host XML nodes.

This change removes multiple levels of nesting due to the old XML
parsing approach used.

Similarly to when parsing XML we need to fill in default ports for the
backing chain. This was missed in commit 5bda8354

Since we now set the default ports when parsing disks, it's not
necessary to have default port numbers encoded in the command line
generator.

Negative ports don't make sense so use a unsigned integer.

Move it to virstring.c and improve it to parse and validate ports. New
name is virStringParsePort.