In "src/conf/domain_conf.h" there are many enumerations (enum)
declarations to be converted as a typedef too. As mentioned before,
it's better to use a typedef for variable types, function types and
other usages. I think this file has most of those enum declarations
at "src/conf/". So, me and Eric Blake plan to keep the cleanups all
over the source code. This time, most of the files changed in this
commit are related to part of one file: "src/conf/domain_conf.h".
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Currently, we don not acquire any job when removing a device after
DEVICE_DELETED event was received from QEMU. This means that if there is
another API running at the time DEVICE_DELETED is delivered and the API
acquired a job, we may happily change the definition of the domain the
API is working with whenever it unlocks the domain object (e.g., to talk
with its monitor). That said, we have to acquire a job before finishing
device removal to make things safe. However, doing so in the main event
loop would cause a deadlock so we need to move most of the event handler
into a separate thread.

Another good reason for both acquiring a job and handling the event in a
separate thread is that we currently remove a device backend immediately
after removing its frontend while we should only remove the backend once
we already received DEVICE_DELETED event. That is, we will have to talk
to QEMU monitor from the event handler.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

If QEMU supports DEVICE_DELETED event, we always call
qemuDomainRemoveDevice from the event handler. However, we will need to
push this call away from the main event loop and begin a job for it (see
the following commit), we need to make sure the device is fully removed
by the original thread (and within its existing job) in case the
DEVICE_DELETED event arrives before qemuDomainWaitForDeviceRemoval times
out.

Without this patch, device removals would be guaranteed to never finish
before the timeout because the could would be blocked by the original
job being still active.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Currently we don't support mixed (external + internal) snapshots. The
code detecting the snapshot type didn't make sure that the memory image
was consistent with the snapshot type leading into strange error
message:

 $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec snapshot=external,file=/tmp/blah
 error: internal error: unexpected code path

Fix the mixed detection code to detect this kind of mistake:

 $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec snapshot=external,file=/tmp/blah
 error: unsupported configuration: mixing internal and external targets for a snapshot is not yet supported

A internal snapshot of a active VM with the memory snapshot disabled
explicitly would actually still take the memory snapshot. Reject it
explicitly.

Before:
 $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec snapshot=no
 Domain snapshot 1401353155 created

After:
 $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec snapshot=no
 error: Operation not supported: internal snapshot of a running VM must include the memory state

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

Even successful start of a VM from a managed save image would spam the
logs with the following message:

Unable to restore from managed state [path]. Maybe the file is
corrupted?

Re-arrange the logic to output the warning only when the image is
corrupted.

The flaw was introduced in commit cfc28c66.

Add argument to return backing file format of a file probed by
virStorageFileGetMetadataFromFD so that it can be used in place of
virStorageFileGetMetadataFromBuf.

qemu 2.0 added the ability to commit the active layer, but slightly
differently than what libvirt had been anticipating in its
implementation of the virDomainBlockCommit call.  As a result, if
you attempt to do a 'virsh blockcommit $dom vda', qemu gets into a
state where it is waiting on libvirt to end the job, while libvirt
is waiting on qemu to end the job, and the guest is effectively
hung with regards to further commands for that block device.

I have patches coming down the pipeline that will add full support
for blockcommit of the active layer when coupled with qemu 2.0 or
later; but they depend on Peter's improvements to block job handling
and form enough of a new feature that they are not ready for
inclusion in the 1.2.5 release.  So for now, just reject the
attempt, rather than letting the user get stuck.  This is no worse
than the behavior of qemu 1.7 rejecting the job.

* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Reject active
commit.
Signed-off-by: NEric Blake <eblake@redhat.com>

QEMU ppce500 board uses the legacy -serial option.

Other PPC boards don't give any way to explicitly wire in a -chardev
except pseries which uses -device spapr-vty with -chardev.

Add test case for -serial option for ppce500
Signed-off-by: NOlivia Yin <Hong-Hua.Yin@freescale.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

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

