Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Prefer the GLib version of the macro.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Since qemuDomainDefPostParse callback requires qemuCaps, we need to make
sure it gets the capabilities stored in the domain's private data if the
domain is running. Passing NULL may cause QEMU capabilities probing to
be triggered in case QEMU binary changed in the meantime. When this
happens while a running domain object is locked, QMP event delivered to
the domain before QEMU capabilities probing finishes will deadlock the
event loop.

Several general functions from domain_conf.c were lazily passing NULL as
the parseOpaque pointer instead of letting their callers pass the right
data. This patch fixes all paths leading to virDomainDefCopy to do the
right thing.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>). VIR_ONCE_GLOBAL_INIT is almost
exclusively called without an ending semicolon, but let's
standardize on using one like the other macros.

Add a dummy struct definition at the end of the macro, so
the compiler will require callers to add a semicolon.
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>

In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.

In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.

With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to  find the
author of a particular bit of code.

This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.

The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NYuri Chornoivan <yurchor@ukr.net>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

When libxlDomainMigrationDstPrepare adds the @args to an
virNetSocketAddIOCallback using libxlMigrateDstReceive as
the target of the virNetSocketIOFunc @func with the knowledge
that the libxlMigrateDstReceive will virObjectUnref @args
at the end thus not needing to Unref during normal processing
for libxlDomainMigrationDstPrepare.

However, Coverity believes there's an issue with this. The
problem is there can be @nsocks virNetSocketAddIOCallback's
added, but only one virObjectUnref. That means the first
one done will Unref and the subsequent callers may not get
the @args (or @opaque) as they expected. If there's only
one socket returned from virNetSocketNewListenTCP, then sure
that works. However, if it returned more than one there's
going to be a problem.

To resolve this, since we start with 1 reference from the
virObjectNew for @args, we will add 1 reference for each
time @args is used for virNetSocketAddIOCallback. Then
since libxlDomainMigrationDstPrepare would be done with
@args, move it's virObjectUnref from the error: label to
the done: label (since error: falls through). That way
once the last IOCallback is done, then @args will be freed.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

It is possible the incoming VM is not fully started when the finish
phase of migration is executed. In libxlDomainMigrationDstFinish,
wait for the thread receiving the VM to complete before executing
finish phase tasks.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

The libxlDomainMigrationDst* functions are a bit flawed in their
handling of modify jobs. A job begins when the destination host
begins receiving the incoming VM and ends after the VM is started.
The finish phase contains another BeginJob/EndJob sequence.

This patch changes the logic to begin a job for the incoming VM
in the prepare phase and end the job in the finish phase.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

The libxlDomainMigrationSrc* functions are a bit flawed in their
handling of modify jobs. A job begins at the start of the begin
phase but ends before the phase completes. No job is running for
the remaining phases of migration on the source host.

Change the logic to keep the job running after a successful begin
phase, and end the job in the confirm phase. The job must also end
in the perform phase in the case of error since confirm phase would
not be executed.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

libxlDoMigrateSrcP2P() performs all phases of the migration
protocol for peer-to-peer migration. Unfortunately the logic
was a bit flawed since it is possible to skip the confirm
phase after a successfull begin and prepare phase. Fix the
logic to always call the confirm phase after a successful begin
and perform. Skip the confirm phase if begin or perform fail.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

If for any reason the restore of a VM fails on the destination host
in a migration operation, the VM is removed (if not persistent) from
the virDomainObjList, meaning it is no longer available for additional
cleanup or processing in the finish phase. Defer removing the VM from
the virDomainObjList until the finish phase, which already contains
logic to remove the VM.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

And replace all calls with virObjectEventStateQueue such that:

    libxlDomainEventQueue(driver, event);

becomes:

    virObjectEventStateQueue(driver->domainEventState, event);

And remove NULL checking from all callers.
Signed-off-by: NAnya Harter <aharter@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

When adding a new object to the domain object list, there should
have been 2 virObjectRef calls made one for each list into which
the object was placed to match the 2 virObjectUnref calls that
would occur during Remove as part of virHashRemoveEntry when
virObjectFreeHashData is called when the element is removed from
the hash table as set up in virDomainObjListNew.

