The helpers allow to simplify restoring original errors in most cases.

There were a few places in our code where the following pattern in 'if'
condition occurred:

if ((foo = bar() < 0))
    do something;

This patch adjusts the conditions to the expected format:

if ((foo = bar()) < 0)
    do something;

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1488192Signed-off-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

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

When updating a virtio enabled vNIC and trying to change either
of rx_queue_size or tx_queue_size success is reported although no
operation is actually performed. Moreover, there's no way how to
change these on the fly. This is due to way we check for changes:
explicitly for each struct member. Therefore it's easy to miss
one.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

A new function virNetDevOpenvswitchUpdateVlan has been created to instruct
OVS of the changes. qemuDomainChangeNet has been modified to handle the
update of the VLAN configuration for a running guest and rely on
virNetDevOpenvswitchUpdateVlan to do the actual update if needed.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

All the pieces are now in place, so we can finally start
using isolation groups to achieve our initial goal, which is
separating hostdevs from emulated PCI devices while keeping
hostdevs that belong to the same host IOMMU group together.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1280542Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

We will soon need to be able to return a NULL pointer
without the caller considering that an error: to make
it possible, change the return type to int and use
an out parameter for the string instead.

Add some documentation for the function as well.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

We use hostdev->info frequently enough that having
a shorter name for it makes the code more readable.
We will also be adding even more uses later on.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

Combine and "clean up" a bit two places that are removing the vport

vcpu 0 must be always enabled and non-hotpluggable, thus you can't
modify it using the vcpu hotplug APIs. Disallow it so that users can't
create invalid configurations.

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

Part of the condition was reverted so no value update was propagated
through.

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

It was added in commit 6c2e4c38
so that Coverity would not complain about passing -1 to
qemuDomainDetachThisHostDevice(), but the function in question
has changed since and so the annotation doesn't apply anymore.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>

Change the settings from qemuDomainUpdateDeviceLive() as otherwise the
call would succeed even though nothing has changed.

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

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

It may happen that a domain is started without any huge pages.
However, user might try to attach a DIMM module later. DIMM
backed by huge pages (why would somebody want to mix regular and
huge pages is beyond me). Therefore we have to create the dir if
we haven't done so far.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Use ATTRIBUTE_FALLTHROUGH, introduced by commit
5d84f596, instead of comments to
indicate that the fall through is an intentional behavior.
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>

The virDomainUSBAddressEnsure returns 0 or -1, so commit id 'de325472'
checking for 1 like qemuDomainAttachChrDeviceAssignAddr was wrong.
Signed-off-by: NShivaprasad G Bhat <sbhat@linux.vnet.ibm.com>

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

Currently, any attempt to change MTU on an interface that is
plugged to a running domain is silently ignored. We should either
do what's asked or error out. Well, we can update the host side
of the interface, but we cannot change 'host_mtu' attribute for
the virtio-net device. Therefore we have to error out.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

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

While implementing MTU (572eda12 and friends), I've forgotten
to actually set MTU on the host NIC in case of hotplug. We
correctly tell qemu on the monitor what the MTU should be, but we
are not actually setting it on the host NIC.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

When adding the aliased serial stub console, the structure wasn't
properly allocated (VIR_ALLOC instead of virDomainChrDefNew) which then
resulted in SIGSEGV in virDomainChrSourceIsEqual during a serial device
coldplug.

https://bugzilla.redhat.com/show_bug.cgi?id=1434278Signed-off-by: NErik Skultety <eskultet@redhat.com>

This reverts commit 2841e675.

It turns out that adding the host_mtu field to the PCI capabilities in
the guest bumps the length of PCI capabilities beyond the 32 byte
boundary, so the virtio-net device gets 64 bytes of ioport space
instead of 32, which offsets the address of all the other following
devices. Migration doesn't work very well when the location and length
of PCI capabilities of devices is changed between source and
destination.

This means that we need to make sure that the absence/presence of
host_mtu on the qemu commandline always matches between source and
destination, which means that we need to make setting of host_mtu an
opt-in thing (it can't happen automatically when the bridge being used
has a non-default MTU, which is what commit 2841e675 implemented).

I do want to re-implement this feature with an <mtu auto='on'/>
setting, but probably won't backport that to any stable branches, so
I'm first reverting the original commit, and that revert can be pushed
to the few releases that have been made since the original (3.1.0 -
3.3.0)

Resolves: https://bugzilla.redhat.com/1449346

Only set releaseaddr to true after the address has been
reserved successfully.

https://bugzilla.redhat.com/show_bug.cgi?id=1452581Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

The error message would contain first vcpu id after the list of vcpus
selected for modification. To print the proper vcpu id remember the
first vcpu selected to be modified.

Adjust the current message to make it clear, that it is the hotplug
operation that is unsupported with the given host device type.

