The logic related to spicedisable_ticketing and spicepasswd was
inverted.  As per man xl.cfg(5), 'spicedisable_ticketing = 1'
means no passwd is required.  On the other hand, a passwd is
required if 'spicedisable_ticketing = 0'.  Fix the logic and
produce and error if 'spicedisable_ticketing = 0' but spicepasswd
is not provided.  Also fix the spice cfg test file.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

Move formating of spice listenAddr to the section of code
where spice ports are formatted.  It is more logical to
format address and ports together.  Account for the change
in spice cfg test file by moving 'spicehost'.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

'graphics->' is a bit easier to read and type, and makes for
shorter lines than 'def->graphics[0]->'.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

Both the hal and udev drivers call virPCI*() functions to the the
SRIOV VF/PF info about PCI devices, and the UDEV backend calls
virPCI*() to get IOMMU group info. Since there is now a single
function call in node_device_linux_sysfs.c to do all of this, replace
all that code in the two backends with calls to
nodeDeviceSysfsGetPCIRelatedDevCaps().

Note that this results in the HAL driver (probably) unnecessarily
calling virPCIDevieAddressGetIOMMUGroupNum(), but in the case that the
host doesn't support IOMMU groups, that function turns into a NOP (it
returns -2, which causes the caller to skip the call to
virPCIDeviceAddressGetIOMMUGroupAddresses()). So in the worst case it
is a few extra cycles spent, and in the best case a mythical platform
that supported IOMMU groups but used HAL rather than UDEV would gain
proper reporting of IOMMU group info.

Because reloading a PF driver with a different number of VFs doesn't
result in any sort of event sent from udev to the libvirt node_device
driver, libvirt's cache of that info can be out of date when a request
arrives for the info about a device. To fix this, we refresh that data
at the time of the dumpxml request, similar to what is already done
for netdev link info and SCSI host capabilities.

Since the same is true for iommu group information (for example, some
other device in the same iommu group could have been detached from the
host), we also create a function to update the iommu group info from
sysfs, and a common function that does both. (a later patch will call
this common function from the udev and hal backends).

This resolves:

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

The udev and hal drivers both already call the same functions as these
new functions added to node_device_linux_sysfs.c, but 1) we need to
call them from node_device_driver.c, and 2) it would be nice to
eliminate the duplicated code from the hal and udev backends.

This file contains only a single function, detect_scsi_host_caps(),
which is declared in node_device_driver.h and called from both the hal
and udev backends. Other things common to the hal and udev drivers
can be placed in that file though. As a prelude to adding further
functions, this patch renames the existing function to something
closer in line with other internal libvirt function names
(nodeDeviceSysfsGetSCSIHostCaps()), and puts the declarations into a
separate .h file.

Makes it nicer as update bits are added for different cap types.

For some reason a union (_virNodeDevCapData) that had only been
declared inside the toplevel struct virNodeDevCapsDef was being used
as an argument to functions all over the place. Since it was only a
union, the "type" attribute wasn't necessarily sent with it. While
this works, it just seems wrong.

This patch creates a toplevel typedef for virNodeDevCapData and
virNodeDevCapDataPtr, making it a struct that has the type attribute
as a member, along with an anonymous union of everything that used to
be in union _virNodeDevCapData. This way we only have to change the
following:

  s/union _virNodeDevCapData */virNodeDevCapDataPtr /

and

  s/caps->type/caps->data.type/

This will make me feel less guilty when adding functions that need a
pointer to one of these.

Use vshCommandOptLongLong() instead of retrieving the value as a
string and converting it to a number manually.

The option didn't have VSH_OT_INT type even thought it's expected
to be numeric, as shown by the fact that vshCommandOptInt() is later
used to retrieve its value.

Replace more than 30 ad-hoc error messages with a single, generic one
that contains the name of the option being processed and some hints
to help the user understand what could have gone wrong.

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

Test the support for enabling/disabling CPACF protected key management
operations for a guest.
Signed-off-by: NTony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: NViktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Introduces two new -machine option parameters to the QEMU command to
enable/disable the CPACF protected key management operations for a guest:

    aes-key-wrap='on|off'
    dea-key-wrap='on|off'

The QEMU code maps the corresponding domain configuration elements to the
QEMU -machine option parameters to create the QEMU command:

    <cipher name='aes' state='on'>   --> aes-key-wrap=on
    <cipher name='aes' state='off'>  --> aes-key-wrap=off
    <cipher name='dea' state='on'>   --> dea-key-wrap=on
    <cipher name='dea' state='off'>  --> dea-key-wrap=off