Some drivers (libxl, lxc, qemu, and vz) handled this inconsistency
by calling virObjectRef upon successful return from virDomainObjListAdd
in order to use virDomainObjEndAPI when done with the returned @vm.
While others (bhyve, openvz, test, and vmware) handled this via only
calling virObjectUnlock upon successful return from virDomainObjListAdd.

This patch will "unify" the approach to use virDomainObjEndAPI
for any @vm successfully returned from virDomainObjListAdd.

Because list removal is so tightly coupled with list addition,
this patch fixes the list removal algorithm to return the object
as entered - "locked and reffed".  This way, the callers can then
decide how to uniformly handle add/remove success and failure.
This removes the onus on the caller to "specially handle" the
@vm during removal processing.

The Add/Remove logic allows for some logic simplification such
as in libxl where we can Remove the @vm directly rather than
needing to set a @remove_dom boolean and removing after the
libxlDomainObjEndJob completes as the @vm is locked/reffed.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Since the @dconn reference via args->conn will be used via a thread
or callback, let's make sure memory associated with it isn't free'd
unexpectedly before we use it. The Unref will be done when the object
is Dispose'd.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

When adding the @vm to the @args for usage during a thread or
callback, let's add the reference to it at the time of adding to
ensure nothing else deletes it. The corresponding Unref is then
added to the Dispose function.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

So far we are repeating the following lines over and over:

  if (!(virSomeObjectClass = virClassNew(virClassForObject(),
                             "virSomeObject",
                             sizeof(virSomeObject),
                             virSomeObjectDispose)))
      return -1;

While this works, it is impossible to do some checking. Firstly,
the class name (the 2nd argument) doesn't match the name in the
code in all cases (the 3rd argument). Secondly, the current style
is needlessly verbose. This commit turns example into following:

  if (!(VIR_CLASS_NEW(virSomeObject,
                      virClassForObject)))
      return -1;
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Inspired by commit ffb7954f to improve readability of the libxl
migration APIs.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

In libxlDomainMigrationPrepare it is possible to dereference a NULL
libxlDomainObjPrivatePtr in early error paths. Check for a valid
'priv' before using it.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Most libxl driver API use the pattern of lock and add a ref to
virDomainObj, perform API, then decrement ref and unlock in
virDomainEndAPI. In some cases the API may call
virDomainObjListRemove, which unlocks the virDomainObj. Relock
the object in such cases so EndAPI is called with a locked object.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

libxlDomainMigrationPrepare adds the incoming domain def to the list
of domains via virDomainObjListAdd, but never adds its own ref to the
returned virDomainObj as other callers of virDomainObjListAdd do.
libxlDomainMigrationPrepareTunnel3 suffers the same discrepancy.

Change both to add a ref to the virDomainObj after a successful
virDomainObjListAdd, similar to other callers. This ensures a consistent
pattern throughout the drivers and allows using the virDomainObjEndAPI
function for cleanup.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The libxlDomainMigrateConfirm3Params API locks and ref counts the associated
virDomainObj but relies on the helper function libxlDomainMigrationConfirm
to unlock the object. Unref'ing the object is not done in either function.
libxlDomainMigrationConfirm is also used by libxlDomainMigratePerform3Params
for p2p migration, but in that case the lock/ref and unref/unlock are
properly handled in the API entry point.

Remove the unlock from libxlDomainMigrationConfirm and adjust
libxlDomainMigrateConfirm3Params to properly unref/unlock the virDomainObj
on success and error paths.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The libxlDomainMigrateBegin3Params API locks and ref counts the associated
virDomainObj but relies on the helper function libxlDomainMigrationBegin
to unref/unlock the object. libxlDomainMigrationBegin is also used by
libxlDomainMigratePerform3Params for p2p migration, but in that case the
lock/ref and unref/unlock are properly handled in the API entry point. So
p2p migrations suffer a double unref/unlock in the Perform API.

Remove the unref/unlock (virDomainObjEndAPI) from libxlDomainMigrationBegin
and adjust libxlDomainMigrateBegin3Params to properly unref/unlock
the virDomainObj on success and error paths.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Range check in virPortAllocatorSetUsed is not useful anymore
when we manage ports for entire unsigned short range values.
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>

During migration, the lock process is paused in the perform phase
but not resumed if there is a subsequent failure, leaving the locked
resource unprotected.