Clean up unix socket files for chardevs using mode='bind',
like we clean up the monitor socket.
They are created by QEMU on startup and not really useful
after shutting it down.

For a clock element as above, libvirt simply converts current system
time with localtime_r(), then starts qemu with a time string that
doesn't contain any timezone information. So, from qemu's point of
view, the -rtc string it gets for:

   <clock offset='variable' basis='utc' adjustment='10800'/>

is identical to the -rtc string it gets for:

   <clock offset='variable' basis='localtime' adjustment='0'/>

(assuming the host is in a timezone that is 10800 seconds ahead of
UTC, as is the case on the machine where this message is being
written).

Since the commandlines are identical, qemu will behave identically
after this point in either case.

There are two problems in the case of basis='localtime' though:

Problem 1) If the guest modifies its RTC, for example to add 20
seconds, the RTC_CHANGE event from qemu will then contain offset:20 in
both cases. But libvirt will have saved the original adjustment into
adjustment0, and will add that value onto the offset in the
event. This means that in the case of basis=;utc', it will properly
emit an event with offset:10820, but in the case of basis='localtime'
the event will contain offset:20, which is *not* the new offset of the
RTC from UTC (as the event it documented to provide).

Problem 2) If the guest is migrated to another host that is in a
different timezone, or if it is migrated or saved/restored after the
DST status has changed from what it was when the guest was originally
started, the newly restarted guest will have a different RTC (since it
will be based on the new localtime, which could have shifted by
several hours).

The solution to both of these problems is simple - rather than
maintaining the original adjustment value along with
"basis='localtime'" in the domain status, when the domain is started
we convert the adjustment offset to one relative to UTC, and set the
status to "basis='utc'". Thus, whatever the RTC offset was from UTC
when it was initially started, that offset will be maintained when
migrating across timezones and DST settings, and the RTC_CHANGE events
will automatically contain the proper offset (which should by
definition always be relative to UTC).

This fixes a problem that was implied but not openly stated in:

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

commit e31b5cf3 attempted to fix libvirt's
VIR_DOMAIN_EVENT_ID_RTC_CHANGE, which is documentated to always
provide the new offset of the domain's real time clock from UTC. The
problem was that, in the case that qemu is provided with an "-rtc
base=x" where x is an absolute time (rather than "utc" or
"localtime"), the offset sent by qemu's RTC_CHANGE event is *not* the
new offset from UTC, but rather is the sum of all changes to the
domain's RTC since it was started with base=x.

So, despite what was said in commit e31b5cf3, if we assume that
the original value stored in "adjustment" was the offset from UTC at
the time the domain was started, we can always determine the current
offset from UTC by simply adding the most recent (i.e. current) offset
from qemu to that original adjustment.

This patch accomplishes that by storing the initial adjustment in the
domain's status as "adjustment0". Each time a new RTC_CHANGE event is
received from qemu, we simply add adjustment0 to the value sent by
qemu, store that as the new adjustment, and forward that value on to
any event handler.

This patch (*not* e31b5cf3, which should be reverted prior to
applying this patch) fixes:

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

(for the case where basis='utc'. It does not fix basis='localtime')

This reverts commit e31b5cf3.

This commit attempted to work around a bug in the offset value
reported by qemu's RTC_CHANGE event in the case that a variable base
date was given on the qemu commandline. The patch mixed up the math
involved in arriving at the corrected offset to report, and in the
process added an unnecessary private attribute to the clock
element. Since that element is private/internal and not used by anyone
else, it makes sense to simplify things by removing it.

Currently the protocol type with index 0 was NBD which made it hard to
distinguish whether the protocol type was actually assigned. Add a new
protocol type with index 0 to distinguish it explicitly.

The gluster volume name was previously stored as part of the source path
string. This is unfortunate when we want to do operations on the path as
the volume is used separately.

Parse and store the volume name separately for gluster storage volumes
and use the newly stored variable appropriately.