Signed-off-by: NTony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: NDaniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Signed-off-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Two new domain configuration XML elements are added to enable/disable
the protected key management operations for a guest:

    <domain>
      ...
      <keywrap>
        <cipher name='aes|dea' state='on|off'/>
      </keywrap>
      ...
    </domain>
Signed-off-by: NTony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: NViktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: NDaniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Currently, the libxl driver does not support any security drivers.
When the qemu driver has no security driver configued,
nodeGetSecurityModel succeeds but returns an empty virSecurityModel
object.  Do the same in the libxl driver instead of reporting

this function is not supported by the connection driver:
virNodeGetSecurityModel

We have previously effectively ignored all <controller type='ide'>
elements in a domain definition.

On the i440fx-based machinetypes there is an IDE controller that is
included in the chipset and can't be removed (which is the ide
controller with index='0'>), so it makes sense to ignore that one
controller. However, if an i440fx domain definition has a 2nd
controller, nothing catches this error (unless you also have a disk
attached to it, in which case qemu will complain that you're trying to
use the ide controller named "ide1", which doesn't exist), and if any
other type of domain has even a single controller defined, it will be
incorrectly ignored.

Ignoring a bogus controller definition isn't such a big problem, as
long as an error is logged when any disk is attached to that
non-existent controller. But in the case of q35-based machinetypes,
the hardcoded id ("alias" in libvirt terms) of its builtin SATA
controller is "ide", which happens to be the same id as the builtin
IDE controller on i440fx machinetypes. So libvirt creates a
commandline believing that it is connecting the disk to the builtin
(but actually nonexistent) IDE controller, qemu thinks that libvirt
wanted that disk connected to the builtin SATA controller, and
everybody is happy.

Until you try to connect a 2nd disk to the IDE controller. Then qemu
will complain that you're trying to set unit=1 on a controller that
requires unit=0 (SATA controllers are organized differently than IDE
controllers).

After this patch, if a domain has an IDE controller defined for a
machinetype that has no IDE controllers, libvirt will log an error
about the controller itself as it is building the qemu commandline
(rather than a (possible) error from qemu about disks attached to that
controller). This is done by adding IDE to the list of controller
types that are handled in the loop that creates controller command
strings in qemuBuildCommandline() (previously it would *always* skip
IDE controllers). Then qemuBuildControllerDevStr() is modified to log
an appropriate error in the case of IDE controllers.

In the future, if we add support for extra IDE controllers (piix3-ide
and/or piix4-ide) we can just add it into the IDE case in
qemuBuildControllerDevStr(). For now, nobody seems anxious to add
extra support for an aging and very slow controller, when there are so
many better options available.

Resolves:

https://bugzilla.redhat.com/show_bug.cgi?id=1176071 (Fedora)

Reorganize the loop that builds controller args to remove unnecessary
duplicated code and superfluous else clauses. No functional change.

Back in 2013, commit 877bc089 added in some tests that made sure no
error was generated on a domain definition that had an automatically
added usb controller if that domain didn't have a PCI bus to attach
the usb controller to. This was done because, at that time, libvirt
was automatically adding a usb controller to *any* domain definition
that didn't have one.  Along with permitting the controller, two
s390-specific tests were added to ensure this behavior was maintained
- one with <controller type='usb' model='none'/> and another (called
"s390-piix-controllers") that had both usb and ide controllers, but
nothing attached to them.

Then in February of this year, commit 09ab9dcc eliminated the annoying
auto-adding of a usb device for s390 and s390x machines, stating:

 "Since s390 does not support usb the default creation of a usb
  controller for a domain should not occur."

Although, as verified here, the s390 doesn't support usb, and usb
controllers aren't currently added to s390 domain definitions
automatically, there are likely still some domain definitions in the
wild that have a usb controller (which was added *by libvirt*, not by
the user), so we will keep the tests verifying that behavior for
now. But this patch changes the names of the tests to reflect that
they don't actually contain a valid s390 config; this way future
developers won't propagate the incorrect idea that an s390 virtual
machine can have a USB (or IDE) bus.

In the case of the IDE controller, though, libvirt has never
automatically added an IDE controller unless a user added an IDE disk
(which itself would have caused an error), and we specifically *do*
want to begin generating an error when someone tries to add an IDE
controller to a domain that can't support one. For that reason, while
renaming the sz390-piix-controllers patch, this patch removes the
<controller type='ide'...> from it (otherwise the upcoming patch would
break make check)

This makes sure that that the commandlines generated for devices and
controller devices are all using the alias that has been set in the
controller's object as the id of the controller, rather than
hardcoding a printf (or worse, encoding exceptions to the standard
${controller}${index} into the logic)

Since this "fixes" the controller name used for the sata controller,
the commandline arg for the sata controller in the sata test case had
to be adjusted to be "sata0" instead of "ahci0". All other tests
remain unchanged, verifying that the patch causes no other functional
change.

Because the function that finds a controller alias based on a device
def requires a pointer to the full domainDef in order to get the list
of controllers, the arglist of a few functions had to have this added.

There are a few extra exceptions that weren't being accounted for when
creating the alias for a controller. This resulted in 1) incorrect
status XML, and 2) exceptions/printfs of what *should* have been
directly available in the controller alias when constructing device
commandline arguments:

1) The primary (and only) IDE controller on a 440FX machinetype is
hardcoded to be "ide" in qemu.

2) The primary SATA controller on a 440FX machinetype is also
hardcoded to be "ide" in qemu.

3) On machinetypes that don't support multiple PCI buses, the PCI bus
is hardcoded in qemu to have the name "pci".

4) The first usb master controller is "usb", all others are the normal
"usb%d". (note that usb controllers that are not a "master" will have
the same index, and thus alias, as the master).

We needed to pass in the full domainDef and qemuCaps in order to
properly make the decisions about these exceptions.

Because there are multiple potential reasons for an error, this
function logs any errors before returning NULL (since the caller won't
have the information needed to determine which was the reason for
failure).

When cancelling drive mirror, always try to do that for all disks even
if it fails for some of them. Report the first error we saw.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Instead of redoing the same filtering over and over everytime we need to
walk through all disks which are being migrated.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

And move it to qemu_domain.[ch] because this API is QEMU-only.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

The APIs take the memory value in KiB and we store it in KiB
internally, but we cannot parse the whole ULONG_MAX range
on 64-bit systems, because virDomainParseScaledValue
needs to fit the value in bytes in an unsigned long long.

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

Again, a clean-up for which we don't have proper syntax-check.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

We don't allow it in normal code, why would it need to be in the
generated one.  IT also splits the line in perl code so it's readable.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Since we don't have syntax-check for this, it has to be checked
manually.  Let's hope this is the only place it happened.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This only affected the servers that re-exec themselves, which is only
virtlockd and it didn't do any mess, so this is mostly a clenaup.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

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

Treat pinning an IOThread via API as if someone added an IOThread to
ensure the iothreadid doesn't cause the guest to disappear

Since 'autofill'd iothreadid entries are not written during XML format
processing, it is possible that if an iothreadid in the middle of an
autofilled list would then change it's id on a subsequent restart.

Thus during the iothreadid deletion, if we determine the delete is not
the "last" thread, then clear the autofill bit for all iothreadid's
following the one being deleted (either the first or one in the middle).
This way, iothreadid's will be printed/saved.

When parsing a cpulist, the virBitmapParse is used. On an invalid
bitmap an error is reported, but the error gets cleared
immediately by subsequent public APIs call, e.g. virDomainFree().
Moreover, we don't check whether bitmap fits into maximal CPU ID
on the host. Therefore the following examples failed without any
error:

 # virsh vcpupin test3 1 aaa

 # virsh vcpupin test3 1 1000
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

We have a lot of passing arguments code just to pass connection
object cause it holds jobTimeout. Taking into account that
right now this value is defined at compile time let's just
get rid of it and make arguments list more clear in many
places.

In case we later need some runtime configurable timeout
value we can provide this value through arguments
function already operate such as a parallels domain
object etc as this timeouts are operation( and thus
object) specific in practice.
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@parallels.com>

As of eeb008db the variable is not used anymore. Drop it.
Signed-off-by: NWang Yufei <james.wangyufei@huawei.com>
Signed-off-by: NZhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This removes some noise when you're working on the repository
and also have a bunch of source files open in Vim in another
terminal.

In order not to bring in any link dependencies, bridge driver doesn't
use the usual stubs as other conditionally-built code does.  However,
having the function as a macro imposes a problem with possibly unused
variables if just defined as "0".  This was worked around by using
(dom=dom, iface=iface, 0) which should act like a 0 if used in a
condition.  However, gcc still bugs about that, so I came up with
another way how to fix that.

Using static inline functions in the header won't collide with anything,
it fixes the bug and does one thing that the macro didn't do.  It checks
whenther passed variables are pointers of compatible type.  It has only
one downside, and that is that we need to either a) define it with
ATTRIBUTE_UNUSED, which needs an exception in cfg.mk or b) do something
like ignore_value(variable); in the function body.  I went with the
first variant.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Otherwise we might allow coldplugging a device
that uses an address that is already occupied, creating
an unstartable domain.

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