When translating CPUID data into CPU model + features, the code
sometimes uses an unexpected CPU model. There may be several reasons for
this, starting with wrong expectations and ending with an actual bug in
our code. These debug messages will help determining the reason.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Since 1b4f66e "security: introduce virSecurityManager
(Set|Restore)ChardevLabel" this is a public API of security manager.
Implementing this in apparmor avoids miss any rules that should be
added for devices labeled via these calls.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
virSecurityManagerDomainSetPathLabel is used to make a path known
to the security modules, but today is used interchangably for
- paths to files/dirs to be accessed directly
- paths to a dir, but the access will actually be to files therein
Depending on the security module it is important to know which of
these types it will be.
The argument allowSubtree augments the call to the implementations of
DomainSetPathLabel that can - per security module - decide if extra
actions shall be taken.
For now dac/selinux handle this as before, but apparmor will make
use of it to add a wildcard to the path that was passed.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This came up in discussions around huge pages, but it will cover
more per guest paths that should be added to the guests apparmor profile:
- keys via qemuDomainWriteMasterKeyFile
- per domain dirs via qemuProcessMakeDir
- memory backing paths via qemuProcessBuildDestroyMemoryPathsImpl
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1527740
Users might use a block device as UEFI VAR store. Or even have
OVMF stored there. Therefore, when starting a domain and separate
mount namespace is used, we have to create all the /dev entries
that are configured for the domain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit id '162efa1a' added support hotplug a redirdev, but
did not add the hot unplug. This patch will add that support
to allow usage of the detach-device --live on the device.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Some ARM platforms, such as the original Raspberry Pi, report the
CPU frequency in the BogoMIPS field of /proc/cpuinfo, so libvirt
parsed that field and returned it through its API.
However, not only many more boards don't report any value there,
but several - including ARMv8-based server hardware, and even the
more recent Raspberry Pi 3 - use this field as originally intended:
to report the BogoMIPS value instead of the CPU frequency.
Since we have no way of detecting how the field is being used,
it's better to report no information at all rather than something
ludicrous like "your shiny 96-core aarch64 virtualization host's
CPUs are running at a whopping 100 MHz".
Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1206353
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Make the parser both more strict, by not ignoring errors reported
by virStrToLong_ui(), and more permissive, by not failing due to
unrelated fields which just happen to have a know prefix and
accepting any amount of whitespace before the numeric value.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Instead of a generic "your architecture", print the actual
architecture name.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
All different architectures use the same copy-pasted code to parse
processor frequency information from /proc/cpuinfo. Let's extract that
code into a function to avoid repetition.
We now also tolerate if the parsing of /proc/cpuinfo is not successful
and just report a warning instead of bailing out and abandoning the rest
of the CPU information.
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1528502
So imagine you have /dev/blah symlink which points to /dev/sda.
You attach /dev/blah as disk to your domain. Libvirt correctly
creates the /dev/blah -> /dev/sda symlink in the qemu namespace.
However, then you detach the disk, change the symlink so that it
points to /dev/sdb and tries to attach the disk again. This time,
however, the attach fails (well, qemu attaches wrong disk)
because the code assumes that symlinks don't change. Well they
do.
This is inspired by test fix written by Eduardo Habkost.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When the -machine pseries,max-cpu-compat=X is supported use
machine parameter instead of -cpu host,compat=X parameter as
that is deprecated now with qemu >= v2.10.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1519146
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Since we have user aliases it may happen that users want to
change it using 'update-device'. Instead of ignoring it silently,
error out loudly. Note that we don't limit the check just for
"ua-" prefixes because users might try to change libvirt
generated aliases too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The qemuMonitorJSONMakeCommand can properly handle a NULL string
by using the "S:" parameter instead of "s:", so let's use that
of having in if/else condition that only adds the "s:".
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell
and Broadwell machines.
Therefore, place microcode version in the virQEMUCaps struct and
XML, and rebuild the cache if the versions do not match.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell and
Broadwell machines.
In order to track the x86 microcode version in the QEMU capabilities,
we have to fetch it and store it in the host CPU. This also makes the
version visible in "virsh capabilities", which is a nice side effect.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The function will be used to initialize internal data of the x86 CPU
driver (including the CPU map).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This new API reads host's CPU microcode version from /proc/cpuinfo.
Unfortunately, there is no other way of reading microcode version which
would be usable from both system and session daemon.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1519130
Commit id 'dc692438' reverted the automagic addition of a SCSI
controller attempt during virDomainHostdevAssignAddress; however,
the logic to determine where to place the next_unit depended upon
the "new" controller being added. Without the new controller the
the next time through the call for the next SCSI hostdev found
would result in the "next_unit" never changing from 0 (zero) and
as a result the addition of the device will fail due to being a
duplicate unit number of the first with the error message:
virDomainDefCheckDuplicateDriveAddresses:$line : unsupported
configuration: SCSI host address controller='0' bus='1'
target='0' unit='0' in use by another SCSI host device
So instead of walking the controller list looking for SCSI
controllers, all we can do is "pretend" that they exist and
allow other code to create them later as necessary.
In virDomainDefMaybeAddHostdevSCSIcontroller when we add a new
controller because someone neglected to add one or we're adding
one because the existing one is full, we should copy over the
model number from the existing controller since whatever we
create should at least have the same characteristics as the one
we cannot use because it's full.
NB: This affects the existing hostdev-scsi-autogen-address test
which would add a default ('lsi') SCSI controller for the various
scsi_host's that would create a controller for the hostdev.
When qemuDomainFindOrCreateSCSIDiskController adds a controller,
let's use the same model as a currently found controller under the
assumption that the reason to add the controller in hotplug is
because virDomainHostdevAssignAddress determined that there were
too many devices on the existing controller, but only assigned a
new controller index and did not add a new controller and we
desire to use the same controller model as any existing controller
and not take a chance that qemuDomainSetSCSIControllerModel would
use a default that may be incompatible.
Let's move the udevEnumerateDevices into a thread to "speed
up" the initialization process. If the enumeration fails we
can set the Quit flag to ensure that udevEventHandleCallback
will not run.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Replace virNetServerClientNeedAuth with
virNetServerClientIsAuthenticated because it makes it clearer what it
means.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
'Squash' virNetServerClientNeedAuthLocked into
virNetServerClientNeedAuth and remove virNetServerClientNeedAuthLocked
as it's not longer needed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
There is a race between virNetServerProcessClients (main thread) and
remoteDispatchAuthList/remoteDispatchAuthPolkit/remoteSASLFinish (worker
thread) that can lead to decrementing srv->nclients_unauth when it's
zero. Since virNetServerCheckLimits relies on the value
srv->nclients_unauth the underrun causes libvirtd to stop accepting
new connections forever.
Example race scenario (assuming libvirtd is using policykit and the
client is privileged):
1. The client calls the RPC remoteDispatchAuthList =>
remoteDispatchAuthList is executed on a worker thread (Thread
T1). We're assuming now the execution stops for some time before
the line 'virNetServerClientSetAuth(client, 0)'
2. The client closes the connection irregularly. This causes the
event loop to wake up and virNetServerProcessClient to be
called (on the main thread T0). During the
virNetServerProcessClients the srv lock is hold. The condition
virNetServerClientNeedAuth(client) will be checked and as the
authentication is not finished right now
virNetServerTrackCompletedAuthLocked(srv) will be called =>
--srv->nclients_unauth => 0
3. The Thread T1 continues, marks the client as authenticated, and
calls virNetServerTrackCompletedAuthLocked(srv) =>
--srv->nclients_unauth => --0 => wrap around as nclient_unauth is
unsigned
4. virNetServerCheckLimits(srv) will disable the services forever
To fix it, add an auth_pending field to the client struct so that it
is now possible to determine if the authentication process has already
been handled for this client.
Setting the authentication method to none for the client in
virNetServerProcessClients is not a proper way to indicate that the
counter has been decremented, as this would imply that the client is
authenticated.
Additionally, adjust the existing test cases for this new field.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Combine virNetServerClientSetAuth(client,
VIR_NET_SERVER_SERVICE_AUTH_NONE) and virNetServerTrackCompletedAuth
into one new function named virNetServerSetClientAuthenticated.
After using this new function the function
virNetServerTrackCompletedAuth was superfluous and is therefore
removed. In addition, it is not very common that a
'{{function}}' (virNetServerTrackCompletedAuth) does more than just
the locking compared to
'{{function}}Locked' (virNetServerTrackCompletedAuthLocked).
virNetServerTrackPendingAuth was already superfluous and therefore
it's also removed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The lock for @client must not only be held for the duration of
checking whether the client wants to close, but also for as long as
we're closing the client. The same applies to the tracking of
authentications.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add virNetServerClientAuthMethodImpliesAuthenticated() for deciding
whether a authentication method implies that a client is automatically
authenticated or not. Use this new function in
virNetServerClientNeedAuthLocked().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
This makes the code more efficient.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Be more precise in which cases the authentication is needed and
introduce *Locked.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Add typedef for the anonymous enum used for the authentication methods
and remove the default case. This allows the usage of the type in a
switch statement and taking advantage of the compilers feature to
detect uncovered cases.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
All calls to virDomainAuditCgroupPath() were passing 'rc == 0' as
argument, when it was supposed to pass the 'rc' value directly.
As a consequence, the audit events that were supposed to be
logged (actual cgroup changes) were never being logged, and bogus
audit events were logged when using regular files as disk image.
Fix all calls to use the return value of
virCgroup{Allow,Deny}Device*() directly as the 'rc' argument.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After commit a693fdb 'vol-dumpxml' missed the ability to show backingStore
information. This commit adds a volume type for files that fixes this
problem.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1529663
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1448149
If a domain has no numa nodes, that means we don't put any
memory-backend-file onto the qemu command line. That in turn
means we can't set access='shared'. Therefore, we should produce
an error instead of ignoring the setting silently.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The PROBE macro used in qemuMonitorIOProcess and the VIR_DEBUG message
in qemuMonitorJSONIOProcess create a lot of logging churn when debug
logging is enabled during monitor communication.
The messages logged from the PROBE macro are rather useless since they
are reporting the partial state of receiving the reply from qemu. The
actual full reply is still logged in qemuMonitorJSONIOProcessLine once
the full message is received.
There are a few more description-related issues that commit @9026d115
forgot to address.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Set a transient hostname on containers. The hostname is computed from
the container name, only keeping the valid characters [a-zA-Z0-9-] in it.
This filtering is based on RFC 1123 and allows a digit to start the
hostname.
There's no argument named @result, use @matches instead.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
These are already exported at header file level because of
VIR_ENUM_DECL being in numa_conf.h. However, they are not being
exported at object level because of missing libvirt_private.syms
record.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
QEMU 2.7 and newer don't allow guests to start unless the initial
vCPUs count is a multiple of the vCPU hotplug granularity, so
validate it and report an error if needed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283700
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
While at the moment we're only performing a single check that is
connected to vCPU hotplugging, we're going to introduce a second
one soon. Move the topology check underneath the capability check
to make that easier; since, after this change, the 'topologycpus'
variable doesn't need to have function scope, we move its
declaration to the inner scope as well.
The comments around the check are modified in order to explain
the different QEMU versions involved.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Similar to qemuDomainAddChardevTLSObjects let's move the chardev
source must be TCP and it has the @haveTLS flag set checks before
trying to delete the TLS objects.
For the Chr device this represents no change; however, for RNG device
this is an additionaly check that was missed in commit id '68808516'.
Before adding the objects, TCP and haveTLS are checked.
Let's make a comment deletion helper similar to the Add helper
that can be called after the ExitMonitor.
The modify qemuDomainRemoveChrDevice and qemuDomainRemoveRNGDevice
to call the helper instead of inlining the copy and pasted code.
So far clients were closed when disposing the daemon, after the state
driver cleanup. This was leading to libvirtd crashing at shutdown due
to missing driver.
Moving the client close in virNetServerClose() fixes the problem.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Prior to this change, we relied solely on the inherited readonly
attribute of a service's socket. This only worked for our UNIX sockets
(and only to some degree), but doesn't work for TCP sockets which are RW
by default, but such connections support RO as well. This patch forces
an update on the client object once we have established a connection to
reflect the nature of the connection itself rather than relying on the
underlying socket's attributes.
Clients connected to the admin server have always been connected as RW
only.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1524399
Signed-off-by: Erik Skultety <eskultet@redhat.com>
ebtables/iptables processing is skipped for any interface connected to
Open vSwitch (they have their own packet filtering), likewise for
midonet (according to
http://blog.midokura.com/2016/04/midonet-rule-chains), but libvirt
would allow adding a <filterref> to interfaces connected in these
ways, so the user might mistakenly believe they were being protected.
This patch checks for a non-NULL <virtualport> element for an
interface (or its network) and logs an error if <virtualport> and
<filterref> are both present. This could cause some previously working
domains to no longer start, but that's really the whole point of this
patch - to warn people that their filterref isn't protecting them as
they might have thought.
I don't bother checking this during post-parse validation, because
such a check would be incomplete - it's possible that a network would
have a <virtualport> that would be applied to an interface, and you
can't know that until the domain is started.
Resolves: https://bugzilla.redhat.com/1502754
When the <bandwidth> of an interface is changed with update-device,
the old settings are cleared with tc, then new settings added with
tc. But if the <bandwidth has been removed, the old settings weren't
being removed, so the bandwidth restrictions would still be active on
the interface although the interface status in libvirt showed that
they had been removed.
This patch fixes it by calling virNetDevBandwidthClear() if the
"modification" to the interface bandwidth was to completely clear
it.
An alternative could have been to modify virNetDevBandwidthSet() to
always clear existing bandwith settings at the beginning of the
function (currently it short circuits in that case, doing nothing),
but that would have led to cases where virNetDevBandwidthClear() was
now being called in cases where it previously wasn't, and while many
of those cases would be NOPs, there could be cases where it would
cause an error. The way this patch works, the ...Clear() function is
only called in cases where the ...Set() function had previously been
called successfully, so the risk of regression is minimized.
Resolves: https://bugzilla.redhat.com/1454709
Also call qemuDomainRemoveInputDevice if we receive the
event after the Detach API ends.
Commit 67486bb failed to include this.
https://bugzilla.redhat.com/show_bug.cgi?id=1524837
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
VM drivers may need to store additional private data to the status XML
so that it can be restored after libvirtd restart. Since not everything
is needed add a callback infrastructure, where VM drivers can add only
stuff they need.
Note that the private data is formatted as a <privateData> sub-element
of the <disk> or <backingStore> <source> sub-element. This is done since
storing it out of band (in the VM private data) would require a complex
matching process to allow to put the data into correct place.
https://bugzilla.redhat.com/show_bug.cgi?id=1523564
If the vhost-scsi device file cannot be found, the generic error
"error: An error occurred, but the cause is unknown"
is returned. Let's add a real error message to make it clear
why the failure occurred.
We cannot be sure someone initialized the passed *vhostfd and we
certainly don't want or need to be calling VIR_FORCE_CLOSE on what
probably is -1. So let's just return -1 immediately.
Commit id '70249927b' neglected to cover this case because the test
had taken the "shortcut" to already add the <address>; however, when
the PCI address assignment code was adjusted by commit id '70249927'
the vhost-scsi (VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) wasn't
covered thus returning a 0 for pciFlags. So I altered the tests too
to make sure it doesn't happen again.
Previously the qemuxml2xmloutdata was a softlink to the source
qemuxml2argvdata, so I unlinked and recreated the output file to
force generation of the adddress. Without the test changes, an
address generation returns:
libvirt: Domain Config error : internal error: Cannot automatically
add a new PCI bus for a device with connect flags 00
if an address was supplied in the test, a restart of libvirtd or
edit of a guest would display the following opaque message:
warning : qemuDomainCollectPCIAddress:1237 :
qemuDomainDeviceCalculatePCIConnectFlags() thinks that the device
with PCI address 0000:00:09.0 should not have a PCI address
where the address is related to the guest PCI address provided.
virStringSplit may return NULL, so we must handle that.
Cc: John Ferlan <jferlan@redhat.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Remove the unnecessary clearing of address_array as VIR_ALLOC_N
initialized the array already.
Cc: John Ferlan <jferlan@redhat.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Commit id 'c5c96545' neglected to validate that the srcPriv was
non-NULL before dereferencing. Similar problem to what was fixed
by commit id '8056721c' but missed during multiple rebases and
code reworks.
Now that we have a private storage pool list, we can take the next
step and convert to using objects. In this case, we're going to use
RWLockable objects (just like every other driver) with two hash
tables for lookup by UUID or Name.
Along the way the ForEach and Search API's will be adjusted to use
the related Hash API's and the various FindBy functions altered and
augmented to allow for HashLookup w/ and w/o the pool lock already
taken.
After virStoragePoolObjRemove we will need to virObjectUnref(obj)
after to indicate the caller is "done" with it's reference. The
Unlock occurs during the Remove.
The NumOf, GetNames, and Export functions all have their own callback
functions to return the required data and the FindDuplicate code
can use the HashSearch function callbacks.
Commit id '5ab746b8' introduced the function as perhaps a copy
of storageVolLookupByPath; however, it did not use the @cleanpath
variable even though it used the virFileSanitizePath. So in essance
the only "check" being done for failure is whether it was possible
to strdup the path.
Looking at the virStoragePoolDefParseXML one will note that the
target.path is stored using the result of virFileSanitizePath.
Therefore, this function should sanitize and use the input @path
for the argument to storagePoolLookupByTargetPathCallback which
is comparing against stored target.path values.
Additionally, if there was an error we should use the proper error
of VIR_ERR_NO_STORAGE_POOL (instead of VIR_ERR_NO_STORAGE_VOL).
Replace the error message during startup of libvirtd with an info
message if audit_level < 2 and audit is not supported by the
kernel. Audit is not supported by the current kernel if the kernel
does not have audit compiled in or if audit is disabled (e.g. by the
kernel cmdline).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
==32171== 32 bytes in 1 blocks are definitely lost in loss record 44 of 107
==32171== at 0x4C2DEF6: calloc (vg_replace_malloc.c:711)
==32171== by 0x55744A9: virAllocN (viralloc.c:191)
==32171== by 0x12CED2: xenMakeIPList (xen_common.c:1186)
==32171== by 0x12D0BE: xenFormatNet (xen_common.c:1221)
==32171== by 0x12F0D2: xenFormatVif (xen_common.c:1889)
==32171== by 0x12F2B4: xenFormatConfigCommon (xen_common.c:1944)
==32171== by 0x13BA32: xenFormatXL (xen_xl.c:1971)
==32171== by 0x1186CA: testCompareParseXML (xlconfigtest.c:105)
==32171== by 0x118A64: testCompareHelper (xlconfigtest.c:205)
==32171== by 0x119E36: virTestRun (testutils.c:180)
==32171== by 0x11970E: mymain (xlconfigtest.c:301)
==32171== by 0x11BEE3: virTestMain (testutils.c:1119)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
==30399== 180 (144 direct, 36 indirect) bytes in 3 blocks are definitely lost in loss record 91 of 111
==30399== at 0x4C2E0FF: realloc (vg_replace_malloc.c:785)
==30399== by 0x5574572: virReallocN (viralloc.c:245)
==30399== by 0x5574668: virExpandN (viralloc.c:294)
==30399== by 0x55747AB: virResizeN (viralloc.c:352)
==30399== by 0x560074D: virStringSplitCount (virstring.c:115)
==30399== by 0x137A59: xenParseXLVnuma (xen_xl.c:442)
==30399== by 0x13952B: xenParseXL (xen_xl.c:1064)
==30399== by 0x11884D: testCompareFormatXML (xlconfigtest.c:152)
==30399== by 0x118A87: testCompareHelper (xlconfigtest.c:207)
==30399== by 0x119E36: virTestRun (testutils.c:180)
==30399== by 0x119186: mymain (xlconfigtest.c:274)
==30399== by 0x11BEE3: virTestMain (testutils.c:1119)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
ncpus would be -1 on error and the cleanup for loop would not be skipped
in this case.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1522706
If domain is active, but the undefine API was called without the
VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag set, the following incorrect
error message is produced:
error: Requested operation is not valid: cannot delete inactive domain with nvram
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Move the IDE controller check from command line building to
controller def validation. Also explicitly include the avoidance
check for the implicit IDE controller from qemuBuildSkipController.
Cause the IDE case for command line building to generate a
failure if called to add an IDE since that shouldn't happen
if the Validate code did the right thing.
Move the call to qemuDomainCheckCCWS390AddressSupport from
qemuBuildControllerDevStr to qemuDomainDeviceDefValidateController.
This means we will get the qemuCaps from the driver opaque
variable passed to qemuDomainDeviceDefValidate.
Xen's xl config format has long supported specifying multiple IP
addresses for virtual interfaces. E.g.
vif = [ "ip=10.0.0.1 10.1.1.1 2000::1, ..." ]
Add support for converting multiple IP addresses to/from domXML.
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
vif-* scripts support it for a long time, and expect addresses to be
separated by spaces. Add appropriate support to libxl driver.
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
When reconnecting to a running domain started by old libvirt, which did
not change host-model into a custom CPU definition, we replace the CPU
definition with a specific CPU model from host capabilities. However,
that CPU model may not be supported by the running qemu process. We need
to translate the CPU model to one of the models which libvirt could have
used when starting the domain.
https://bugzilla.redhat.com/show_bug.cgi?id=1521202
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
virQEMUCapsProbeQMPCPUDefinitions is now a small wrapper which fills in
qemuCaps with CPU models fetched by virQEMUCapsFetchCPUDefinitions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Since we are re-detecting the backing chain after pivoting to the active
block commit target (or block copy target) the disk index needs to be
reset to 0. This is necessary since we move a member of the backing
chain to disk->src but clear indexes only starting from
disk->src->backingStore. The freshly detected images have indexes
starting from 1, but since we've pivoted into an image which was
previously a backing store it would have a non-0 index.
The lookup function would then return the top of the chain for queries
like 'vda[1]' instead of the first backing store.
This problem will not be present once we keep the disk indexes stable.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1519745
Since commit 5e5019bf, we've no longer use
VIR_ERR_AGENT_UNSYNCED anymore.
Mark it as DEPRECATED.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
virStorageFileReportBrokenChain uses data from the driver private data
pointer to print the user and group. This would lead to a crash in call
paths where we did not initialize the storage backend as recently added
in commit 24e47ee2b9 to qemuDomainDetermineDiskChain.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1522682
Separate the logic of creating devices from their gathering.
Use this new function in qemuDomainNamespaceSetupHostdev and
qemuDomainNamespaceSetupDisk.
This patch pass event error up to the place where we can
use it. Error is passed only for sync blockjob event mode
as we can't use the error in async mode. In async mode we
just pass the event details to the client thru event API
but current blockjob event API can not carry extra parameter.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The unprivileged libvirtd does not support nwfilter config, by leaves the
driver active. It is supposed to result in all APIs being an effective
no-op, but several APIs rely on driver->nwfilters being non-NULL, or they
will reference a NULL pointer. Rather than adding checks for NULL in many
places, just make sure driver->nwfilters is always initialized.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
SELinux and DAC drivers already have both functions but they were not
exported as public API of security manager.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The manual page clearly states that
gettid() is Linux-specific and should not be used in programs
that are intended to be portable.
Unfortunately, it looks like macOS implemented the functionality
and defined SYS_gettid accordingly, only to deprecate syscall()
altogether with 10.12 (Sierra), released last late year.
To avoid compilation errors, call gettid() on Linux only.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Clean up the style a bit w/r/t to not using a unary operator on an
integer value that could be zero - compare vs. zero instead.
Set the def->mem_nodes[*].distances to rdist or ldist inside the
if condition - no need to set outside since the value being set
to is what was fetched.
During cleanup, be sure to initialize the ndistances on error and
use the < 0 comparison not the unary one.
==899== 39 bytes in 1 blocks are definitely lost in loss record 732 of 1,003
==899== at 0x4C2AEDF: malloc (vg_replace_malloc.c:299)
==899== by 0x8B68CE7: vasprintf (in /lib64/libc-2.25.so)
==899== by 0x55498D2: virVasprintfInternal (virstring.c:708)
==899== by 0x55499E7: virAsprintfInternal (virstring.c:729)
==899== by 0x2BECFFF0: qemuGetMemoryBackingBasePath (qemu_conf.c:1757)
==899== by 0x2BF23225: qemuStateInitialize (qemu_driver.c:893)
==899== by 0x563073D: virStateInitialize (libvirt.c:770)
==899== by 0x124CC4: daemonRunStateInit (libvirtd.c:834)
==899== by 0x55521CD: virThreadHelper (virthread.c:206)
==899== by 0x88D9686: start_thread (in /lib64/libpthread-2.25.so)
==899== by 0x8BEAEFE: clone (in /lib64/libc-2.25.so)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
==1277== 8 bytes in 4 blocks are definitely lost in loss record 39 of 131
==1277== at 0x4C2AEDF: malloc (vg_replace_malloc.c:299)
==1277== by 0x68BBBC8: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4)
==1277== by 0x53B1DC2: virXMLPropString (virxml.c:510)
==1277== by 0x53D696A: virDomainDiskBackingStoreParse (domain_conf.c:8639)
==1277== by 0x53DA684: virDomainDiskDefParseXML (domain_conf.c:9590)
==1277== by 0x53F619F: virDomainDefParseXML (domain_conf.c:19233)
==1277== by 0x53F96EE: virDomainDefParseNode (domain_conf.c:20083)
==1277== by 0x53F9540: virDomainDefParse (domain_conf.c:20027)
==1277== by 0x53F95E6: virDomainDefParseFile (domain_conf.c:20053)
==1277== by 0x44D1D4: testCompareDomXML2XMLFiles (testutils.c:1265)
==1277== by 0x42FC7C: testXML2XMLActive (qemuxml2xmltest.c:71)
==1277== by 0x44AD20: virTestRun (testutils.c:180)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
==861== 3 bytes in 1 blocks are definitely lost in loss record 3 of 168
==861== at 0x4C2AEDF: malloc (vg_replace_malloc.c:299)
==861== by 0x8C7FBC8: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4)
==861== by 0x5DCCDC2: virXMLPropString (virxml.c:510)
==861== by 0x5DF1232: virDomainDiskSourceNetworkParse (domain_conf.c:8445)
==861== by 0x5DF1728: virDomainDiskSourceParse (domain_conf.c:8576)
==861== by 0x5DF41A5: virDomainDiskDefParseXML (domain_conf.c:9238)
==861== by 0x5E1119F: virDomainDefParseXML (domain_conf.c:19233)
==861== by 0x5E146EE: virDomainDefParseNode (domain_conf.c:20083)
==861== by 0x5E14540: virDomainDefParse (domain_conf.c:20027)
==861== by 0x5E145E6: virDomainDefParseFile (domain_conf.c:20053)
==861== by 0x4053CC: testCompareXMLToArgv (qemuxml2argvtest.c:455)
==861== by 0x41F135: virTestRun (testutils.c:180)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Raw local files do not pass through the backing store detector and thus
the code did not allocate the required backing store terminator for
them. Previously the terminating element would be formatted into the XML
since the default values used for the metadata allowed that. This is a
regression since a693fdba01 which was not detected in the review.
This patch also reverts all the changes in the test files.
Until now we would skip loading of the backing chain for files which
don't support backing chains only when starting up the VM. Move the
check from qemuProcessPrepareHostStorage with some adaptations so that's
always applied.
The graphics code is complex and there are a lot of exceptions and
backward compatible combinations. One of them is the possibility
to configure "spice_auto_unix_socket" in qemu.conf which will convert
all spice graphics with listen type "address" without any address
specified to listen type "socket" when the guest is started.
We don't format this generated socket into migratable XML to make
migration work with older libvirt. However, spice has another
exception that if autoport='no' and there is no port configured
it is converted to listen type "none". Because of this we need
to format autoport='yes' to make sure that the listen type will
be the same as the offline XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511407
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
In status XML, we do not store the QEMU version information, we only
format all the capabilities. We dropped QEMU_CAPS_PCI_MULTIBUS
in commit 5b783379 which was released in libvirt 3.2.0.
Therefore the only way of telling if the already running domain
at the time of daemon restart has been started with a QEMU that does
use 'pci.0' or not on PPC is to look at the pci-root controller's
alias. This is not an option if the domain has a user-specified alias
for the pci-root.
Instead of reintroducing the capability, assume 'pci.0' when we have
no version information. That way the only left broken use case would
be the combination of user aliases and very old QEMU.
Partially reverts commit 3a37af1e4.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148
We do not fill out qemuCaps->arch when parsing status XML.
Use def->os.arch like we do for PPC.
This fixes hotplug after daemon restart for domains that use
a user alias for the implicit pci-root on x86.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148
For some corner cases, virQEMUCapsHasPCIMultiBus depends on the QEMU
version, which is by design not stored in the status XML and therefore
it cannot be fixed for all existing running domains.
Prefer the controller alias read from the status XML when formatting
PCI addresses and only fall back to using virQEMUCapsHasPCIMultiBus
if the alias is a user alias.
This fixes hotplug after daemon restart for domains not using user
aliases.
Partially reverts commit 937f3195.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148
Adjust function descriptions of virQEMUCapsInitCPUModelS390 and
virQEMUCapsInitCPUModel to the changes introduced with
commitID 74fc32a955.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Even though we never format the device on the QEMU command line,
as it's a platform serial device that's not user-instantiable,
we should still make sure it's available before using it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All serial devices shoule have an associated capability.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We should make sure the isa-serial device is available before
formatting it on the QEMU command line.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All serial devices shoule have an associated capability.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that <serial> and <console> on s390/s390x behave a bit more like the
other architectures, remove this extra differentation, and use sclp
console by default for new guests. New virtio consoles can still be
added, and it is actually needed because of the limited number of
instances for sclp and sclplm.
This reverts commit b1c88c1476, whose
reasons are not totally clear.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Introduce specific a target types with two models for the console
devices (sclp and sclplm) used in s390 and s390x guests, so isa-serial
is no more used for them.
This makes <serial> usable on s390 and s390x guests, with at most only
a single sclpconsole and one sclplmconsole devices usable in a single
guest (due to limitations in QEMU, which will enforce already at
runtime).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449265
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can finally introduce a specific target model for the pl011 device
used by mach-virt guests, which means isa-serial will no longer show
up to confuse users.
We make sure migration works in both directions by interpreting the
isa-serial target type, or the lack of target type, appropriately
when parsing the guest XML, and skipping the newly-introduced type
when formatting if for migration. We also verify that pl011 is not
used for non-mach-virt guests and add a bunch of test cases.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=151292
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The existing implementation set the address type for all serial
devices to spapr-vio, which made it impossible to use other devices
such as usb-serial and pci-serial; moreover, some decisions were
made based on the address type rather than the device type.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1512934
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can finally introduce a specific target model for the spapr-vty
device used by pSeries guests, which means isa-serial will no longer
show up to confuse users.
We make sure migration works in both directions by interpreting the
isa-serial target type, or the lack of target type, appropriately
when parsing the guest XML, and skipping the newly-introduced type
when formatting if for migration. We also verify that spapr-vty is
not used for non-pSeries guests and add a bunch of test cases.
This commit is best viewed with 'git show -w'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511421
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Instead duplicating the capability check for each possible target
model, introduce a small helper that matches the target model with
the corresponding capability and collapse all existing checks into
a single one.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that we've created a distinction between target type and target
model, with the latter being the concrete device name, it's time to
switch to formatting the model instead of the type.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Target model and target type must agree for the configuration
to make sense, so check that's actually the case and error out
otherwise.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This information will be used to select, and store in the guest
configuration in order to guarantee ABI stability, the concrete
(hypervisor-specific) model for serial devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Instead of validating each target type / address type combination
separately, create a small helper to perform the matching and
collapse all existing checks into a single one.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Instead of waiting until we get to command line generation, we can
validate the target for a char device much earlier.
Move all the checks out of qemuBuildSerialChrDeviceStr() and into
the new fuction. This will later allow us to validate the target
for platform devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Formatting the <target/> element for serial devices will become a
bit more complicated later on, and leaving the fallthrough behavior
there would do nothing but complicate it further.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Make the switch statement type-aware, avoid calling
virDomainChrTargetTypeToString() more than once and check its
return value before using it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The function can fail, but none of the caller were accounting
for that.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We don't need to store the return value since we never modify it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move formatting of the <target/> element for char devices out of
virDomainChrDefFormat() and into its own function.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This attribute was used to decide whether to format the type
attribute of the <target> element, but the logic didn't take into
account all possible cases and as such could lead to unexpected
results. Moreover, it's one more thing to keep track of, and can
easily fall out of sync with other attributes.
Now that we have VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE, we can
use that value to signal that no specific target type has been
configured for the serial device and as such the attribute should
not be formatted at all. All other values are now formatted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This is the first step in getting rid of the assumption that
isa-serial is the default target type for serial devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The devicePostParse() callback is invoked for all devices so that
drivers have a chance to set their own specific values; however,
virDomainDefAddImplicitDevices() runs *after* the devicePostParse()
callbacks have been invoked and can add new devices, in which case
the driver wouldn't have a chance to customize them.
Work around the issue by invoking the devicePostParse() callback
after virDomainDefAddImplicitDevices(), only for the first serial
devices, which might have been added by it. The same was already
happening for the first video device for the very same reason.
This will become important later on, when we will change
virDomainDefAddConsoleCompat() not to set a targetType for
automatically added serial devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Having a separate function for char device handling is better than
adding even more code to qemuDomainDeviceDefPostParse().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1425757
The blockdev-add code provides a mechanism to sanely provide user
and password-secret arguments for iscsi without placing them on the
command line to be viewable by a 'ps -ef' type command or needing
to create separate -iscsi devices for each disk/volume found.
So modify the iSCSI command line building to check for the presence
of the capability in order properly setup and use the domain master
secret object to encrypt the password in a secret object and alter
the parameters for the command line to utilize.
Modify the xml2argvtest to exhibit the syntax for both disk and
hostdev configurations.
Detect the capability via the query-qmp-schema for blockdev-add
to find the 'password-secret' parameter that will allow the iSCSI
code to use the master secret object to encrypt the secret for an
and only need to provide the object id of the secret on the command
line thus obsfuscating the passphrase.
Rather than picking apart the two pieces we need/want (path, hosts,
and auth)- let's allocate/use a virStorageSourcePtr for iSCSI storage.
The end result is that qemuBuildSCSIiSCSIHostdevDrvStr doesn't need
to "fake" one for the qemuBuildNetworkDriveStr call.
Libvirt prints an error on startup when it is missing host cpu model
information for any queried qemu binary. On s390 we only have host cpu model
information for kvm enabled qemu instances. So when virt type is not kvm, this
is actually not an error on s390.
This patch adds virt type as a parameter to virQEMUCapsInitCPUModelS390, and a
new return code 2 for virQEMUCapsInitCPUModel and virQEMUCapsInitCPUModelS390.
If the virt type is not kvm then we skip printing the scary error message
and return 2 because this case is actually expected behavior. The new return
code is meant to differentiate between the failure case and the case where we
simply expect the cpu model information to be unattainable.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit id '5d5c732d7' had an incorrect assignment and was found
by travis build:
storage/storage_driver.c:1668:14: error: equality comparison with extraneous
parentheses [-Werror,-Wparentheses-equality]
if ((obj == virStoragePoolObjListSearch(&driver->pools,
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Create an API to search through the storage pool objects looking for
a specific truism from a callback API in order to return the specific
storage pool object that is desired.
Create an API to walk the pools->objs[] list in order to perform a
callback function for each element of the objs array that doesn't care
about whether the action succeeds or fails as the desire is to run the
code over every element in the array rather than fail as soon as or if
one fails.
For now it'll just call the virStoragePoolObjUnlock, but a future
adjustment will do something different. Since the new API will check
for a NULL object before the Unlock call, callers no longer need to
check for NULL before calling.
The virStoragePoolObjUnlock is now private/static to virstorageobj.c
with a short term forward reference.
Commit id '36555364' removed the setting of the driver->privileged,
which the udevProcessPCI would need in order to read the PCI device
configs.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move the setup of the disk attribute to the disk source prepare function
which will allow proper usage with JSON props and move the fallback
(legacy) generating code into the block which is executed with legacy
options.
As a side-effect of this change we can clean up propagation of 'cfg'
into the command generator.
Also it's nice to see that the test output is the same even when the
value is generated in a different place.
Some drive backends allow output of debugging information which can be
configured using properties of the image. Add fields to virStorageSource
which will allow configuring them.
The 'file.password-secret' injection should be used only if we are using
the old formatter. When formatting the source string from the JSON
properties, the property should be added there.
Also drop the comment which refers to stuff that will not be used in
libvirt since -blockdev is the way to go.
Qemu has now an internal mechanism for locking images to fix specific
cases of disk corruption. This requires libvirt to mark the image as
shared so that qemu lifts certain restrictions.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378242
'share-rw' for the disk device configures qemu to allow concurrent
access to the backing storage.
The capability is checked in various supported disk frontend buses since
it does not make sense to partially backport it.
Creating a snapshot would introduce a possibly unsupported member for
sharing into the backing chain. Add a check to prevent that from
happening.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480
Disk sharing between two VMs may corrupt the images if the format driver
does not support it. Check that the user declared use of a supported
storage format when they want to share the disk.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480
Storage source format backing a shared device (e.g. running a cluster
filesystem) needs to support the sharing so that metadata are not
corrupted. Add a central function for checking this.
Since we already have such support for libxl all we need is qemu
driver adjustment. And a test case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This capability says if qemu is capable of specifying distances
between NUMA nodes on the command line. Unfortunately, there's no
real way to check this and thus we have to go with version check.
QEMU introduced this in 0f203430dd8 (and friend) which was
released in 2.10.0.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The function returns true/false depending on distance
configuration being present in the domain XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There's no point in checking if numa->mem_nodes[node].ndistances
is set if we check for numa->mem_nodes[node].distances. However,
it makes sense to check if the sibling node (@cellid) caller
passed falls within boundaries.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When QEMU dies, we read its output stored in a log file and use it for
reporting a hopefully useful error. However, virReportError will trim
the message to (VIR_ERROR_MAX_LENGTH - 1) characters, which means the
end of the log (which likely contains the error message we want to
report) may get lost. We should trim the beginning of the log instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1335534
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When reading QEMU log for reporting it as an error message, we want to
skip "char device redirected to" line. However, this string is not
printed at the beginning of a line, which means STRPREFIX will never
find it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Historically we've formatted a lot of the attributes of a disk (disk
geometry, etc) with -drive. Since we use -device now, they should be
formatted there. Extract them to a separate function for keeping
compatibility with SDcards which still use only -drive.
Start this by moving the geometry into a separate function.
When doing block commit we need to allow write for members of the
backing chain so that we can commit the data into them.
qemuDomainDiskChainElementPrepare was used for this which since commit
786d8d91b4 calls qemuDomainNamespaceSetupDisk which has very adverse
side-effects, namely it relabels the nodes to the same label it has in
the main namespace. This was messing up permissions for the commit
operation since its touching various parts of a single backing chain.
Since we are are actually not introducing new images at that point add a
flag for qemuDomainDiskChainElementPrepare which will refrain from
calling to the namespace setup function.
Calls from qemuDomainSnapshotCreateSingleDiskActive and
qemuDomainBlockCopyCommon do introduce new members all calls from
qemuDomainBlockCommit do not, so the calls are anotated accordingly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1506072
https://bugzilla.redhat.com/show_bug.cgi?id=1434451
Just like in 9324f67a57 we need to put default sata alias
(which is hardcoded to "ide", obvious, right?) onto the command
line instead of the one provided by user.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This function only queries domain @def. It doesn't change it.
Therefore it should take const pointer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The compiler can warn us if we add a value to the
virDomainChrSerialTargetType enumeration but forget to handle
it properly in the code. Let's take advantage of that.
This commit is best viewed with 'git diff -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add a separate capability for the sclplmconsole device, and check it
specifically instead of using QEMU_CAPS_DEVICE_SCLPCONSOLE for that too.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Give a better name to the capability for the sclpconsole device.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Up until now we assumed the spapr-vty device would always be
present, which is not very nice. Check for its availability before
using it instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Starting from qemu 2.11, the `-device vmcoreinfo` will create a fw_cfg
entry for a guest to store dump details, necessary to process kernel
dump with KASLR enabled and providing additional kernel details.
In essence, it is similar to -fw_cfg name=etc/vmcoreinfo,file=X but in
this case it is not backed by a file, but collected by QEMU itself.
Since the device is a singleton and shouldn't use additional hardware
resources, it is presented as a <feature> element in the libvirt
domain XML.
The device is arm/x86 only for now (targets that support fw_cfg+dma).
Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=1395248
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Because the cache banks are initialized based on the order in which their
respective directories exist on the filesystem, they can appear in different
order. This is here mainly for tests because the cache directory might have
different order of children nodes and tests would fail otherwise. It should not
be the case with sysfs, but one can never be sure. And this does not take
almost any extra time, mainly because it gets initialized once per driver.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Sometimes the size of the bitmap matters and it might not be guessed correctly
when parsing from some type of input. For example virBitmapNewData() has Byte
granularity, virBitmapNewString() has nibble granularity and so on.
virBitmapParseUnlimited() can be tricked into creating huge bitmap that's not
needed (e.g.: "0-2,^99999999"). This function provides a way to shrink the
bitmap. It is not supposed to free any memory.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Already introduced in the past with 9479642fd3, but then renamed to
virBitmapIntersect by a908e9e45e. This time we'll really use it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Our bitmaps can be represented as data (raw bytes for which we have
virBitmapNewData() and virBitmapToData()), human representation (list
of numbers in a string for which we have virBitmapParse() and
virBitmapFormat()) and hexadecimal string (for which we have only
virBitmapToString()). So let's add the missing complement for the
last one so that we can parse hexadecimal strings.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Truncate the output so that it is only as big as is needed to fit all
the bits, not all the units from the map. This will be needed in the
future in order to properly format bitmaps for kernel's sysfs files.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It is literally only a wrapper around virBitmapNewData() and
virBitmapFormat(), only the naming was wrong since it was introduced.
And because we have virBitmap*String functions where the meaning of
the 'String' is constant, this might confuse someone.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This follows the virBitmapToData() function and, similarly to
virBitmapNewData(), we'll be able to have virBitmapNewString() later
on without name confusion.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We can't output better memory sizes if we want to be compatible with libvirt
older than the one which introduced /memory/unit, but for new things we can just
output nicer capacity to the user if available. And this function enables that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Resolve a storage driver crash as a result of a long running
storageVolCreateXML when the virStorageVolPoolRefreshThread is
run as a result of when a storageVolUpload completed and ran the
virStoragePoolObjClearVols without checking if the creation
code was currently processing a buildVol after incrementing
the driver->asyncjob count.
The refreshThread will now check the pool asyncjob count before
attempting to pursue the pool refresh. Adjust the documentation
to describe the condition.
Crash from valgrind is as follows (with a bit of editing):
==21309== Invalid read of size 8
==21309== at 0x153E47AF: storageBackendUpdateVolTargetInfo
==21309== by 0x153E4C30: virStorageBackendUpdateVolInfo
==21309== by 0x153E52DE: virStorageBackendVolRefreshLocal
==21309== by 0x153DE29E: storageVolCreateXML
==21309== by 0x562035B: virStorageVolCreateXML
==21309== by 0x147366: remoteDispatchStorageVolCreateXML
...
==21309== Address 0x2590a720 is 64 bytes inside a block of size 336 free'd
==21309== at 0x4C2F2BB: free
==21309== by 0x54CB9FA: virFree
==21309== by 0x55BC800: virStorageVolDefFree
==21309== by 0x55BF1D8: virStoragePoolObjClearVols
==21309== by 0x153D967E: virStorageVolPoolRefreshThread
...
==21309== Block was alloc'd at
==21309== at 0x4C300A5: calloc
==21309== by 0x54CB483: virAlloc
==21309== by 0x55BDC1F: virStorageVolDefParseXML
==21309== by 0x55BDC1F: virStorageVolDefParseNode
==21309== by 0x55BE5A4: virStorageVolDefParse
==21309== by 0x153DDFF1: storageVolCreateXML
==21309== by 0x562035B: virStorageVolCreateXML
==21309== by 0x147366: remoteDispatchStorageVolCreateXML
...
This is similar to the virDomainQemuMonitorCommand API, it can change
the domain state in a way that libvirt may not understand.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We put the server into a hash table as we do with the other daemons,
there is no compelling reason why it should have another pointer
dedicated just to the server. Besides, the locking daemon doesn't have
it and virtlogd is essentially a copy paste of virtlockd.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Most of the time it's okay to leave this up to negotiation between
the guest and the host, but in some situations it can be useful to
manually decide the behavior, especially to enforce its availability.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1308743
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Setup everything related to disks in one place rather than calling in
from various places.
The change to ordering of the setup steps is necessary since secrets
need the master key to be present.
In some cases it does not make sense to pursue that the private data
will be allocated (especially when we don't need to put anything in it).
Ensure that the code works without it.
This also fixes few crashes pointed out in
https://bugzilla.redhat.com/show_bug.cgi?id=1510323
If creation of the main JSON object containing the storage portion of a
virStorageSource would fail but we'd allocate the server structure we'd
leak it. Found by coverity.
Return NULL right away in qemuBlockStorageSourceGetBackendProps when an
invalid storage source is presented so that virJSONValueObjectAdd isn't
called with a NULL argument.
Found by coverity.
The terminator would not be parsed properly since the XPath selector was
looking for an populated element, and also the code did not bother
assigning the terminating virStorageSourcePtr to the backingStore
property of the parent.
Some tests would catch it if there wasn't bigger fallout from the change
to backing store termination in a693fdba01. Fix them properly now.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1509110
https://bugzilla.redhat.com/show_bug.cgi?id=1497410
This reverts commit bc8a99ef06.
The vhostuser is not a TAP. Therefore our QoS code is not able to
set any bandwidth. I don't really understand what I was thinking.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch generates a NUMA distance-aware libxl description from the
information extracted from a NUMA distance-aware libvirt XML file.
By default, if no NUMA node distance information is supplied in the
libvirt XML file, this patch uses the distances 10 for local and 20
for remote nodes/sockets.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Add support for describing NUMA distances in a domain's <numa> <cell>
XML description.
Below is an example of a 4 node setup:
<cpu>
<numa>
<cell id='0' cpus='0-3' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='10'/>
<sibling id='1' value='21'/>
<sibling id='2' value='31'/>
<sibling id='3' value='21'/>
</distances>
</cell>
<cell id='1' cpus='4-7' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='21'/>
<sibling id='1' value='10'/>
<sibling id='2' value='21'/>
<sibling id='3' value='31'/>
</distances>
</cell>
<cell id='2' cpus='8-11' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='31'/>
<sibling id='1' value='21'/>
<sibling id='2' value='10'/>
<sibling id='3' value='21'/>
</distances>
<cell id='3' cpus='12-15' memory='2097152' unit='KiB'>
<distances>
<sibling id='0' value='21'/>
<sibling id='1' value='31'/>
<sibling id='2' value='21'/>
<sibling id='3' value='10'/>
</distances>
</cell>
</numa>
</cpu>
A <cell> defines a NUMA node. <distances> describes the NUMA distance
from the <cell> to the other NUMA nodes (the <sibling>s). For example,
in above XML description, the distance between NUMA node0 <cell id='0'
...> and NUMA node2 <sibling id='2' ...> is 31.
Valid distance values are '10 <= value <= 255'. A distance value of 10
represents the distance to the node itself. A distance value of 20
represents the default value for remote nodes but other values are
possible depending on the physical topology of the system.
When distances are not fully described, any missing sibling distance
values will default to 10 for local nodes and 20 for remote nodes.
If distance is given for A -> B, then we default B -> A to the same
value instead of 20.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1434451
When testing user aliases it was discovered that for 440fx
machine type which has default IDE bus builtin, domain cannot
start if IDE controller has the user provided alias. This is
because for 440fx we don't put the IDE controller onto the
command line (since it is builtin) and therefore any device that
is plugged onto the bus must use the default alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
libvirt reports a fake NUMA topology in virConnectGetCapabilities
even if built without numactl support. The fake NUMA topology consists
of a single cell representing the host's cpu and memory resources.
Currently this is the case for ARM and s390[x] RPM builds.
A client iterating over NUMA cells obtained via virConnectGetCapabilities
and invoking virNodeGetMemoryStats on them will see an internal failure
"NUMA isn't available on this host" from virNumaGetMaxNode. An example
for such a client is VDSM.
Since the intention seems to be that libvirt always reports at least
a single cell it is necessary to return "fake" node memory statistics
matching the previously reported fake cell in case NUMA isn't supported
on the system.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Simply add the 5.2 SDK header to the existing unified framework. No
other special handling is needed as there's no API break between
existing 5.1 and the just added 5.2.
There was a recent report of the xen-xl converter not handling
config files missing an ending newline
https://www.redhat.com/archives/libvir-list/2017-October/msg01353.html
Commit 3cc2a9e0 fixed a similar problem when parsing content of a
file but missed parsing in-memory content. But AFAICT, the better
fix is to properly set the end of the content when initializing the
virConfParserCtxt in virConfParse().
This commit reverts the part of 3cc2a9e0 that appends a newline to
files missing it, and fixes setting the end of content when
initializing virConfParserCtxt. A test is also added to check
parsing in-memory content missing an ending newline.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
In 4f15707202 I've tried to make duplicates detection for
nested /dev mount better. However, I've missed the obvious case
when there are two same mount points. For instance if:
# mount --bind /dev/blah /dev/blah
# mount --bind /dev/blah /dev/blah
Yeah, very unlikely (in qemu driver world) but possible.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Split on the last colon and avoid parsing port if the split remainder
contains the closing square bracket, so that IPv6 addresses are
interpreted correctly.
In some cases management application needs to allocate memory for
qemu upfront and then just let qemu use that. Since we don't want
to expose path for memory-backend-file anywhere in the domain
XML, we can generate predictable paths. In this case:
$memoryBackingDir/libvirt/qemu/$shortName/$alias
where $shortName is result of virDomainDefGetShortName().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When removing path where huge pages are call virFileDeleteTree
instead of plain rmdir(). The reason is that in the near future
there's going to be more in the path than just files - some
subdirs. Therefore plain rmdir() is not going to be enough.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
At the same time, move its internals into a separate function so
that they can be reused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Very soon qemuBuildMemoryBackendStr() is going to use memory cell
aliases. Therefore set one. At the same time, move it a bit
further - if virAsprintf() fails, there's no point in setting
rest of the members.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In VirtualBox SAS and SCSI are separate controller types whereas libvirt
does not make such distinction. This patch adds support for attaching
the VBOX SAS controllers by mapping the 'lsisas1068' controller model in
libvirt XML to VBOX SAS controller type. If VBOX VM has disks attached
to both SCSI and SAS controller libvirt domain XML will have two
<controller type='scsci'> elements with index and model attributes set
accordingly. In this case, each respective <disk> element must have
<address> element specified to assign it to respective SCSI controller.
This patch adds <address> element to each <disk> device since device
names alone won't adequately reflect the storage device layout in the
VM. With this patch, the ouput produced by dumpxml will faithfully
reproduce the storage layout of the VM if used with define.
Previously any removable storage device without media attached was
omitted from domain XML dump. They're still (rightfully) omitted in
snapshot XML dump but need to be accounted properly to for the device
names to stay in 'sync' between domain and snapshot XML dumps.
Primer the code for further changes:
* move variable declarations to the top of the function
* group together free/release statements
* error check and report VBOX API calls used
If a VBOX VM has e.g. a SATA and SCSI disk attached, the XML generated
by dumpxml used to produce "sda" for both of those disks. This is an
invalid domain XML as libvirt does not allow duplicate device names. To
address this, keep the running total of disks that will use "sd" prefix
for device name and pass it to the vboxGenerateMediumName which no
longer tries to "compute" the value based only on current and max
port and slot values. After this the vboxGetMaxPortSlotValues is not
needed and was deleted.
Both vboxSnapshotGetReadWriteDisks and vboxSnapshotGetReadWriteDisks do
not need to free the def->disks on cleanup because it's being done by
the caller via virDomainSnaphotDefFree
This patch prepares the vboxSnapshotGetReadOnlyDisks and
vboxSnapshotGetReadWriteDisks functions for further changes so that
the code movement does not obstruct the gist of those future changes.
This is done primarily because we'll need to know the type of vbox
storage controller as early as possible and make decisions based on
that info.
With this patch, the vbox driver will no longer attach all supported
storage controllers by default even if no disk devices are associated
with them. Instead, it will attach only those that are implicitly added
by virDomainDefAddImplicitController based on <disk> element or if
explicitly specified via the <controller> element.
Since the VBOX API requires to register an initial VM before proceeding
to attach any remaining devices to it, any failure to attach such
devices should result in automatic cleanup of the initially registered
VM so that the state of VBOX registry remains clean without any leftover
"aborted" VMs in it. Failure to cleanup of such partial VMs results in a
warning log so that actual define error stays on the top of the error
stack.
Libvirt historically stores storage source path including the volume as
one string in the XML, but that is not really flexible enough when
dealing with the fields in the code. Previously we'd store the slash
separating the two as part of the image name. This was fine for gluster
but it's not necessary and does not scale well when converting other
protocols.
Don't store the slash as part of the path. The resulting change from
absolute to relative path within the gluster driver should be okay,
as the root directory is the default when accessing gluster.
Extract the part formatting the basic URI part so that it can be reused
to format JSON backing definitions. Parts specific to the command line
format will remain in qemuBuildNetworkDriveURI. The new function is
called qemuBlockStorageSourceGetURI.
Original implementation used 'SocketAddress' equivalent from qemu for
the disk server field, while qemu documentation specifies
'InetSocketAddress'. The backing store parser uses the correct parsing
function but the formatter used the incorrect one (and also with the
legacy mode enabled which was wrong).
To allow merging this with other disk type checks we need to check
qemuCaps only when available, since some of the checks are executed on
disk cold-plug and thus capabilities should not be checked.
Make the checks optional by making them conditional on qemuCaps not
being NULL.
All of the error message are already in a conditional block with known
bus type. Inline the bus type rather than formatting it from a separate
variable.
The disk index validation is used only in very specific cases and does
not need to be performed otherwise. Move it out of the global check into
the usage place.
busid and unitid are ever used only if the device is an SD card due to
the check in qemuDiskBusNeedsDeviceArg. Since the SD card does not have
an bus or unit number, most of the code and command line formatter can
be removed since it will never be used.
In near future we will need more than just a plain VIR_STRDUP().
Better implement that in a separate function and in
qemuBuildMemoryBackendStr() which is complicated enough already.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function works over domain definition and not domain object.
Its name is thus misleading.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
After the virNetDaemonAddServerPostExec call in virtlogd we should have
netserver refcount set to 2. One goes to netdaemon servers hashtable
and one goes to virt{logd,lock} own reference to netserver. Let's add
the missing increment in virNetDaemonAddServerPostExec itself while
holding the daemon lock.
Since lockd defers management of the @srv object by the presence
in the hash table, virLockDaemonNewPostExecRestart must Unref the
alloc'd Ref on the @srv object done as part of virNetDaemonAddServerPostExec
and virNetServerNewPostExecRestart processing. The virNetDaemonGetServer
in lock_daemon main will also take a reference which is Unref'd during
main cleanup.
Commit id '252610f7d' used a hash table to store the @srv, but
didn't handle the virObjectUnref if virNetDaemonNew failed nor
did it use virObjectUnref once successfully placed into the table
which will now be managing it's lifetime (and would cause the
virObjectRef if successfully inserted into the table).
When coverage build is enabled, gcc complains about it:
In file included from qemu/qemu_agent.h:29:0,
from qemu/qemu_driver.c:47:
qemu/qemu_driver.c: In function 'qemuDomainSetInterfaceParameters':
./conf/domain_conf.h:3397:1: error: inlining failed in call to
'virDomainNetTypeSharesHostView': call is unlikely and code size would
grow [-Werror=inline]
virDomainNetTypeSharesHostView(const virDomainNetDef *net)
^
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This patch exposes additional methods of the native VBOX API to the
libvirt 'unified' vbox API to deal with IStorageController. The exposed
methods are:
* IStorageController->GetStorageControllerType()
* IStorageController->SetStorageControllerType()
* IMachine->GetStorageControllers()
Original code was checking for non empty disk source before proceeding
to actually attach disk device to VM. This prevented from creating
empty removable devices like DVD or floppy. Therefore, this patch
re-organizes the loop work-flow to allow such configurations as well as
makes the code follow better libvirt practices. Additionally, adjusted
debug logs to be more helpful - removed old ones and added new which
give more valuable info for troubleshooting.
Previously, if one tried to define a VBOX VM and the API failed to
perform the requested actions for some reason, it would just log the
error and move on to process remaining disk definitions. This is not
desired as it could result in incorrectly defined VM without the caller
even knowing about it. So now all the code paths that call
virReportError are now treated as hard failures as they should have
been.
Remove the setting since it's unused as of commit 34364df3 which should
have never copied it in from the old code which ended up getting removed
as part of commit c7c286c6.
This commit primes vboxAttachDrives for further changes so when they
are made, the diff is less noisy:
* move variable declarations to the top of the function
* add disk variable to replace all the def->disks[i] instances
* add cleanup at the end of the loop body, so it's all in one place
rather than scattered through the loop body. It's purposefully
called 'cleanup' rather than 'skip' or 'continue' because future
commit will treat errors as hard-failures.
Previously, the driver was computing VBOX's devicePort/deviceSlot values
based on device name and max port/slot values. While this worked, it
completely ignored <address> values. Additionally, libvirt's built-in
virDomainDiskDefAssignAddress already does a good job setting default
values on virDomainDeviceDriveAddress struct which we can use to set
devicePort and deviceSlot and accomplish the same result while allowing
the customizing those via XML. Also, this allows to remove some code
which will make further patches smaller.
When registering a VM we call OpenMedium on each disk image which adds it
to vbox's global media registry. Therefore, we should make sure to call
Close when unregistering VM so we cleanup the media registry entries
after ourselves - this does not remove disk image files. This follows
the behaviour of the VBoxManage unregistervm command.
