Now that we have a private storage pool list, we can take the next
step and convert to using objects. In this case, we're going to use
RWLockable objects (just like every other driver) with two hash
tables for lookup by UUID or Name.
Along the way the ForEach and Search API's will be adjusted to use
the related Hash API's and the various FindBy functions altered and
augmented to allow for HashLookup w/ and w/o the pool lock already
taken.
After virStoragePoolObjRemove we will need to virObjectUnref(obj)
after to indicate the caller is "done" with it's reference. The
Unlock occurs during the Remove.
The NumOf, GetNames, and Export functions all have their own callback
functions to return the required data and the FindDuplicate code
can use the HashSearch function callbacks.
Create an API to search through the storage pool objects looking for
a specific truism from a callback API in order to return the specific
storage pool object that is desired.
For now it'll just call the virStoragePoolObjUnlock, but a future
adjustment will do something different. Since the new API will check
for a NULL object before the Unlock call, callers no longer need to
check for NULL before calling.
The virStoragePoolObjUnlock is now private/static to virstorageobj.c
with a short term forward reference.
All APIs which expect a list of CPU models supported by hypervisors were
switched from char **models and int models to just accept a pointer to
virDomainCapsCPUModels object stored in domain capabilities. This avoids
the need to transform virDomainCapsCPUModelsPtr into a NULL-terminated
list of model names and also allows the various cpu driver APIs to
access additional details (such as its usability) about each CPU model.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1497396
The other APIs accept both, ifname and MAC address. There's no
reason virDomainInterfaceStats can't do the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Every caller reports the error themselves. Might as well move it
into the function and thus unify it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In the past we updated host-model CPUs with host CPU data by adding a
model and features, but keeping the host-model mode. And since the CPU
model is not normally formatted for host-model CPU defs, we had to pass
the updateCPU flag to the formatting code to be able to properly output
updated host-model CPUs. Libvirt doesn't do this anymore, host-model
CPUs are turned into custom mode CPUs once updated with host CPU data
and thus there's no reason for keeping the hacks inside CPU XML
formatters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This commit adds new events for two methods and operations: *PoolBuild() and
*PoolDelete(). Using the event-test and the commands set below we have the
following outputs:
$ sudo ./event-test
Registering event callbacks
myStoragePoolEventCallback EVENT: Storage pool test Defined 0
myStoragePoolEventCallback EVENT: Storage pool test Created 0
myStoragePoolEventCallback EVENT: Storage pool test Started 0
myStoragePoolEventCallback EVENT: Storage pool test Stopped 0
myStoragePoolEventCallback EVENT: Storage pool test Deleted 0
myStoragePoolEventCallback EVENT: Storage pool test Undefined 0
Another terminal:
$ sudo virsh pool-define test.xml
Pool test defined from test.xml
$ sudo virsh pool-build test
Pool test built
$ sudo virsh pool-start test
Pool test started
$ sudo virsh pool-destroy test
Pool test destroyed
$ sudo virsh pool-delete test
Pool test deleted
$ sudo virsh pool-undefine test
Pool test has been undefined
This commits can be a solution for RHBZ #1475227.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1475227
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Create/use virStoragePoolObjAddVol in order to add volumes onto list.
Create/use virStoragePoolObjRemoveVol in order to remove volumes from list.
Create/use virStoragePoolObjGetVolumesCount to get count of volumes on list.
For the storage driver, the logic alters when the volumes.obj list grows
to after we've fetched the volobj. This is an optimization of sorts, but
also doesn't "needlessly" grow the volumes.objs list and then just decr
the count if the virGetStorageVol fails.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The implementation of virConnectBaselineCPU may be different for each
hypervisor. Thus it shouldn't really be implmented in the cpu code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
To be consistent with the API definition, use the @maxnames instead
of @nnames when describing/comparing against the maximum names to
be provided for the *ConnectList[Defined]Networks APIs.
Signed-off-by: John Ferlan <jferlan@redhat.com>
In preparation to privatize the virNetworkObj - create an accessor function
to get the current @persistent value. Also change the value to a bool rather
than an unsigned int (since that's how it's generated anyway).
Signed-off-by: John Ferlan <jferlan@redhat.com>
In order to privatize the virNetworkObj create accessors in virnetworkobj
in order to handle the get/set of the active value.
Also rather than an unsigned int, convert it to a boolean to match other
drivers representation and the reality of what it is.
Signed-off-by: John Ferlan <jferlan@redhat.com>
In preparation for making the object private, create a couple of API's
to get the obj->def & obj->newDef and set the obj->def.
While altering networkxml2conftest.c to use the virNetworkObjSetDef
API, fix the name of the variable from @dev to @def
Signed-off-by: John 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.
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.
Alter the virStoragePoolObjNumOfVolumes, virStoragePoolObjVolumeGetNames,
and virStoragePoolObjVolumeListExport APIs to take a virStoragePoolObjPtr
instead of the &obj->volumes and obj->def.
Signed-off-by: John 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: John 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: John Ferlan <jferlan@redhat.com>
Now that we have a bit more control, let's convert our object into
a lockable object and let that magic handle the create and lock/unlock.
This also involves creating a virNodeDeviceEndAPI in order to handle
the object cleanup for API's that use the Add or Find API's in order
to get a locked/reffed object. The EndAPI will unlock and unref the
object returning NULL to indicate to the caller to not use the obj.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Ensure that any function that walks the node device object list is prefixed
by virNodeDeviceObjList.
Also, modify the @filter param name for virNodeDeviceObjListExport to
be @aclfilter.
Signed-off-by: John Ferlan <jferlan@redhat.com>
In preparation to make things private, make the ->devs be pointers to a
virNodeDeviceObjList and then manage everything inside virnodedeviceobj
Signed-off-by: John Ferlan <jferlan@redhat.com>
- In testDestroyVport rather than use a cleanup label, just return -1
immediately since nothing else is needed.
- In testStoragePoolDestroy, if !privpool, then just return -1 since
nothing else will happen anyway.
- Rather than "goto cleanup;" on failure to virNodeDeviceObjFindByName
an @obj, just return directly. This then allows the cleanup: label code
to not have to check "if (obj)" before calling virNodeDeviceObjUnlock.
This also simplifies some exit logic...
- In testNodeDeviceObjFindByName use an error: label to handle the failure
and don't do the ncaps++ within the VIR_STRDUP() source target index.
Only increment ncaps after success. Easier on eyes at error label too.
- In testNodeDeviceDestroy use "cleanup" rather than "out" for the goto
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than passing the object to be removed by reference, pass by value
and then let the caller decide whether or not the object should be free'd
and how to handle the logic afterwards. This includes free'ing the object
and/or setting the local variable to NULL to prevent subsequent unexpected
usage (via something like virNodeDeviceObjRemove in testNodeDeviceDestroy).
For now this function will just handle the remove of the object from the
list for which it was placed during virNodeDeviceObjAssignDef.
This essentially reverts logic from commit id '61148074' that free'd the
device entry on list, set *dev = NULL and returned. Thus fixing a bug in
node_device_hal.c/dev_refresh() which would never call dev_create(udi)
since @dev would have been set to NULL.
Signed-off-by: John Ferlan <jferlan@redhat.com>
If a remote call fails during event registration (more than likely from
a network failure or remote libvirtd restart timed just right), then when
calling the virObjectEventStateDeregisterID we don't want to call the
registered @freecb function because that breaks our contract that we
would only call it after succesfully returning. If the @freecb routine
were called, it could result in a double free from properly coded
applications that free their opaque data on failure to register, as seen
in the following details:
Program terminated with signal 6, Aborted.
#0 0x00007fc45cba15d7 in raise
#1 0x00007fc45cba2cc8 in abort
#2 0x00007fc45cbe12f7 in __libc_message
#3 0x00007fc45cbe86d3 in _int_free
#4 0x00007fc45d8d292c in PyDict_Fini
#5 0x00007fc45d94f46a in Py_Finalize
#6 0x00007fc45d960735 in Py_Main
#7 0x00007fc45cb8daf5 in __libc_start_main
#8 0x0000000000400721 in _start
The double dereference of 'pyobj_cbData' is triggered in the following way:
(1) libvirt_virConnectDomainEventRegisterAny is invoked.
(2) the event is successfully added to the event callback list
(virDomainEventStateRegisterClient in
remoteConnectDomainEventRegisterAny returns 1 which means ok).
(3) when function remoteConnectDomainEventRegisterAny is hit,
network connection disconnected coincidently (or libvirtd is
restarted) in the context of function 'call' then the connection
is lost and the function 'call' failed, the branch
virObjectEventStateDeregisterID is therefore taken.
(4) 'pyobj_conn' is dereferenced the 1st time in
libvirt_virConnectDomainEventFreeFunc.
(5) 'pyobj_cbData' (refered to pyobj_conn) is dereferenced the
2nd time in libvirt_virConnectDomainEventRegisterAny.
(6) the double free error is triggered.
Resolve this by adding a @doFreeCb boolean in order to avoid calling the
freeCb in virObjectEventStateDeregisterID for any remote call failure in
a remoteConnect*EventRegister* API. For remoteConnect*EventDeregister* calls,
the passed value would be true indicating they should run the freecb if it
exists; whereas, it's false for the remote call failure path.
Patch based on the investigation and initial patch posted by
fangying <fangying1@huawei.com>.
virDomainXMLOption gains driver specific callbacks for parsing and
formatting save cookies.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This will be used later when a save cookie will become part of the
snapshot XML using new driver specific parser/formatter functions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that we have a bit more control, let's convert our object into
a lockable object and let that magic handle the create and lock/unlock.
This commit also introduces virInterfaceObjEndAPI in order to handle the
lock unlock and object unref in one call for consumers returning a NULL
obj upon return. This removes the need for virInterfaceObj{Lock|Unlock}
external API's.
Signed-off-by: John Ferlan <jferlan@redhat.com>
While checking for ABI stability, drivers might pose additional
checks that are not valid for general case. For instance, qemu
driver might check some memory backing attributes because of how
qemu works. But those attributes may work well in other drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In preparation for privatizing the virNodeDeviceObj - create an accessor
for the @def field and then use it for various callers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Alter the algorithm to return a list of matching names rather than a
list of match virInterfaceObjPtr which are then just dereferenced
extracting the def->name and def->mac. Since the def->mac would be
the same as the passed @mac, just return a list of names and as long
as there's only one, extract the [0] entry from the passed list.
Also alter the error message on failure to include the mac that wasn't
found.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move the structs into virinterfaceobj.c, create necessary accessors, and
initializers.
This also includes reworking virInterfaceObjListClone to handle receiving
a source interfaces list pointer, creating the destination interfaces object,
and copying everything from source into dest.
Signed-off-by: John Ferlan <jferlan@redhat.com>
We're about to make the obj much more private, so make it easier to
see future changes which will require accessors for the obj->def
This also includes modifying some interfaces->objs[i]->X references to be
obj = interfaces->objs[i]; and then def = obj->def
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than using goto cleanup on object find failure and having cleanup
need to check if the obj was present before unlocking, just return immediately.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit 4337bc57be introduced code that would in certain error paths
unref the last reference of a pointer, but return it.
Clear the pointers before returning them.
Commit id '865f479da' altered the logic to use a common test*ObjFindByName
helpers which would lock/unlock the test driver; however, a few cleanup paths
in that cleanup missed removing the Unlock, so remove it now.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Use "virNetworkObj" as a prefix for any external API in virnetworkobj.
Also a couple of functions were local to virnetworkobj.c, so remove their
external defs in virnetworkobj.h.
Move all the virNetworkObj related API/data structures into their own
modules virnetworkobj.{c,h} from the network_conf.{c,h}
Purely code motion at this point plus adjustments to cleanly build
Mostly code motion to move storageConnectList[Defined]StoragePools
and similar test driver code into virstorageobj.c and rename to
virStoragePoolObjGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unify the NumOf[Defined]StoragePools API into virstorageobj.c from
storage_driver and test_driver. The only real difference between the
two is the test driver doesn't call using the aclfilter API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Mostly code motion to move storagePoolListVolumes code into virstorageobj.c
and rename to virStoragePoolObjVolumeGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unify the NumOfVolumes API into virstorageobj.c from storage_driver and
test_driver. The only real difference between the two is the test driver
doesn't call using the aclfilter API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unify the *ListDevice API into virnodedeviceobj.c from node_device_driver
and test_driver. The only real difference between the two is that the test
driver doesn't call the aclfilter API. The name of the new API follows that
of other drivers to "GetNames".
NB: Change some variable names to match what they really are - consistency
with other drivers. Also added a clear of the input names.
This also allows virNodeDeviceObjHasCap to be static to virnodedeviceobj
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unify the NumOfDevices API into virnodedeviceobj.c from node_device_driver
and test_driver. The only real difference between the two is that the test
driver doesn't call the aclfilter API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Clean up the code to adhere to more of the standard two spaces between
functions, separate lines for type and function name, one argument per line.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unlike other drivers, this is a test driver only API. Still combining
the logic of testConnectListInterfaces and testConnectListDefinedInterfaces
makes things a bit easier in the long run.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Unlike other drivers, this is a test driver only API. Still combining
the logic of testConnectNumOfInterfaces and testConnectNumOfDefinedInterfaces
makes things a bit easier in the long run.
Signed-off-by: John Ferlan <jferlan@redhat.com>
So far our code is full of the following pattern:
dom = virGetDomain(conn, name, uuid)
if (dom)
dom->id = 42;
There is no reasong why it couldn't be just:
dom = virGetDomain(conn, name, uuid, id);
After all, client domain representation consists of tuple (name,
uuid, id).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There is no reason for it not to be in the utils, all global symbols
under that file already have prefix vir* and there is no reason for it
to be part of DRIVER_SOURCES because that is just a leftover from
older days (pre-driver modules era, I believe).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Move all the StoragePoolObj related API's into their own module
virstorageobj from the storage_conf
Purely code motion at this point, plus adjustments to cleanly build
Signed-off-by: John Ferlan <jferlan@redhat.com>
Fix a "bug" in the storage pool test driver code which "assumed"
testStoragePoolObjSetDefaults should fill in the configFile for
both the Define/Create (persistent) and CreateXML (transient) pools
by just VIR_FREE()'ing it during CreateXML. Because the configFile
was filled in, during Destroy the pool wouldn't be free'd which
could cause issues for future patches which add tests to validate
vHBA creation for the storage pool using the same name.
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>
Add a new test to fchosttest in order to test creation of our vHBA
via the Storage Pool logic. Unlike the real code, we cannot yet use
the virVHBA* API's because they (currently) traverse the file system
in order to get the parent vport capable scsi_host. Besides there's
no "real" NPIV device here - so we have to take some liberties, at
least for now.
Instead, we'll follow the node device tests partially in order to
create and destroy the vHBA with the test node devices.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Make common helpers testNetworkObjFindByUUID and testStoragePoolObjFindByUUID
which will replace the repeated patter for each to find objects by UUID.
As a bonus, the error message processing will also provide the failed uuidstr
rather than a generic error message.
Rather than have multiple places using the same pattern to find
a network by name using virNetworkObjFindByName, create a common
helper which will provide a consistent error message as well.
Rather than have continued repeated sequences of :
testDriverLock()
xxx = vir*ObjFindByName()
testDriverUnlock()
if (xxx == NULL) {
virReportError
goto cleanup;
}
Make some common helpers which will use the pattern and make a single
reference using a single common error message.
Altered for Interfaces, Storage Pools, Storage Volumes, and Node Devices.
For each the common error message can now also indicate which 'name' was
not found. For Storage Volumes, the "new" error will be more specific
rather than just invalid argument.
Move all the NodeDeviceObj API's into their own module virnodedeviceobj
from the node_device_conf
Purely code motion at this point, plus adjustments to cleanly build.
This function is calling public APIs (virNodeDeviceLookupByName
etc.). That requires the driver lock to be unlocked and locked
again. If we, however, replace the public APIs calls with the
internal calls (that public APIs call anyway), we can drop the
lock/unlock exercise.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Rework the code to perform the various searches by parent, parent_wwnn/
parent_wwpn, parent_fabric_wwn, or vport capable in order to return the
'parent_host' number that is vHBA capable.
The former virNodeDeviceGetParentHost is renamed to add the ByParent
on it fixes an issue where if no parent was supplied in the XML to
create the vHBA, then virNodeDeviceFindByName was called with a NULL
second parameter which had bad results.
The reworked code will make the various calls to fetch the NPIV host
by the passed parameter options or if none are provided find a vport
capable NPIV HBA to perform the create. If the call is from the delete
path, then this option won't be allowed.
Each of virNodeDeviceGetParentHostBy* functions is now static, so
remove them external definitions.
A secondary benefit of this is the test_driver now can make use of
the new API to add some new tests to test the various creation options.
Modify the code to react more like a real HBA -> vHBA creation.
Currently the code would just modify the input XML definition to
set the name to a wwpn and then modify the scsi_host capability
entry for the defintion to change the scsi_host# and unique_id
before adding that into the node device.
This patch does things a bit better. It finds and copies a known
existing vHBA (scsi_host11) in the node_device database and modifies
that definition to change the name to scsi_host12 and set the wwnn/
wwpn to what the input XML would expect before adding the def to the
node device object list.
Then rather than create a returned "dev" using the (poorly) mocked
name - perform the lookup using the new device name.
Rather than inline the dummy creation of a vHBA to add to the node
devices - create a helper to do that work.
Also just tidy up a couple of things while at it...
Predefine a second NPIV capable HBA as well as a vHBA using the first
NPIV capable HBA. This will allow for a mechanism to perform more
realistic create vHBA testing.
Alter "test-scsi-host-vport" to be "scsi_host1" to match the real
environment. This is the vport capable HBA - IOW the NPIV device.
Add more fields to scsi_host1 as well.
Alter the XML being used by the objecttest to create a vHBA in order
to match the scsi_host1 parent name and to use validateable wwnn/wwpn.
This will allow for realistic testing.
When the 'parent' was added to the virNodeDevicePtr structure
by commit id 'e8a4ea75a' the 'parent' field was not properly filled
in when a virGetNodeDevice call was made within driver/config code.
Only the device name was ever filled in. Fetching the parent required
a second trip via virNodeDeviceGetParent into the node device lookup
code was required in order to retrieve the specific parent field (and
still the parent field was never filled in although it was free'd).
Since we have the data when we initially call virGetNodeDevice from
within driver/node_config code - let's just fill in the parent field
as well for anyone that wants it without requiring another trip into
the node_device lookup just to get the parent.
This will allow API's such as virConnectListAllNodeDevices,
virNodeDeviceLookupByName, and virNodeDeviceLookupSCSIHostByWWN
to retrieve both name and parent in the returned virNodeDevicePtr.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When changing the metadata via virDomainSetMetadata, we now
emit an event to notify the app of changes. This is useful
when co-ordinating different applications read/write of
custom metadata.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When redoing the website we deleted the libvirtLogo.png file
not remembering that the test driver screenshot API impl
relied on it.
Rather than having the test driver use the logo as a side
effect, give it its own dedicated image to use. This is
installed in /usr/share/libvirt/test-screenshot.png and
is taken from a NeXT Cube running WorldWideWeb[1]. The
very first web browser in existance, running on the
hardware it was originally written on.
[1] https://en.wikipedia.org/wiki/WorldWideWeb
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
New line character in name of domain is now forbidden because it
mess virsh output and can be confusing for users.
Validation of name is done in drivers, after parsing XML to avoid
problems with dissappeared domains which was already created with
new-line char in name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Some operations like reboot, save, coreDump, blockStats,
ifaceStats make sense iff domain is running. While it is
technically possible for our test driver to return success
regardless of domain state, we should copy constraints from
other drivers and thus deny these operations over inactive
domains.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
