virDomainDefCollectBootOrder() is called for every item on the list
for each type of device. One of the checks it makes is to gather the
order attributes from the <boot> element of all devices, and assure
that no two devices have been given the same order.

Since (internally to libvirt, *not* in the domain XML) an <interface
type='hostdev'> is on both the list of hostdev devices and the list of
network devices, it will be counted twice, and the code that checks
for multiple devices with the same boot order will give a false
positive.

To remedy this, we make sure to return early for hostdev devices that
have a parent.type != NONE.

This was introduced in commit 5b75a4, which was first in libvirt-4.4.0.

Resolves: https://bugzilla.redhat.com/1601318Signed-off-by: NLaine Stump <laine@laine.org>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Broken by v3.10.0-68-gaed3d038.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Attempting to use a chardev definition like

  <serial type='unix'>
    <target type='isa-serial'/>
  </serial>

correctly results in an error being reported, since the source
path - a required piece of information - is missing; however,
the very similar

  <serial type='unix'>
    <target type='pci-serial'/>
  </serial>

was happily accepted by libvirt, only to result in libvirtd
crashing as soon as the guest was started.

The issue was caused by checking the chardev's targetType
against whitelisted values from virDomainChrChannelTargetType
without first checking the chardev's deviceType to make sure
it is actually a channel, for which the check makes sense,
rather than a different type of chardev.

The only reason this wasn't spotted earlier is that the
whitelisted values just so happen to correspond to USB and
PCI serial devices and Xen and UML consoles respectively,
all of which are fairly uncommon.

https://bugzilla.redhat.com/show_bug.cgi?id=1609720Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Since its introduction in commit 2e37bf42 the naming of the arguments
between the prototype and the definition does not match.
Signed-off-by: NJán Tomko <jtomko@redhat.com>

To add CMT/MBM feature and let code be consistent in later patches,
renaming variable name from 'controlBuf' to 'childrenBuf', locates
in functions 'virCapabilitiesFormatCaches' and
'virCapabilitiesFormatMemoryBandwidth'.
Signed-off-by: NWang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

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

Instantiation of an nwfilter binding is only allowed when
the net->filter is defined for the network; however, the
teardown of the binding does not make this check. This
leaves open the possibility that the teardown could be
called during guest shutdown/teardown in session mode
resulting in the following error being logged:

    error : nwfilterConnectOpen:383 : internal error: unexpected
    nwfilter URI path '/session', try nwfilter:///system

So before going through the teardown processing, let's
be sure the network had a filter and then attempt to
get a connection. For session mode it's not even possible
create an nwfilter binding.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The struct is called virPCIDeviceAddress and the
functions operating on it should be named accordingly.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

It's a better fit than conf/domain_conf.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

The corresponding structs are declared there.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

These are simple predicates, which makes bool a more
appropriate return type than int.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The function is called on a virDomainDeviceInfo, so it
should be declared along with it.

Moving this function requires moving and making public
virDomainDeviceCCWAddressIsValid() as well, but that's
perfectly fine since the same reasoning above also
applies to it, due to virDomainDeviceCCWAddress being
(correctly) declared in device_conf.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

It's used in virDomainDeviceInfo, which makes
domain_conf the wrong place to declare it.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Just like a few commits earlier, checking for pool source
duplicates and unlocking pools list afterwards is a buggy
pattern. The check must go into virStoragePoolObjAssignDef.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The @conn argument is needed only to do some source matching in
case of iSCSI source. Anyway, it's used just for node device
driver and as such can be replaced with virGetConnectNodeDev().

At the same time, the @conn struct member is dropped from
_virStoragePoolObjFindDuplicateData.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This function is going to be made static in used in
virStoragePoolObjAssignDef(). Therefore move it and all the
static functions it calls a few lines up.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Even though we do some checking it is not as thorough as it
should be. We already have virStoragePoolObjIsDuplicate but the
way we use it is a typical TOCTOU. Imagine two threads trying to
define two pools with the same name but different UUIDs. With the
current code neither of them finds a duplicate and thus proceed
to virStoragePoolObjAssignDef where only names are compared.
Therefore both threads succeed which is obviously wrong.