Refactor the function to accept a virStorageSourcePtr instead of just
the path, add a check to run it only on local storage and fix callers
(possibly by using a newly introduced wrapper that wraps a path in the
 virStorageSource struct for legacy code)

Refresh the disk backing chains when reconnecting to a qemu process
after daemon restart. There are a few internal fields that don't get
refreshed from the XML. Until we are able to do that, let's reload all
the metadata by the backing chain crawler.

This is similar to the previous commit in that we need to explicitly
send migrate_cancel when libvirt detects an error other than those
reported by query-migrate. However, the possibility to hit such error is
pretty small.

When QEMU reports failed or cancelled migration, we don't need to send
it migrate_cancel QMP command. But in all other error paths, such as if
we detect broken connection to a destination daemon or something else
happens inside libvirt, we need to explicitly send migrate_cancel
command instead of relying on the migration to be implicitly cancelled
when destination QEMU is killed.

Because we were not doing so, one could end up with a paused domain
after failed migration.

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

If virDomainMemoryStats is called too soon after domain startup,
QEMU returns:
"error":{"class":"GenericError","desc":"guest hasn't updated any stats yet"}
when we try to query balloon stats.

Check for this reply and log it as OPERATION_INVALID instead of
INTERNAL_ERROR. This means the daemon only logs it at the debug level,
without polluting system logs.

Reported by Laszlo Pal:
https://www.redhat.com/archives/libvirt-users/2014-May/msg00023.html

In the f56c773b we've made the substitution but forgot to fix one
comment which is still referring to the old name. This may be
potentially misleading.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

When doing an external checkpoint of a VM with no disk selected we'd
return failure but not set error code. This was a result of ret not
being set to 0 during walking of the disk array.

Rework early failure checking and set the error code to success before
iterating the array of disks so that we return success if no disks are
snapshotted.

Fixes the following symptom (or without --diskspec for diskless VMs)

 $ virsh snapshot-create-as snapshot-test  --memspec /tmp/asdf --diskspec hda,snapshot=no
 error: An error occurred, but the cause is unknown

If neither disks nor memory are selected for snapshot we'd record
metadata in case of external snapshot and do a disk snapshot in case of
external disk snapshot. Forbid this as it doesn't make much sense.

qemu's savevm command does a snapshot of all non readonly disks of a VM.
Libvirt though allowed disabling snapshot for certain disk of a VM.

Convert the switch to a typecasted value so that the compiler tracks
additions for us.

For now, we set the migration URI via command line '--migrate_uri' or
construct the URI by looking up the dest host's hostname which could be
solved by DNS automatically.

But in cases the dest host have two or more NICs to reach, we may need to
send the migration data over a specific NIC which is different from the
automatically resolved one for some reason like performance, security, etc.
Thus we must explicitly specify the migrateuri in command line everytime,
but it is too troublesome if there are many such hosts (and don't forget
virt-manager).

This patch adds a configuration file option on dest host to save the
default value set which can be specified to a migration hostname or
one of this host's addresses used for transferring data, thus user doesn't
have to specify it in command line everytime.
Signed-off-by: NChen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Commit d5c86278 was incomplete; other functions also triggered
compiler warnings about collisions in the use of 'sync'.

* src/qemu/qemu_driver.c (qemuDomainSetTime): Fix another client.
* tools/virsh-domain-monitor.c (cmdDomTime): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

Old gcc complains about shadowing 'sync' variable:

../../src/qemu/qemu_agent.c: In function 'qemuAgentSetTime':
../../src/qemu/qemu_agent.c:1737: warning: declaration of 'sync'
  shadows a global declaration [-Wshadow]
/usr/include/unistd.h:464: warning: shadowed declaration is here
  [-Wshadow]
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

The commit 84c59ffa improved the way we change ejectable media.
If for any reason the first "eject" didn't open the tray we
should return with error.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

This partially reverts commits b279e52f and ea18f8b2.

It turns out our code base is full of:

