This patch fixes a bug which occurs when we check a bus and unit number
for a new attached disk. We should do this check in ValidadionCallback,
not in PostParse callback. Because in PostParse we have not initialized
disk->info.addr.drive struct yet.
Move part of code from domainPostParseCallback to domainValidateCallback
and part from devicesPostParseCallback to deviceValidateCallback.
PostParse callbacks are for modification data.
ValidateCallbacks are only for checks.
Unfortunately vz sdk do not provide detail information on migration
progress, only progress percentage. Thus vz driver provides percents
instead of bytes in data fields of virDomainJobInfoPtr.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Current vz driver implementation is not usable when it comes to
long runnig operations. Migration or saving a domain blocks all
other operations even query ones which are expecteted to be available.
This patch addresses this problem.
All vz driver API calls fall into next 3 groups:
1. only query domain cache (virDomainObj, vz cache statistic)
examples are vzDomainGetState, vzDomainGetXMLDesc etc.
2. use thread shared sdkdom object
examples are vzDomainSetMemoryFlags, vzDomainAttachDevice etc.
3. use no thread shared sdkdom object nor domain cache
examples are vzDomainSnapshotListNames, vzDomainSnapshotGetXMLDesc etc
API calls from group 1 don't need to be changed as they hold domain lock only
for short period of time. These calls [1] are easily distinguished. They query
domain object thru libvirt common code or query vz sdk statistics handle thru
vz sdk sync operations.
vzDomainInterfaceStats is the only exception. It uses sdkdom object to
convert interface name to its vz sdk stack index which could not be saved in
domain cache. Interface statistics is available thru this stack index as a key
rather than name. As a result we can have accidental 'not known interface'
errors on quering intrerface stats. The reason is that in the process of
updating domain configuration we drop all devices and then recreate them again
in sdkdom object and domain lock can be dropped meanwhile (to remove networks
for existing bridged interfaces and(or) (re)create new ones). We can fix this
by changing the way we support bridged interfaces or by reordering operations
and changing bridged networks beforehand. Anyway this is better than moving
this API call into 2 group and making it an exclusive job.
As to API calls from group 2, first thread shared sdkdom object needs to be
explained. vz sdk has only one handle for a given domain, thus threads need
exclusive access to operate on it. These calls are fixed to drop and reacquire
domain lock on any lengthy operations - namely waiting the result of async vz
sdk operation. As lock is dropped we need to take extra reference to domain
object if it is not taken already as domain object can be deleted from list
while lock is dropped. As this operations use thread shared sdkdom object, the
simplest way to make calls from group 2 be consistent to each other is to make
them mutually exclusive. This is done by taking/releasing job condition thru
calling correspondent job routine. This approach makes group 1 and group
2 calls consistent to each other too. Not all calls of group 2 change the
domain cache but those that do update it thru prlsdkUpdateDomain which holds
the lock thoughout the update.
API calls from group [2] are easily distinguished too. They use
beginEdit/commit to change domain configuration (vzDomainSetMemoryFlags) or/and
update domain cache from sdkdom at the end of operation (vzDomainSuspend).
There is a known issue however. Frankly speaking it was introduced by ealier
patch '[PATCH 6/9] vz: cleanup loading domain code' from a different series.
The patch significantly reduced amount of time when the driver lock is held when
creating domain from API call or as a result of domain added event from vz sdk.
The problem is these two paths race on using thread shared sdkdom as we don't
have libvirt domain object and can not lock on it. However this don't
invalidates the patch as we can't use the former approach of preadding domain
into the list as we need name at least and name is not given by event. Anyway
i'm against adding half baked object into the list. Eventually this race can be
fixed by extra measures. As to current situation races with different
configurations are unlikely and race when adding domain thru vz driver and
simultaneous event from vz sdk is not dangerous as configuration is the same.
The last group [3] is API calls that need only sdkdom object to make vz sdk
call and don't change thread shared sdkdom object or domain cache in any way.
For now these are mostly domain snapshot API calls. The changes are similar to
those of group 2 - they add extra reference and drop/reacquire the lock on waiting
vz async call result. One can simply take the immutable sdkdom object from the
cache and drop the lock for the rest of operations but the chosen approach
makes implementation of these API calls somewhat similar to those of from group
2 and thus a bit futureproof. As calls of group 3 don't need vz driver
domain/vz sdk cache in any way, they are consistent with respect to API calls from
groups 1 and 3.
There is another exception. Calls to make-snapshot/revert-to-snapshot/migrate
are moved to group 2. That is they are made mutually exclusive. The reason
is that libvirt API supports control/query only for one job per domain and
these are jobs that are likely to be queried/aborted.
Appendix.
[1] API calls that only query domain cache.
(marked [*] are included for a different reason)
.domainLookupByID = vzDomainLookupByID, /* 0.10.0 */
.domainLookupByUUID = vzDomainLookupByUUID, /* 0.10.0 */
.domainLookupByName = vzDomainLookupByName, /* 0.10.0 */
.domainGetOSType = vzDomainGetOSType, /* 0.10.0 */
.domainGetInfo = vzDomainGetInfo, /* 0.10.0 */
.domainGetState = vzDomainGetState, /* 0.10.0 */
.domainGetXMLDesc = vzDomainGetXMLDesc, /* 0.10.0 */
.domainIsPersistent = vzDomainIsPersistent, /* 0.10.0 */
.domainGetAutostart = vzDomainGetAutostart, /* 0.10.0 */
.domainGetVcpus = vzDomainGetVcpus, /* 1.2.6 */
.domainIsActive = vzDomainIsActive, /* 1.2.10 */
.domainIsUpdated = vzDomainIsUpdated, /* 1.2.21 */
.domainGetVcpusFlags = vzDomainGetVcpusFlags, /* 1.2.21 */
.domainGetMaxVcpus = vzDomainGetMaxVcpus, /* 1.2.21 */
.domainHasManagedSaveImage = vzDomainHasManagedSaveImage, /* 1.2.13 */
.domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */
.domainBlockStats = vzDomainBlockStats, /* 1.2.17 */
.domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
.domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ [*]
.domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
.domainMigrateBegin3Params = vzDomainMigrateBegin3Params, /* 1.3.5 */
.domainMigrateConfirm3Params = vzDomainMigrateConfirm3Params, /* 1.3.5 */
[2] API calls that use thread shared sdkdom object
(marked [*] are included for a different reason)
.domainSuspend = vzDomainSuspend, /* 0.10.0 */
.domainResume = vzDomainResume, /* 0.10.0 */
.domainDestroy = vzDomainDestroy, /* 0.10.0 */
.domainShutdown = vzDomainShutdown, /* 0.10.0 */
.domainCreate = vzDomainCreate, /* 0.10.0 */
.domainCreateWithFlags = vzDomainCreateWithFlags, /* 1.2.10 */
.domainReboot = vzDomainReboot, /* 1.3.0 */
.domainDefineXML = vzDomainDefineXML, /* 0.10.0 */
.domainDefineXMLFlags = vzDomainDefineXMLFlags, /* 1.2.12 */ (update part)
.domainUndefine = vzDomainUndefine, /* 1.2.10 */
.domainAttachDevice = vzDomainAttachDevice, /* 1.2.15 */
.domainAttachDeviceFlags = vzDomainAttachDeviceFlags, /* 1.2.15 */
.domainDetachDevice = vzDomainDetachDevice, /* 1.2.15 */
.domainDetachDeviceFlags = vzDomainDetachDeviceFlags, /* 1.2.15 */
.domainSetUserPassword = vzDomainSetUserPassword, /* 1.3.6 */
.domainManagedSave = vzDomainManagedSave, /* 1.2.14 */
.domainSetMemoryFlags = vzDomainSetMemoryFlags, /* 1.3.4 */
.domainSetMemory = vzDomainSetMemory, /* 1.3.4 */
.domainRevertToSnapshot = vzDomainRevertToSnapshot, /* 1.3.5 */ [*]
.domainSnapshotCreateXML = vzDomainSnapshotCreateXML, /* 1.3.5 */ [*]
.domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.3.5 */ [*]
.domainUpdateDeviceFlags = vzDomainUpdateDeviceFlags, /* 2.0.0 */
prlsdkHandleVmConfigEvent
[3] API calls that do not use thread shared sdkdom object
.domainManagedSaveRemove = vzDomainManagedSaveRemove, /* 1.2.14 */
.domainSnapshotNum = vzDomainSnapshotNum, /* 1.3.5 */
.domainSnapshotListNames = vzDomainSnapshotListNames, /* 1.3.5 */
.domainListAllSnapshots = vzDomainListAllSnapshots, /* 1.3.5 */
.domainSnapshotGetXMLDesc = vzDomainSnapshotGetXMLDesc, /* 1.3.5 */
.domainSnapshotNumChildren = vzDomainSnapshotNumChildren, /* 1.3.5 */
.domainSnapshotListChildrenNames = vzDomainSnapshotListChildrenNames, /* 1.3.5 */
.domainSnapshotListAllChildren = vzDomainSnapshotListAllChildren, /* 1.3.5 */
.domainSnapshotLookupByName = vzDomainSnapshotLookupByName, /* 1.3.5 */
.domainHasCurrentSnapshot = vzDomainHasCurrentSnapshot, /* 1.3.5 */
.domainSnapshotGetParent = vzDomainSnapshotGetParent, /* 1.3.5 */
.domainSnapshotCurrent = vzDomainSnapshotCurrent, /* 1.3.5 */
.domainSnapshotIsCurrent = vzDomainSnapshotIsCurrent, /* 1.3.5 */
.domainSnapshotHasMetadata = vzDomainSnapshotHasMetadata, /* 1.3.5 */
.domainSnapshotDelete = vzDomainSnapshotDelete, /* 1.3.5 */
[4] Known issues.
1. accidental errors on getting network statistics
2. race with simultaneous use of thread shared domain object on paths
of adding domain thru API and adding domain on vz sdk domain added event.
9c14a9ab introduced vzNewDomain function to enlist libvirt domain
object before actually creating vz sdk domain. Fix should fix
race on same vz sdk domain added event where libvirt domain object is
enlisted too. But later eb5e9c1e added locked checks for
adding livirtd domain object to list on vz sdk domain added event.
Thus now approach of 9c14a9ab is unnecessary complicated.
See we have otherwise unuseful prlsdkGetDomainIds function only
to create minimal domain definition to create libvirt domain object.
Also vzNewDomain is difficult to use as it creates partially
constructed domain object.
Let's move back to original approach where prlsdkLoadDomain do
all the necessary job. Another benefit is that we can now
take driver lock for bare minimum and in single place. Reducing
locking time have small disadvatage of double parsing on race
conditions which is typical if domain is added thru vz driver.
Well we have this double parse inevitably with current vz sdk api
on any domain updates so i would not take it here seriously.
Performance events subscribtion is done before locked check and
therefore could be done twice on races but this is not the problem.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Move graphic device config to post parse. This way we
detect error on early stage and leverage checking on detach too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
This is as easy as moving disks checks from domain post
parse callback to device post parse callback.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
The approach of subscribing on first stat API call and then waiting
for receiving of performance event from sdk to process the call originates
in times when every vz libvirt connections spawns its own sdk connection.
Thus without this waiting virsh stat call would return empty stats. Now
with single sdk connection this scheme is unnecessary complicated.
This patch subscribes to performance events on first domain appearence
and unsubscribe on its removing.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
vz puts uuids into curly braces. Simply introduce new contstant to reflect this
and get rid of magic +2 in code.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
This session uuid acts as authN token for different multihost vz operations one
of which is migration. Unfortunately we can't get it from server at any time
thus we need to save it at login.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
This solution does not keep snapshots cache because vz sdk lacks good support
for snapshot related events.
Libvirt and vz sdk has different approach to snapshot ids. vz sdk always
auto generate them while libvirt has ability to specify id by user.
Thus I have no other choice rather than simply ignore ids set by user
or generated by libvirt.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
We don't need them anymore as all pointers within vzDriver structure
are not changed during the time it exists.
Where we still need to synchronize we use virObjectLock/Unlock as far
as vzDriver is lockable object.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
This patch introduces a new 'vzDriver' lockable object and provides
helper functions to allocate/destroy it and we pass it to prlsdkXxx
functions instead of virConnectPtr.
Now we store domain related objects such as domain list, capabitilies
etc. within a single vz_driver vzDriver structure, which is shared by
all driver connections. It is allocated during daemon initialization or
in a lazy manner when a new connection to 'vz' driver is established.
When a connection to vz daemon drops, vzDestroyConnection is called,
which in turn relays disconnect event to all connection to 'vz' driver.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
No need to remember connection name and have corresponding
domain type to keep backward compatibility with former
'parallels' driver. It is enough to be able to accept 'parallels'
uri and domain types.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Each version of virtuozzo supports only one type of SCSI controller
So if we add disk on SCSI bus, we should set SCSI controller model.
We can take it from vzCapabilities structure.
Because Vz6 supports SCSI(BUSLOGIC), IDE and SATA controllers only and
Vz7 supports SCSI(VIRTIO_SCSI) and IDE only we add list of supported
controllers and scsi models to vzCapabilities structure.
When a new connection opens, we select proper capabilities values according
to Virtuozzo version and check them in XMLPostParse.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
As far as Virtuozzo6 and Virtuozzo7 support different disk types for virtual
machines (ploop and qcow2 respectively) and different buses (vz6: IDE, SCSI,
SATA; vz7: IDE SCSI) we add vzCapabilities structure to help undestand which
disk formats and buses are supported in the context of a current connection.
When a new connection opens, we select proper capabilities in accordance to
current Virtuozzo version.
Race condition:
User calls defineXML to create new instance.
The main thread from vzDomainDefineXMLFlags() creates new instance by prlsdkCreateVm.
Then this thread calls prlsdkAddDomain to add new domain to domains list.
The second thread receives notification from hypervisor that new VM was created.
It calls prlsdkHandleVmAddedEvent() and also tries to add new domain to domains list.
These two threads call virDomainObjListFindByUUID() from prlsdkAddDomain() and don't find new domain.
So they add two domains with the same uuid to domains list.
This fix splits logic of prlsdkAddDomain() into two functions.
1. vzNewDomain() creates new empty domain in domains list with the specific uuid.
2. prlsdkLoadDomain() add data from VM to domain object.
New algorithm for creating an instance:
In vzDomainDefineXMLFlags() we add new domain to domain list by calling vzNewDomain()
and only after that we call CreateVm() to create VM.
It means that we "reserve" domain object with the specific uuid.
After creation of new VM we add info from this VM
to reserved domain object by calling prlsdkLoadDomain().
Before this patch prlsdkLoadDomain() worked in 2 different cases:
1. It creates and initializes new domain. Then updates it from sdk handle.
2. It updates existed domain from sdk handle.
In this patch we remove code which creates new domain from LoadDomain()
and move it to vzNewDomain().
Now prlsdkLoadDomain() only updates domain from skd handle.
In notification handler prlsdkHandleVmAddedEvent() we check
the existence of a domain and if it doesn't exist we add new domain by calling
vzNewDomain() and load info from sdk handle via prlsdkLoadDomain().
Our domain_conf.* files are big enough. Not only they contain XML
parsing code, but they served as a storage of all functions whose
name is virDomain prefixed. This is just wrong as it gathers not
related functions (and modules) into one big file which is then
harder to maintain. Split virDomainObjList module into a separate
file called virdomainobjlist.[ch].
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In fact, it was never used as far as vz has no features supporting it.
That is why there will be no harm to anyone if we just remove this code to
prevent further misunderstanding and efforts to support dead code.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
At the time this code was added we had intentions to support libvirt interface
to manage vz networks. In fact, it was never implemented completely to work
correctly that makes me think that there will be no harm to anyone if we just
rip it off. Moreover, in vz7 we started to use libvirt bridge network driver to
manage networks.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
This patch moves all src/parallels/parallels* files to vz/vz*
and fixes build accordingly.
No functional changes.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com>