We should check for duplicates where we care for them.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

This function is going to be made static in used in
virStoragePoolObjAssignDef(). Therefore move it a few lines up.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

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

Changing MTU on a running guest is not possible and trying to do
so made us face many problems. That's why we forbid it in
5f44d7e3. However, there is still one possible path where
users can sneak in change: migration XML.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

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

When introducing this check back in 4ad54a41 my mindset was
that if an element is missing in update XML then user is
requesting for removal of the corresponding setting. For
instance, if <bandwidth/> is not present in update XML any QoS
previously set on <interface/> is cleared out. Well this
assumption is correct but only to some extent.

Turns out, we have some users who when updating path to ISO
image construct very minimalistic disk XML and pass it to device
update API. Such XML is lacking a lot of information, and alias
is one of them. This triggers error in
virDomainDefCompatibleDevice() because we think that user is
requesting to remove the alias. Well, they are not.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

If @vm has flagged as "to be removed" virDomainObjListFindByNameLocked
returns NULL (although the definition actually exists). Therefore, the
possibility exits that "virHashAddEntry" will raise the error
"Duplicate key" => virDomainObjListAddObjLocked fails =>
virDomainObjEndAPI(&vm) is called and this leads to a freeing of @def
since @def is already assigned to vm->def. But actually this leads to
a double free since the common usage pattern is that the caller of
virDomainObjListAdd(Locked) is responsible for freeing @def in case of
an error.

Let's fix this by setting vm->def to NULL in case of an error.

Backtrace:

   ➤  bt
   #0  virFree (ptrptr=0x7575757575757575)
   #1  0x000003ffb5b25b3e in virDomainResourceDefFree
   #2  0x000003ffb5b37c34 in virDomainDefFree
   #3  0x000003ff9123f734 in qemuDomainDefineXMLFlags
   #4  0x000003ff9123f7f4 in qemuDomainDefineXML
   #5  0x000003ffb5cd2c84 in virDomainDefineXML
   #6  0x000000011745aa82 in remoteDispatchDomainDefineXML
   ...
Reviewed-by: NBjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: NMarc Hartmayer <mhartmay@linux.ibm.com>

None of the existing models is suitable for use with
RISC-V virt guests, and we don't want information about
the serial console to be missing from the XML.

The name is based on comments in qemu/hw/riscv/virt.c:

  RISC-V machine with 16550a UART and VirtIO MMIO

and in qemu/hw/char/serial.c:

  QEMU 16550A UART emulation

along with the output of dmesg in the guest:

  Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
  10000000.uart: ttyS0 at MMIO 0x10000000 (irq = 13,
    base_baud= 230400) is a 16550A
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Commit deb057fd added a switch without a default case.
Add it and call virReportEnumRangeError for _LAST too.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Turn

  virPCIDeviceAddressIsEmpty()
  virDeviceInfoPCIAddressIsWanted()
  virDeviceInfoPCIAddressIsPresent()

from inline functions to regular functions.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The affected functions are

  virDeviceInfoPCIAddressWanted()
  virDeviceInfoPCIAddressPresent()

which get renamed to

  virDeviceInfoPCIAddressIsWanted()
  virDeviceInfoPCIAddressIsPresent()

to comply with the naming convention used for other
predicates.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Similarly to backing store indexes which will become stable eventually
we need also to be able to format and store in the status XML for later
use the index for the top level of the backing chain.

Add XML formatter, parser, schema and docs.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Allow storing of private data in the status XML for disks.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

If a user configures the backing chain in the XML we should not ignore
it. We already do parse it but don't format it out. As a
safety-precaution don't attempt to format detected chain into the
inactive XML.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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

Due to historical reasons we were not parsing device info on
guestfwd channel. Sure, it doesn't make much sense to parse
<address/> but it surely makes sense to parse its alias (which
might be an user alias).

