Since iothreadid = 0 is invalid, we need to check for it when attempting
to add a disk; otherwise, someone would think/believe their attempt to
add an IOThread to the disk would succeed. Luckily other code ignored
things when ->iothread == 0...

We just need to update the entry in the second hash table. Since commit 8728a565
we have two hash tables for the domain list so that we can do O(1) lookup
regardless of looking up by UUID or name. Since with renaming a domain UUID does
not change, we only need to update the second hash table, where domains are
referenced by their name.

We will call both functions from the qemuDomainRename().
Signed-off-by: NTomas Meszaros <exo@tty.sk>

Since it's not used, let's remove it to avoid any future usage.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

This needs a reorder of XML option definitions.  It might come in handy
one day.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1210587  (completed)

When generating the default drive address for a SCSI <disk> device,
check the generated address to ensure it doesn't conflict with a SCSI
<hostdev> address. The <disk> address generation algorithm uses the
<target> "dev" name in order to determine which controller and unit
in order to place the device. Since a SCSI <hostdev> device doesn't
require a target device name, its placement on the guest SCSI address
"could" conflict.  For instance, if a SCSI <hostdev> exists at
controller=0 unit=0 and an attempt to hotplug 'sda' into the guest
made, there would be a conflict if the <hostdev> is already using
/dev/sda.

Create local controller/bus variables to be used by a future patch

https://bugzilla.redhat.com/show_bug.cgi?id=1210587  (partial)

If a SCSI subsystem <hostdev> element address is provided, we need to
make sure the address provided doesn't conflict with an existing or
libvirt generated address for a SCSI <disk> element. We can handle
this condition in device post processing since we're not generating an
address based on some target name - rather it's either generated based
on space or provided from the user. If the user provides one that conflicts,
then we need to disallow the change.

This will fix the issue where the domain XML provided an <address> for
the <hostdev>, but not the <disk> element where the address provided
ends up being the same address used for the <disk>. A <disk> address
is generated using it's assigned <target> 'dev' name prior to the
check/validation of the <hostdev> address value.

If you pass <disk><serial> XML to UpdateDevice, and the original device
didn't have a <serial> block, libvirtd crashes trying to read the original
NULL serial string.

Use _NULLABLE string comparisons to avoid the crash. A couple other
properties needed the change too.

Commit a6f9af82 added checking for address colisions between
starting and ending addresses of forwarding addresses, but forgot that
there might be no addresses set at all.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This patch modifies virSocketAddrGetRange() to function properly when
the containing network/prefix of the address range isn't known, for
example in the case of the NAT range of a virtual network (since it is
a range of addresses on the *host*, not within the network itself). We
then take advantage of this new functionality to validate the NAT
range of a virtual network.

Extra test cases are also added to verify that virSocketAddrGetRange()
works properly in both positive and negative cases when the network
pointer is NULL.

This is the *real* fix for:

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

Commits 1e334a and 48e8b9 had earlier been pushed as fixes for that
bug, but I had neglected to read the report carefully, so instead of
fixing validation for the NAT range, I had fixed validation for the
DHCP range. sigh.

This will be used with a virtio-scsi controller later on.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

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

The following XML really does not make any sense:

<inbound average="-1" burst="-2" peak="-3" floor="-4"/>

There can't be a negative packet rate. Well, so far we haven't
assigned any meaning to it. So reject it unless users harm themselves,
because otherwise we turn the negative numbers into really big values.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

By specifying parentIndex in a call to virNetworkUpdate(), it was
possible to direct libvirt to add a dhcp range or static host of a
non-matching address family to the <dhcp> element of an <ip>. For
example, given:

 <ip address='192.168.122.1' netmask='255.255.255.0'/>
 <ip family='ipv6' address='2001:db6:ca3:45::1' prefix='64'/>

you could provide a static host entry with an IPv4 address, and
specify that it be added to the 2nd <ip> element (index 1):

  virsh net-update default add ip-dhcp-host --parent-index 1 \
  '<host mac="52:54:00:00:00:01" ip="192.168.122.45"/>'

This would be happily added with no error (and no concern of any
possible future consequences).

This patch checks that any dhcp range or host element being added to a
network ip's <dhcp> subelement has addresses of the same family as the
ip element they are being added to.

This resolves:

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