The perform phase itself can fail, in which case the lock process
should be resumed before returning from perform. The finish phase
could also fail on the destination host, in which case the migration
is canceled in the confirm phase and the VM is resumed. The lock
process needs to be resumed there as well.
Signed-off-by: NJim Fehlig <jfehlig@suse.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: NMichal 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: NMartin Kletzander <mkletzan@redhat.com>

As discussed here [0][1] Coverity reported two issues:

- On libxlDomainMigrationPrepareTunnel3 @@mig will be leaked on failures
after sucessfull call libxlDomainMigrationPrepareAny hence we free it.

Setting mig = NULL after @mig is assigned plus adding libxlMigrationCookieFree
on error paths addresses the issue. In case virThreadCreate fails,
unref of args frees the cookie on dispose function (libxlMigrationDstArgsDispose)

- On libxlMigrationStartTunnel @tc would be leaked.

Fixed by correctly saving the newly allocated @tc onto @tnl such that
libxlMigrationStopTunnel would free it up.

[0] https://www.redhat.com/archives/libvir-list/2017-February/msg00791.html
[1] https://www.redhat.com/archives/libvir-list/2017-February/msg00833.htmlSigned-off-by: NJoao Martins <joao.m.martins@oracle.com>

Tunnelled migration doesn't require any extra network connections
beside the libvirt daemon.  It's capable of strong encryption and the
default option of openstack-nova.

This patch adds the tunnelled migration(Tunnel3params) support to
libxl.  On the source side, the data flow is:

 * libxlDoMigrateSend() -> pipe libxlTunnel3MigrationFunc() polls pipe
 * out and then write to dest stream.

While on the destination side:
 * Stream -> pipe -> 'recvfd of libxlDomainStartRestore'

The usage is the same as p2p migration, execpt adding one extra
'--tunnelled' to the libvirt p2p migration command.
Signed-off-by: NBob Liu <bob.liu@oracle.com>
Signed-off-by: NJoao Martins <joao.m.martins@oracle.com>

The newly introduced function libxlDomainMigrationPrepareAny
will be shared between P2P and tunnelled variations.
Signed-off-by: NJoao Martins <joao.m.martins@oracle.com>

Although the migration port is immediately released in the
finish phase of migration, it was never set in the domain
private object when allocated in the prepare phase. So
libxlDomainMigrationFinish() always released a 0-initialized
migrationPort, leaking any allocated port. After enough
migrations to exhaust the migration port pool, migration would
fail with

error: internal error: Unable to find an unused port in range
       'migration' (49152-49216)

Fix it by setting libxlDomainObjPrivate->migrationPort to the
port allocated in the prepare phase. While at it, also fix
leaking an allocated port if the prepare phase fails.

We want to pass the proper opaque pointer instead of NULL to
virDomainDefParse and subsequently virDomainDefParseNode too.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

By default, virt-manager (and likely other libvirt-based apps) sets
the VIR_MIGRATE_PERSIST_DEST flag when invoking the migrate API, which
fails in a Xen setup since the libxl driver does not support the flag.

Persisting a domain is a trivial task in the grand scheme of migration,
so be nice to libvirt apps and add support for VIR_MIGRATE_PERSIST_DEST
in the libxl driver.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

In the cleanup path, @vm cannot be possibly NULL. If it were so,
we would receive SIGSEGV much earlier. At the beginning of the
function we do libxlDomainObjBeginJob(.., vm, ..); and so on.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Introduce libxl hook and use it for start, prepare, started,
stop, stopped, migrate events.

In libxl driver we do virObjectRef in libxlDomainObjBeginJob,
If virCondWaitUntil failed, it goes to error, do virObjectUnref,
There's a chance that someone undefine the vm at the same time,
and refs unref to zero, vm is freed in libxlDomainObjBeginJob.
But the vm outside function is not Null, we do virObjectUnlock(vm).
That's how we overwrite the vm memory after it's freed. I fix it.
Signed-off-by: NWang Yufei <james.wangyufei@huawei.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Until now we weren't able to add checks that would reject configuration
once accepted by the parser. This patch adds a new callback and
infrastructure to add such checks. In this patch all the places where
rejecting a now-invalid configuration wouldn't be a good idea are marked
with a new parser flag.