This reverts commit 47a3dd46
which fixed https://bugzilla.redhat.com/show_bug.cgi?id=1172526.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Keep with the recent effort of replacing as many explicit *Free
functions with their automatic equivalents.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

Qemu-3.0 supports Hyper-V-style PV TLB flush, Windows guests can benefit
from this feature as KVM knows which vCPUs are not currently scheduled (and
thus don't require any immediate action).
Signed-off-by: NVitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Qemu-3.0 supports so-called 'Reenlightenment' notifications and this (in
conjunction with 'hv-frequencies') can be used make Hyper-V on KVM pass
stable TSC page clocksource to L2 guests.
Signed-off-by: NVitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Qemu-2.12 gained 'hv-frequencies' cpu flag to enable Hyper-V frequency
MSRs. These MSRs are required (but not sufficient) to make Hyper-V on
KVM pass stable TSC page clocksource to L2 guests.
Signed-off-by: NVitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

name match functions to be the vir prefix and interface name followed by ObjMatch

    ex. for virNetworkObjListExport, the match function is named
        virNetworkObjMatch
Signed-off-by: NAnya Harter <aharter@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

name functions to be the name of the export function followed by Callback

    ex. for virInterfaceObjListExport, the callback function is named
        virInterfaceObjListExportCallback
Signed-off-by: NAnya Harter <aharter@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

name structs to be the name of the Export function followed by Data
also tweak definitions to follow standard struct definition pattern

    ex. for virInterfaceObjListExport, the struct is defined as follows:

        typedef struct _virInterfaceObjListExportData virInterfaceObjListExportData;
        typedef virInterfaceObjListExportData *virInterfaceObjListExportDataPtr;
        struct _virInterfaceObjListExportData {...};
Signed-off-by: NAnya Harter <aharter@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Add new XML section to report host's memory bandwidth allocation
capability. The format as below example:

 <host>
 .....
   <memory_bandwidth>
     <node id='0' cpus='0-19'>
       <control granularity='10' min ='10' maxAllocs='8'/>
     </node>
   </memory_bandwidth>
</host>

granularity   ---- granularity of memory bandwidth, unit percentage.
min           ---- minimum memory bandwidth allowed, unit percentage.
maxAllocs     ---- maximum memory bandwidth allocation group supported.
Signed-off-by: NBing Niu <bing.niu@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Add return value check to virResctrlAllocForeachCache in
virDomainCachetuneDefFormat. The virResctrlAllocForeachCache does have
return value, so need check return value to make sure function executed
without error.
Signed-off-by: NBing Niu <bing.niu@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Introduce a new section memorytune to support memory bandwidth allocation.
This is consistent with existing cachetune. As the example:
below:
  <cputune>
    ......
    <memorytune vcpus='0'>
      <node id='0' bandwidth='30'/>
    </memorytune>
  </cputune>

vpus      --- vpus subjected to this memory bandwidth.
id        --- on which node memory bandwidth to be set.
bandwidth --- the memory bandwidth percent to set.
Signed-off-by: NBing Niu <bing.niu@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Factor out vcpus virDomainResctrlDef update from
virDomainCachetuneDefParse and introduce virDomainResctrlAppend.
virDomainResctrlAppend will format vcpus string and append a new
virDomainResctrlDef to virDomainDefPtr. So that this logic can
be reusable.
Signed-off-by: NBing Niu <bing.niu@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Factor out vcpus overlapping detecting part from
virDomainCachetuneDefParse and introduce virDomainResctrlVcpuMatch.
Instead of allocating virResctrlAllocPtr by default, allocating
virResctrlAllocPtr after confirm vcpus not overlap with existing ones.
And virDomainResctrlVcpuMatch can be reused by other resource control
technologies. virDomainResctrlVcpuMatch can clarify old vcpus overlap
error whether an overlap or a redefinition.
Signed-off-by: NBing Niu <bing.niu@intel.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>