Replace:
if (virBufferError(&buf)) {
    virBufferFreeAndReset(&buf);
    virReportOOMError();
    ...
}

with:
if (virBufferCheckError(&buf) < 0)
    ...

This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)

This introduces two new attributes "cmd_per_lun" and "max_sectors" same
with the names QEMU uses for virtio-scsi. An example of the XML:

<controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50'
max_sectors='512'/>

The corresponding QEMU command line:

-device virtio-scsi-pci,id=scsi0,cmd_per_lun=50,max_sectors=512,
bus=pci.0,addr=0x3
Signed-off-by: NMike Perez <thingee@gmail.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

The IDE bus doesn't support readonly disks, so inform the user with an
error message instead of let qemu fail with a more obscure "Device
'ide-hd' could not be initialized" error message.

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939Signed-off-by: NGiuseppe Scrivano <gscrivan@redhat.com>

Since commit d86c876a we are using
guestfwd=tcp:IP:PORT,chardev=ID for guestfwd specification, however,
that has not changed in qemu, so guestfwd does not work since.

Apart from that, guestfwd is not working with older qemu that doesn't
have QEMU_CAPS_DEVICE.

Both regressions exist since late 2009 and nobody found that (until
now), so I'm only fixing the first one.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1112066Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

The QEMU VNC client arg code has a long standing typo
of SASL_CONF_DIR when it should be SASL_CONF_PATH for
the env variable name.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

There are no options to parse here other than the name of the device,
and all three possible device names have the same prefix
("virtio-balloon" with "-ccw", "-pci", or "-device" appended), so the
code is fairly simple. It has been implemented such that it will be
easier to add handling for other -device entries that aren't otherwise
recognized - just add another "else if (STRPREFIX(opts, ....)" clause.

qemuParseCommandLineString() previously would always add a <memballoon
model='virtio'/> to every result (the comments erroneously say that it
is adding a <memballoon model='none'/>) This has been changed to add
model='none', and 84 test case xml's updated accordingly (so that
qemuxml2argvtest won't fail).

Now that the memballoon device is properly parsed, we can safely add a
test for properly ignoring -nodefconfig and -nodefaults. Rather than
adding an entire new test case for this (and memballoon), we just
randomly pick the clock-utc test and modify it slightly to fulfill the
purpose.

Just code movement and rename.

The qemu driver always adds these options to the qemu commandlines,
but the commandline parser didn't recognize them, so sending a
libvirt-generated qemu commandline to its own argvtoxml would always
result in a warning message and a qemu namespace added to the
xml. Since the options don't add any functionality to the domain, they
should just be ignored (similar to -S).

Note that we can't yet add a test for this to qemuargv2xmltest,
because we would have to add QEMU_CAPS_NODEFCONFIG and
QEMU_CAPS_DEVICE to the capabilities for any corresponding
xml2argvtest, and QEMU_CAPS_DEVICE would necessitate having support
for parsing a memballoon device in order for qemuargv2xmltest to
pass. So we wait to add a test for -nodefconfig and -nodefaults until
after adding support for parsing -device virtio-balloon-*.

As part of the work on backing chains, I'm finding that it would
be easier to directly manipulate chains of pointers (adding a
snapshot merely adjusts pointers to form the correct list) rather
than copy data from one struct to another.  This patch converts
domain disk source to be a pointer.

In this patch, the pointer is ALWAYS allocated (thanks in part to
the previous patch forwarding all disk def allocation through a
common point), and all other changse are just mechanical fallout of
the new type; there should be no functional change.  It is possible
that we may want to leave the pointer NULL for a cdrom with no
medium in a later patch, but as that requires a closer audit of the
source to ensure we don't fault on a null dereference, I didn't do
it here.

* src/conf/domain_conf.h (_virDomainDiskDef): Change type of src.
* src/conf/domain_conf.c: Adjust all clients.
* src/security/security_selinux.c: Likewise.
* src/qemu/qemu_domain.c: Likewise.
* src/qemu/qemu_command.c: Likewise.
* src/qemu/qemu_conf.c: Likewise.
* src/qemu/qemu_process.c: Likewise.
* src/qemu/qemu_migration.c: Likewise.
* src/qemu/qemu_driver.c: Likewise.
* src/lxc/lxc_driver.c: Likewise.
* src/lxc/lxc_controller.c: Likewise.
* tests/securityselinuxlabeltest.c: Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

A future patch wants to create disk definitions with non-zero
default contents; to avoid crashes, all callers that allocate
a disk definition should go through a common point.

I found allocation points by looking for any code that increments
ndisks, as well as any matches for ALLOC.*disk.  Most places that
modified ndisks were covered by the parse from XML to domain/device
definition by initial domain creation or device hotplug; I also
hand-checked all drivers that generate a device struct on the
fly during getXMLDesc.