https://bugzilla.redhat.com/show_bug.cgi?id=1450072Signed-off-by: NErik Skultety <eskultet@redhat.com>

As with virtio-scsi, the "internal error" messages after
preparing a vhost-scsi hostdev overwrites more meaningful
error messages deeper in the callchain.  Remove it too.
Signed-off-by: NEric Farman <farman@linux.vnet.ibm.com>

I tried to attach a SCSI LUN to two different guests, and forgot
to specify "shareable" in the hostdev XML.  Attaching the device
to the second guest failed, but the message was not helpful in
telling me what I was doing wrong:

  $ cat scsi_scratch_disk.xml
    <hostdev mode='subsystem' type='scsi'>
      <source>
        <adapter name='scsi_host3'/>
        <address bus='0' target='15' unit='1074151456'/>
      </source>
    </hostdev>

  $ virsh attach-device dasd_sles_d99c scsi_scratch_disk.xml
  Device attached successfully

  $ virsh attach-device dasd_fedora_0e1e scsi_scratch_disk.xml
  error: Failed to attach device from scsi_scratch_disk.xml
  error: internal error: Unable to prepare scsi hostdev: scsi_host3:0:15:1074151456

I eventually discovered my error, but thought it was weird that
Libvirt doesn't provide something more helpful in this case.
Looking over the code we had just gone through, I commented out
the "internal error" message, and got something more useful:

  $ virsh attach-device dasd_fedora_0e1e scsi_scratch_disk.xml
  error: Failed to attach device from scsi_scratch_disk.xml
  error: Requested operation is not valid: SCSI device 3:0:15:1074151456 is already in use by other domain(s) as 'non-shareable'

Looking over the error paths here, we seem to issue better
messages deeper in the callchain so these "internal error"
messages overwrite any of them.  Remove them, so that the
more detailed errors are seen.
Signed-off-by: NEric Farman <farman@linux.vnet.ibm.com>

Signed-off-by: NEric Farman <farman@linux.vnet.ibm.com>

it should be a comparison of modes between new and old devices. So
the argument of the second virDomainNetGetActualDirectMode should be
newdev.
Signed-off-by: NZhiPeng Lu <lu.zhipeng@zte.com.cn>

In the vcpu hotplug code if exit from the monitor failed we would still
attempt to save the status XML. When the daemon is terminated the
monitor socket is closed. In such case, the written status XML would not
contain the monitor path and thus be invalid.

Avoid this issue by only saving status XML on success of the monitor
command.

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

The function is used only in the hotplug module.

Introduce new wrapper functions without *Machine* in the function
name that take the whole virDomainDef structure as argument and
call the existing functions with *Machine* in the function name.

Change the arguments of existing functions to *machine* and *arch*
because they don't need the whole virDomainDef structure and they
could be used in places where we don't have virDomainDef.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Make sure that non-hotpluggable vcpus stay clustered at the beginning
after modifying persistent definition.

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

Validate that users don't try to disable vcpu 0.

Vcpu order is required to stay sequential. Clear the order on cpu
coldplug to avoid issues with removing vcpus out of sequence.

'next' is declared as 'ssize_t' so use '%zd'

Buggy condition meant that vcpu0 would not be iterated in the checks.
Since it's not hotpluggable anyways we would not be able to break the
configuration of a live VM.

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

A mediated device will be identified by a UUID (with 'model' now being
a mandatory <hostdev> attribute to represent the mediated device API) of
the user pre-created mediated device. We also need to make sure that if
user explicitly provides a guest address for a mdev device, the address
type will be matching the device API supported on that specific mediated
device and error out with an incorrect XML message.

The resulting device XML:
<devices>
  <hostdev mode='subsystem' type='mdev' model='vfio-pci'>
    <source>
      <address uuid='c2177883-f1bb-47f0-914d-32a22e3a8804'>
    </source>
  </hostdev>
</devices>
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Add an asyncJob argument for add/delete TLS Objects. A future patch will
add/delete TLS objects from a migration which may have a job to join.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Some users might want to pass a blockdev or a chardev as a
backend for NVDIMM. In fact, this is expected to be the mostly
used configuration. Therefore libvirt should allow the device in
devices CGroup then.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Now that we have APIs for relabel memdevs on hotplug, fill in the
missing implementation in qemu hotplug code.

The qemuSecurity wrappers might look like overkill for now,
because qemu namespace code does not deal with the nvdimms yet.
Nor does our cgroup code.  But hey, there's cgroup_device_acl
variable in qemu.conf. If users add their /dev/pmem* device in
there, the device is allowed in cgroups and created in the
namespace so they can successfully passthrough it to the domain.
It doesn't look like overkill after all, does it?
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Frankly, this function is one big mess. A lot of arguments,
complicated behaviour. It's really surprising that arguments were
in random order (input and output arguments were mixed together),
the documentation was outdated, the description of return values
was bogus.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>