if ((struct.member = virBlahFromString(str)) < 0)
    goto error;

Meanwhile, the C standard says it is up to the compiler whether
an enum is signed or unsigned when all of its declared values
happen to be positive.  In my testing (Fedora 20, gcc 4.8.2),
the compiler picked signed, and nothing changed.  But others
testing with gcc 4.7 got compiler warnings, because it picked
the enum to be unsigned, but no unsigned value is less than 0.
Even worse:

if ((struct.member = virBlahFromString(str)) <= 0)
    goto error;

is silently compiled without warning, but incorrectly treats -1
from a bad parse as a large positive number with no warning; and
without the compiler's help to find these instances, it is a
nightmare to maintain correctly.  We could force signed enums
with a dummy negative declaration in each enum, or cast the
result of virBlahFromString back to int after assigning to an
enum value, or use a temporary int for collecting results from
virBlahFromString, but those actions are all uglier than what we
were trying to cure by directly using enum types for struct
values in the first place.  It's better off to just live with int
members, and use 'switch ((virFoo) struct.member)' where we want
the compiler to help, than to track down all the conversions from
string to enum and ensure they don't suffer from type problems.

* src/util/virstorageencryption.h: Revert back to int declarations
with comment about enum usage.
* src/util/virstoragefile.h: Likewise.
* src/conf/domain_conf.c: Restore back to casts in switches.
* src/qemu/qemu_driver.c: Likewise.
* src/qemu/qemu_command.c: Add cast rather than revert.
Signed-off-by: NEric Blake <eblake@redhat.com>

For internal structs, we might as well be type-safe and let the
compiler help us with less typing required on our part (getting
rid of casts is always nice).  In trying to use enums directly,
I noticed two problems in virstoragefile.h that can't be fixed
without more invasive refactoring: virStorageSource.format is
used as more of a union of multiple enums in storage volume
code (so it has to remain an int), and virStorageSourcePoolDef
refers to pooltype whose enum is declared in src/conf, but where
src/util can't pull in headers from src/conf.

* src/util/virstoragefile.h (virStorageNetHostDef)
(virStorageSourcePoolDef, virStorageSource): Use enums instead of
int for fields of internal types.
* src/qemu/qemu_command.c (qemuParseCommandLine): Cover all values.
* src/conf/domain_conf.c (virDomainDiskSourceParse)
(virDomainDiskSourceFormat): Simplify clients.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskInternal): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

One caveat though, qemu-ga is expecting time and returning time
in nanoseconds. With all the buffering and propagation delay, the
time is already wrong once it gets to the qemu-ga, but there's
nothing we can do about it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Coverity complains about event being leaked in
qemuDomainCheckRemoveOptionalDisk. The best fix for it is to remove the
disk directly since we already know its index.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this
commit are related to storage (storage_conf) enums.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

When qemu driver is polling for migration to finish (in
qemuMigrationWaitForCompletion), it may happen that another job allowed
during migration is running and if it does not finish within 30 seconds,
migration would be cancelled because of that. However, we can just
ignore the timeout and let the waiting loop try again later.

If an event fired at the end of migration is ever implemented in QEMU,
we can just wait for the event instead of polling for migration status
and libvirt will behave consistently, i.e., migration won't be cancelled
in case another job started during migration takes long time to finish.

For bug https://bugzilla.redhat.com/show_bug.cgi?id=1083238Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

As a side effect, the return value of qemuDomainObjEnterMonitorAsync is
not directly used as the return value of qemuProcess{Start,Stop}CPUs.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

If job queue is full or waiting for a job times out, the function
returns -2 so that it can be handled in a different way by callers.

The change is safe since all existing callers of
qemuDomainObjBeginNestedJob check the return value to be less than zero.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

It's only used within qemu_domain.c.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

If the compression program for external snapshot memory image isn't
found we exitted the function without terminating the domain job. This
caused the domain to be unusable.

The problem was introduced in commit 7df5093f.

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