* src/conf/domain_conf.h (virDomainDiskDefNew): New prototype.
* src/conf/domain_conf.c (virDomainDiskDefNew): New function.
(virDomainDiskDefParseXML): Use it.
* src/parallels/parallels_driver.c (parallelsAddHddInfo):
Likewise.
* src/qemu/qemu_command.c (qemuParseCommandLine): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/vmx/vmx.c (virVMXParseDisk): Likewise.
* src/xenxs/xen_sxpr.c (xenParseSxprDisks, xenParseSxpr):
Likewise.
* src/xenxs/xen_xm.c (xenParseXM): Likewise.
* src/libvirt_private.syms (domain_conf.h): Export it.
Signed-off-by: NEric Blake <eblake@redhat.com>

In "src/conf/domain_conf.h" there are many enum declarations. The
cleanup in this header filer was started, but it wasn't enough and
there are many other files that has enum variables declared. So, the
commit was starting to be big. This commit finish the cleanup in this
header file and in other files that has enum variables, parameters,
or functions declared.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

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>

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.

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>

Move sharable PCI handling functions to domain_addr.[ch], and
change theirs prefix from 'qemu' to 'vir':

 - virDomainPCIAddressAsString;
 - virDomainPCIAddressBusSetModel;
 - virDomainPCIAddressEnsureAddr;
 - virDomainPCIAddressFlagsCompatible;
 - virDomainPCIAddressGetNextSlot;
 - virDomainPCIAddressReleaseSlot;
 - virDomainPCIAddressReserveAddr;
 - virDomainPCIAddressReserveNextSlot;
 - virDomainPCIAddressReserveSlot;
 - virDomainPCIAddressSetFree;
 - virDomainPCIAddressSetGrow;
 - virDomainPCIAddressSlotInUse;
 - virDomainPCIAddressValidate;

The only change here is function names, the implementation itself
stays untouched.

Extract common allocation code from DomainPCIAddressSetCreate
into virDomainPCIAddressSetAlloc.

Introduce new files (domain_addr.[ch]) to provide
an API for domain device handling that could be
shared across the drivers.

A list of data types were extracted and moved there:

 qemuDomainPCIAddressBus -> virDomainPCIAddressBus
 qemuDomainPCIAddressBusPtr -> virDomainPCIAddressBusPtr
 _qemuDomainPCIAddressSet -> virDomainPCIAddressSet
 qemuDomainPCIAddressSetPtr -> virDomainPCIAddressSetPtr
 qemuDomainPCIConnectFlags -> virDomainPCIConnectFlags

Also, move the related definitions and macros.

QEMU commit 5e2ac51 added a boolean '-msg timestamp=[on|off]'
option, which can enable timestamps on errors:
$ qemu-system-x86_64 -msg timestamp=on zghhdorf
2014-04-09T13:25:46.779484Z qemu-system-x86_64: -msg timestamp=on: could
not open disk image zghhdorf: Could not open 'zghhdorf': No such file or
directory

Enable this timestamp if the QEMU binary supports it.

Add a 'log_timestamp' option to qemu.conf for disabling this behavior.

This uses the new QEMU_CAPS_HOST_PCI_MULTIDOMAIN capability when
present, for -devivce pci-assign, -device vfio-pci, and -pcidevice.

While creating tests for this new functionality, I noticed that the
xmls for two existing tests had erroneously specified an
until-now-ignored domain="0x0002", so I corrected those two tests, and
also added two failure tests to be sure that we alert users who
attempt to use a non-zero domain with a qemu that doesn't support it.

In "src/util/" there are many enumeration (enum) declarations.
Sometimes, it's better using a typedef for variable types,
function types and other usages. Other enumeration will be
changed to typedef's in the future.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

A couple of places in the QEMU XML -> ARGV conversion code
raised an error but then forgot to return an error status
due to missing gotos. While fixing this also tweak style
of a couple of other error reports
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

The check for a network being active during interface attach was being
done individually in several places (by both the lxc driver and the
qemu driver), but those places were too specific, leading to it *not*
being checked when allocating a connection/device from a macvtap or
hostdev network.

This patch puts a single check in networkAllocateActualDevice(), which
is always called before the any network interface is attached to any
type of domain. It also removes all the other now-redundant checks
from the lxc and qemu drivers.

NB: the following patches are prerequisites for this patch, in the
case that it is backported to any branch:

  440beeb7 network: fix virNetworkObjAssignDef and persistence
  8aaa5b68 network: create statedir during driver initialization
  b9e95491 network: change location of network state xml files
  411c5486 network: set macvtap/hostdev networks active if their state
          file exists

This fixes:

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