This controller can be connected only to a port on a
pcie-switch-upstream-port. It provides a single hotpluggable port that
will accept any PCI or PCIe device, as well as any device requiring a
pcie-*-port (the only current example of such a device is the
pcie-switch-upstream-port).

This controller can be connected only to a pcie-root-port or a
pcie-switch-downstream-port (which will be added in a later patch),
which is the reason for the new connect type
VIR_PCI_CONNECT_TYPE_PCIE_PORT. A pcie-switch-upstream-port provides
32 ports (slot=0 to slot=31) on the downstream side, which can only
have pci controllers of model "pcie-switch-downstream-port" plugged
into them, which is the reason for the other new connect type
VIR_PCI_CONNECT_TYPE_PCIE_SWITCH.

This controller can be connected (at domain startup time only - not
hotpluggable) only to a port on the pcie root complex ("pcie-root" in
libvirt config), hence the new connect type
VIR_PCI_CONNECT_TYPE_PCIE_ROOT. It provides a hotpluggable port that
will accept any PCI or PCIe device.

New attributes must be added to the controller <target> subelement for
this - chassis and port are guest-visible option values that will be
set by libvirt with values derived from the controller's index and pci
address information.

There are some configuration options to some types of pci controllers
that are currently automatically derived from other parts of the
controller's configuration. For example, in qemu a pci-bridge
controller has an option that is called "chassis_nr"; up until now
libvirt has always set chassis_nr to the index of the pci-bridge. So
this:

  <controller type='pci' model='pci-bridge' index='2'/>

will always result in:

  -device pci-bridge,chassis_nr=2,...

on the qemu commandline. In the future we may decide there is a better
way to derive that option, but even in that case we will need for
existing domains to retain the same chassis_nr they were using in the
past - that is something that is visible to the guest so it is part of
the guest ABI and changing it would lead to problems for migrating
guests (or just guests with very picky OSes).

The <target> subelement has been added as a place to put the new
"chassisNr" attribute that will be filled in by libvirt when it
auto-generates the chassisNr; it will be saved in the config, then
reused any time the domain is started:

  <controller type='pci' model='pci-bridge' index='2'>
    <model type='pci-bridge'/>
    <target chassisNr='2'/>
  </controller>

The one oddity of all this is that if the controller configuration
is changed (for example to change the index or the pci address
where the controller is plugged in), the items in <target> will
*not* be re-generated, which might lead to conflict. I can't
really see any way around this, but fortunately if there is a
material conflict qemu will let us know and we will pass that on
to the user.

This new subelement is used in PCI controllers: the toplevel
*attribute* "model" of a controller denotes what kind of PCI
controller is being described, e.g. a "dmi-to-pci-bridge",
"pci-bridge", or "pci-root". But in the future there will be different
implementations of some of those types of PCI controllers, which
behave similarly from libvirt's point of view (and so should have the
same model), but use a different device in qemu (and present
themselves as a different piece of hardware in the guest). In an ideal
world we (i.e. "I") would have thought of that back when the pci
controllers were added, and used some sort of type/class/model
notation (where class was used in the way we are now using model, and
model was used for the actual manufacturer's model number of a
particular family of PCI controller), but that opportunity is long
past, so as an alternative, this patch allows selecting a particular
implementation of a pci controller with the "name" attribute of the
<model> subelement, e.g.:

  <controller type='pci' model='dmi-to-pci-bridge' index='1'>
    <model name='i82801b11-bridge'/>
  </controller>

In this case, "dmi-to-pci-bridge" is the kind of controller (one that
has a single PCIe port upstream, and 32 standard PCI ports downstream,
which are not hotpluggable), and the qemu device to be used to
implement this kind of controller is named "i82801b11-bridge".

Implementing the above now will allow us in the future to add a new
kind of dmi-to-pci-bridge that doesn't use qemu's i82801b11-bridge
device, but instead uses something else (which doesn't yet exist, but
qemu people have been discussing it), all without breaking existing
configs.

(note that for the existing "pci-bridge" type of PCI controller, both
the model attribute and <model> name are 'pci-bridge'. This is just a
coincidence, since it turns out that in this case the device name in
qemu really is a generic 'pci-bridge' rather than being the name of
some real-world chip)

If a pci address had a function number out of range, the error message
would be:

  Insufficient specification for PCI address

which is logged by virDevicePCIAddressParseXML() after
virDevicePCIAddressIsValid returns a failure.

