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.

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

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>

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().

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

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>

We should distinguish between success and timeout, to let the user
handle those two events differently.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

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

Commit id 'e0e29055' added a check to determine if the same bus
had the same target value.  It seems that's not quite good enough
as the check should check the target name value regardless of bus type.

Also added a DO_TEST_DIFFERENT to exhibit the issue

As the backend of shmem server is a unix type chr device, save it in
virDomainChrSourceDef, so we can reuse the existing code for chr device.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

Using a custom device tree image may cause unexpected behavior in
architectures that use this approach to detect platform devices. Since
usually the device tree is generated by qemu and thus it's not normally
used let's taint VMs using it to make it obvious as a possible source of
problems.

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

The formatdomain.html description for <disk> device 'lun' indicates that
it must be either a type 'block' or type 'network' with protocol 'iscsi';
however, we did not make that check until domain startup.

This caused issues for virt-manager which had an unexpected failure at
run time rather config time.

This patch adds a check in post part disk device checking for the specific
and supported lun types as well as adjusting the test failure to be for
parse config rather than run time.

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

According to our XML definition, zero is as valid as any other value.
Mainly because it should be kernel-agnostic.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

This patch provides support for the new watchdog model "diag288".
Signed-off-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NDaniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NTony Krowiak <akrowiak@linux.vnet.ibm.com>

This patch provides support for a new watchdog action "inject-nmi" which
allows to define an inject of a non-maskable interrupt into a guest.
Signed-off-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NDaniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Reviewed-by: NStefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: NTony Krowiak <akrowiak@linux.vnet.ibm.com>

Commit id '1feaccf0' attempted to handle an empty secrettype value; however,
it made a mistake by processing the secretType as if it was the original
secrettype string.  The 'secretType' is actually whether 'usage' or 'uuid'
was used.

Thus adjust part of the change to make the same check for def->src->type !=
VIR_STORAGE_TYPE_VOLUME before setting auth_secret_usage from the
secrettype field.

Luckily the aforementioned commits misdeed would be overwritten by the
call to virStorageTranslateDiskSourcePool

Just refactor existing code to use a child buf instead of
check all element before format <blkiotune> and <cputune>.
This will avoid the more and more bigger element check during
we introduce new elements in <blkiotune> and <cputune> in the
future.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

We require the scheduler name attribute in the schemas but the code
would actually be fine when it was omitted. Make it mandatory.

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

The SCSI Architecture Model defines a logical unit address
as 64-bits in length, so change the field accordingly so
that the entire value could be stored.
Signed-off-by: NEric Farman <farman@linux.vnet.ibm.com>

The address elements are all unsigned integers, so we should
use the appropriate print directive when printing it.
Signed-off-by: NEric Farman <farman@linux.vnet.ibm.com>

The SCSI address element attributes bus, target, and unit are expected
to be positive values, so make sure no one provides a negative value since
the value is stored as an unsigned.
Signed-off-by: NEric Farman <farman@linux.vnet.ibm.com>

So that they can format private data (e.g., disk private data) stored
elsewhere in the domain object.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Complex jobs, such as migration, need to monitor several events at once,
which is impossible when each of the event uses its own condition
variable. This patch adds a single condition variable to each domain
object. This variable can be used instead of the other event specific
conditions.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>