When a VM fails to launch due to error creating nwfilter
rules, we must avoid overwriting the original error when
tearing down the partially created rules.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Recent discussions around naming of 'pci' vs 'pci.0' for PPC
made me go back and look at the PPC emulator in every historical
version of QEMU since 1.0. The results were worse than I imagined.
This patch adds the logic required to make libvirt work with PPC
correctly with naming variations across all versions & machine
types.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Currently, libvirt is using legacy USB controller as default. There
are problems with VGA which can't work correctly with USB Keyboard and
USB Mouse.

While providing -nodefaults, ppc64 should be specifying the usb
controller explicitly in place of using the legacy
controller(-usb). Qemu spapr initialization code when sees "-usb" adds a
USB Keyboard and USB Mouse by default. And libvirt has added a USB
keyboard and USB mouse.

A recent fix in the in qemu VGA code uncoverd this problem, which
resulted in addition of extra keyboard and mouse to the qemu machine.

This patch is to set pci-ohci as USB default controller.
Signed-off-by: NLi Zhang <zhlcindy@linux.vnet.ibm.com>

The function isn't used in any other file. Convert it to static.

Refactor the function to avoid multiple wrappers splitting identical
fields from the now common metadata struct.

The refactor is done by folding in the wrapper used for disk sources
which allows us to lookup secrets via the secret driver. This may allow
using stored secrets for snapshot disk images too in the future.

Now that the storage source definition is uniform convert the helpers to
retrieve the actual storage type to a single one.

Since it is an abbreviation, PCI should always be fully
capitalized or full lower case, never Pci.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Since it is an abbreviation, USB should always be fully
capitalized or full lower case, never Usb.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Since it is an abbreviation, SCSI should always be fully
capitalized or full lower case, never Scsi.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

I almost wrote a hash value free function that just called
VIR_FREE, then realized I couldn't be the first person to
do that.  Sure enough, it was worth factoring into a common
helper routine.

* src/util/virhash.h (virHashValueFree): New function.
* src/util/virhash.c (virHashValueFree): Implement it.
* src/util/virobject.h (virObjectFreeHashData): New function.
* src/libvirt_private.syms (virhash.h, virobject.h): Export them.
* src/nwfilter/nwfilter_learnipaddr.c (virNWFilterLearnInit): Use
common function.
* src/qemu/qemu_capabilities.c (virQEMUCapsCacheNew): Likewise.
* src/qemu/qemu_command.c (qemuDomainCCWAddressSetCreate):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorGetBlockInfo): Likewise.
* src/qemu/qemu_process.c (qemuProcessWaitForMonitor): Likewise.
* src/util/virclosecallbacks.c (virCloseCallbacksNew): Likewise.
* src/util/virkeyfile.c (virKeyFileParseGroup): Likewise.
* tests/qemumonitorjsontest.c
(testQemuMonitorJSONqemuMonitorJSONGetBlockInfo): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

nmdm is a FreeBSD driver which allows to create a pair of tty
devices one of which is passed to the guest and second is used
by the client.

This patch adds new 'nmdm' character device type. Its definition
looks this way:

<serial type='nmdm'>
  <source master='/dev/nmdm0A' slave='/dev/nmdm0B'/>
</serial>

Master is passed to the hypervisior and slave is used for client
connection.

Also implement domainOpenConsole() for bhyve driver based on that.

Right now, virStorageFileMetadata tracks bool backingStoreIsFile
for whether the backing string specified in metadata can be
resolved as a file (covering both block and regular file
resources) or is treated as a network protocol.  But when
merging this struct with virStorageSource, it will be easier
to just actually track which type of resource it is, as well
as have a reserved value for the case where the resource type
is unknown (or had an error during probing).

* src/util/virstoragefile.h (virStorageType): Add a placeholder
value, swap order to match similar public enum.
* src/util/virstoragefile.c (virStorage): Update string mapping.
* src/conf/domain_conf.c (virDomainDiskSourceParse)
(virDomainDiskDefParseXML, virDomainDiskDefFormat)
(virDomainDiskSourceFormat): Adjust clients.
* src/conf/snapshot_conf.c (virDomainSnapshotDiskDefParseXML):
Likewise.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskExternalOverlayInactive)
(qemuDomainSnapshotPrepareDiskInternal)
(qemuDomainSnapshotCreateSingleDiskActive): Likewise.
* src/qemu/qemu_command.c (qemuGetDriveSourceString): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

This one is a relatively easy move.  We don't ever convert the
enum to or from strings (it is inferred from other elements in
the xml, rather than directly represented).

* src/conf/domain_conf.h (virDomainDiskSecretType): Move...
* src/util/virstoragefile.h (virStorageSecreteType): ...and
rename.
* src/conf/domain_conf.c (virDomainDiskSecretType): Drop unused
enum conversion.
(virDomainDiskAuthClear, virDomainDiskDefParseXML)
(virDomainDiskDefFormat): Adjust clients.
* src/qemu/qemu_command.c (qemuGetSecretString): Likewise.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePoolAuth):
Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>