This patch enhances virDevicePCIAddressIsValid() to optionally report
the error itself (since it is the place that decides which part of the
address is "invalid"), and uses that feature when calling from
virDevicePCIAddressParseXML(), so that the error will be more useful,
e.g.:

  Invalid PCI address function=0x8, must be <= 7

Previously, virDevicePCIAddressIsValid didn't check for the
theoretical limits of domain or bus, only for slot or function. While
adding log messages, we also correct that ommission. (The RNG for PCI
addresses already enforces this limit, which by the way means that we
can't add any negative tests for this - as far as I know our
domainschematest has no provisions for passing XML that is supposed to
fail).

Note that virDevicePCIAddressIsValid() can only check against the
absolute maximum attribute values for *any* possible PCI controller,
not for the actual maximums of the specific controller that this
device is attaching to; fortunately there is later more specific
validation for guest-side PCI addresses when building the set of
assigned PCI addresses. For host-side PCI addresses (e.g. for
<hostdev> and for network device pools), we rely on the error that
will be logged when it is found that the device doesn't actually
exist.

This resolves:

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

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

Some users think this is a good idea:

      <vcpu placement='static'>4</vcpu>
      <cpu mode='host-model'>
        <model fallback='allow'/>
        <numa>
          <cell id='0' cpus='0-1' memory='1048576' unit='KiB'/>
          <cell id='1' cpus='9-10' memory='2097152' unit='KiB'/>
        </numa>
      </cpu>

It's not. Lets therefore introduce a check and discourage them in
doing so.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This function should return the greatest CPU number set in
/domain/cpu/numa/cell/@cpus. The idea is that we should compare
the returned value against /domain/vcpu value. Yes, there exist
users who think the following is a good idea:

  <vcpu placement='static'>4</vcpu>
  <cpu mode='host-model'>
    <model fallback='allow'/>
    <numa>
      <cell id='0' cpus='0-1' memory='1048576' unit='KiB'/>
      <cell id='1' cpus='9-10' memory='2097152' unit='KiB'/>
    </numa>
  </cpu>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The recent changes to perform SCSI device address checks during the
post parse callbacks ran afoul of the Coverity checker since the changes
assumed that the 'xmlopt' parameter to virDomainDeviceDefPostParse
would be non NULL (commit id 'ca2cf74e'); however, what was missed
is there was an "if (xmlopt &&" check being made, so Coverity believed
that it could be possible for a NULL 'xmlopt'.

Checking the various calling paths seemingly disproves that. If called
from virDomainDeviceDefParse, there were two other possible calls that
would end up dereffing, so that path could not be NULL. If called via
virDomainDefPostParseDeviceIterator via virDomainDefPostParse there
are two callers (virDomainDefParseXML and qemuParseCommandLine)
which deref xmlopt either directly or through another call.

So I'm removing the check for non-NULL xmlopt.

Rather than provide a somewhat generic error message when the API
returns false, allow the caller to supply a "report = true" option
in order to cause virReportError's to describe which of the 3 paths
that can cause failure.

Some callers don't care about what caused the failure, they just want
to have a true/false - for those, calling with report = false should
be sufficient.

Rather than calling virDomainDiskDefAssignAddress during the parsing of
the XML, moving the setting of disk addresses into the domain/device post
processing.

Commit id '37588b25' which introduced VIR_DOMAIN_DEF_PARSE_DISK_SOURCE
in order to avoid generating the address which wasn't required will not
be affected by this as all it cared about was processing the source XML.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Remove unused xmlopt param
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Rather than calling virDomainHostdevAssignAddress during the parsing
of the XML, move the setting of a default hostdev address to domain/
device post processing.

Since the parse code no longer generates an address, we can remove
the virDomainDefMaybeAddHostdevSCSIcontroller since the call to
virDomainHostdevAssignAddress will attempt to add the controllers
that were not already defined in the XML.

This patch will also enforce that the address type is type 'drive'
when a SCSI subsystem <hostdev> element is provided with an <address>.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

If virDomainControllerSCSINextUnit failed to find a slot on the current
VIR_DOMAIN_CONTROLLER_TYPE_SCSI controller(s), try to add a new controller;
otherwise, there may be multiple unit=0 entries for the same "next"
controller.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

