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>

virDomainXMLOptionNew() gladly accepts NULL and it is used in some
drivers.  There is no need for additional variable with no members set
Signed-off-by: NMartin Kletzander <mkletzan@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>

When generating HTML documentation we put enum values into a
table so that we can display the value's name, numerical value
and description (if it has one). Now the last part is problem. If
the value doesn't have description the table row has just two
cells and if it has one the row counts three cells. This makes
HTML engines render the description into very little space - for
instance see:

  html/libvirt-libvirt-domain.html#virDomainMemoryStatTags

We can avoid this problem if we let the cell that corresponds to
numerical value span over two cells if there's no description.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

After f4cb85c6 we only have two options for placing enum
values descriptions. It's either:

    typedef enum {
        /* Some long description. Therefore it's placed before
         * the value. */
        VIR_ENUM_A_VAL = 1,
    } virEnumA;

or:

    typedef enum {
        VIR_ENUM_B_VAL = 1, /* Some short description */
    } virEnumB;

However, our apibuild.py script is not able to deal with the
former one. It messes up comments. To fix this couple of things
needs to be done:

a) DO NOT reset self.comment in parseEnumBlock(). This is a
result from our tokenizer. Upon calling token() if it finds a
comment block it stores it in self.comment and returns the next
token (which is not comment). Therefore, if we reset self.comment
we might lose the first comment in the enum block.

b) we need a variable to track if the current enum block uses
value descriptions before or after values. That is if it's type
virEnumA or virEnumB. Depending on that, it we're dealing with
virEnumA type and the current token is a comma ',' we can add the
value into the list as we already have everything needed:
comment, name and value.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.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.

Commit id '905f1024' had a rogue editing mistake that inadvertently
dropped a goto cleanup in storagePoolSetAutostart, but Coverity noted it.

So the way we format this huge virQEMUCaps enum is we group the
values in groups of five. And then at the beginning of each group
we have a small comment that says what's the number of the first
item in the group. Well, the last commit of 11b2ebf3 does not
follow this formatting.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Alter the virStoragePoolObjNumOfVolumes, virStoragePoolObjVolumeGetNames,
and virStoragePoolObjVolumeListExport APIs to take a virStoragePoolObjPtr
instead of the &obj->volumes and obj->def.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

A virStoragePoolObjPtr will be an 'obj'.

A virStoragePoolPtr will be a 'pool'.

A virStorageVolPtr will be a 'vol'.

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

A virStoragePoolObjPtr will be an 'obj'.

Also modify the @matchpool to @matchobj.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Use the < 0 rather than == -1 (consistently) for virAsprintf errors.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Remove repetitive code, replace with common function.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Rather than have repetitive code - create/use a couple of helpers:

    testStoragePoolObjFindActiveByName
    testStoragePoolObjFindInactiveByName

This will also allow for the reduction of some cleanup path logic.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Rework some of the test driver API's to remove the need to return
failure when testStoragePoolObjFindByName returns NULL rather than
going to cleanup. This removes the need for check for "if (obj)" and in
some instances the need to for a cleanup label and a local ret variable.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

A virStoragePoolObjPtr will be an 'obj'.

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

Fix some spacing/formatting in the storage pool/vol test driver code.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: Ndann frazier <dann.frazier@canonical.com>

Add a path for UEFI VMs for AArch32 VMs, based on the path Debian is using.
libvirt is the de facto canonical location for defining where distros
should place these firmware images, so let's define this path here to try
and minimize distro fragmentation.

The call to qemuBuildDeviceAddressStr() happens no matter
what, so we can move it to the outer possible scope inside
the function.

We can also move the call to virBufferAsprintf() after all
the checks have been performed, where it makes more sense.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>