While searching the hostdevs the drive type can be either *_TYPE_DRIVE
or *_TYPE_NONE.  If the type is _TYPE_NONE on the first scsi_host, then
there is an erroneous "match" that the address already exists.

Although this works by chance currently because hostdev's are added one
at a time and 'nhostdevs' would be zero, thus returning false for the
first hostdev added, a future patch will move the hostdev address
assignment into post processing resulting in the bad match.

This code is only called by path's expecting either drive or none.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Add the xmlopt parameter that was saved during virDomainDefPostParse
to the parameters. A future patch will use it.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Move the functions above the post processing for upcoming patch
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Modify virDomainDriveAddressIsUsedBy{Disk|Hostdev} and
virDomainSCSIDriveAddressIsUsed to take 'bus' and 'target'
parameters.  Will be used by future patches for more complete
address conflict checks
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Since the only way virDomainHostdevAssignAddress can be called is from
within virDomainHostdevDefParseXML when hostdev->source.subsys.type is
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI, thus there's no need for redundancy.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

There are some non-0 default values in virDomainControllerDef (and
will soon be more) that are easier to not forget if the remembering is
done by a single initializer function (rather than inline code after
allocating the obejct with generic VIR_ALLOC().

The function that auto-assigns PCI addresses was written with the
hardcoded assumptions that any PCI bus would have slots available
starting at 1 and ending at 31. This isn't true for many types of
controllers (some have a single slot/port at 0, some have slots/ports
from 0 to 31). This patch updates that function to remove the
hardcoded assumptions. It will properly find/assign addresses for
devices that can only connect to pcie-(root|downstream)-port (which
have minSlot/maxSlot of 0/0) or a pcie-switch-upstream-port (0/31).

It still will not auto-create a new bus of the proper kind for these
connections when one doesn't exist, that task is for another day.

This makes the range and static host array management in
virNetworkDHCPDefParseXML() more similar to what is done in
virNetworkDefUpdateIPDHCPRange() and virNetworkDefUpdateIPDHCPHost() -
they use VIR_APPEND_ELEMENT rather than a combination of
VIR_REALLOC_N() and separate incrementing of the array size.

The one functional change here is that a memory leak of the contents
of the last (unsuccessful) virNetworkDHCPHostDef was previously leaked
in certain failure conditions, but it is now properly cleaned up.

Adding functionality to libvirt that will allow
it query the interface for the availability of RDMA and
tx-udp_tnl-segmentation Offloading NIC capabilities

Here is an example of the feature XML definition:

<device>
<name>net_eth4_90_e2_ba_5e_a5_45</name>
  <path>/sys/devices/pci0000:00/0000:00:03.0/0000:08:00.1/net/eth4</path>
  <parent>pci_0000_08_00_1</parent>
  <capability type='net'>
    <interface>eth4</interface>
    <address>90:e2:ba:5e:a5:45</address>
    <link speed='10000' state='up'/>
    <feature name='rx'/>
    <feature name='tx'/>
    <feature name='sg'/>
    <feature name='tso'/>
    <feature name='gso'/>
    <feature name='gro'/>
    <feature name='rxvlan'/>
    <feature name='txvlan'/>
    <feature name='rxhash'/>
    <feature name='rdma'/>
    <feature name='txudptnl'/>
    <capability type='80203'/>
  </capability>
</device>

If one calls update-device with information that is not updatable,
libvirt reports success even though no data were updated.  The example
used in the bug linked below uses updating device with <boot order='2'/>
which, in my opinion, is a valid thing to request from user's
perspective.  Mainly since we properly error out if user wants to update
such data on a network device for example.

And since there are many things that might happen (update-device on disk
basically knows just how to change removable media), check for what's
changing and moreover, since the function might be usable in other
drivers (updating only disk path is a valid possibility) let's abstract
it for any two disks.

We can't possibly check for everything since for many fields our code
does not properly differentiate between default and unspecified values.
Even though this could be changed, I don't feel like it's worth the
complexity so it's not the aim of this patch.

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

Add virNetworkDefGetRouteByIndex() similarly to
virNetworkDefGetIpByIndex(), but for routes.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

In commit 714b38cb I tried to avoid
having two disks with the same WWN in a VM. I forgot to check the
hotplug paths though which make it possible bypass that check. Reinforce
the fix by checking the wwn when attaching the disk.

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

There are multiple consumers for the domain condition and we should
always wake them all.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>