ppc64 NVDIMM support was implemented in QEMU by commit [1].
The support is similar to what x86 already does, aside from
an extra 'uuid' element.
This patch introduces a new optional 'uuid' element for the
NVDIMM memory model. This element behaves like the 'uuid'
element of the domain definition - if absent, we'll create
a new one, otherwise use the one provided by the XML.
The 'uuid' element is exclusive to pseries guests and are
unavailable for other architectures.
Next patch will use this new element to add NVDIMM support
for ppc64.
[1] ee3a71e366
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use existing function built for this exact purpose.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
So far, when using the qemu:///embed driver, management
applications can't chose whether they want to register their
domains in machined or not. While having that option is certainly
desired, it will require more work. What we can do meanwhile is
to generate names that include part of hash of the root
directory. This is to ensure that if two applications using
different roots but the same domain name (and ID) start the
domain no clashing name for machined is generated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
During startup the udev node device driver impl uses a background thread
to populate the list of devices to avoid blocking the daemon startup
entirely. There is no synchronization to the public APIs, so it is
possible for an application to start calling APIs before the device
initialization is complete.
This was not a problem in the old approach where libvirtd was started
on boot, as initialization would easily complete before any APIs were
called.
With the use of socket activation, however, APIs are invoked from the
very moment the daemon starts. This is easily seen by doing a
'virsh -c nodedev:///system list'
the first time it runs it will only show one or two devices. The second
time it runs it will show all devices. The solution is to introduce a
flag and condition variable for APIs to synchronize against before
returning any data.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Some disk backends support configuring the readahead buffer or timeout
for requests. Add the knobs to the XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add possibility to specify one or more cookies for http based disks.
This patch adds the config parser, storage and validation of the
cookies.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To allow turning off verification of SSL cerificates add a new element
<ssl> to the disk source XML which will allow configuring the validation
process using the 'verify' attribute.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
According to the linked BZ, machined expects either valid
hostname or valid FQDN (see systemd commit
v239-3092-gd65652f1f2). While in case of multiple dots, a
trailing one doesn't violate FQDN, it does violate the rule in
case of something simple, like "domain.". But it's safe to remove
it in both cases.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808499
Fixes: 45464db8ba
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When rewriting the virDomainDiskTranslateSourcePool() function in
v6.1.0-rc1~184 a typo was introduced. Previously, we allowed
startup policy only for those volumes which translated to
VIR_STORAGE_TYPE_FILE. But starting with the referenced commit,
the value we checked for was changed to VIR_STORAGE_VOL_FILE
which comes from a different enum and has a different value too.
This is wrong, because virStorageSourceGetActualType() returns a
value from the original enum.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1811728
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use 'g_autofree' to clean both 'path' and 'xml' which mandates
initialization and get rid of the 'cleanup' label and 'ret variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add more elements for tuning the virtiofsd daemon
and the vhost-user-fs device:
<driver type='virtiofs' queue='1024' xattr='on'>
<binary path='/usr/libexec/virtiofsd'>
<cache mode='always'/>
<lock posix='off' flock='off'/>
</binary>
</driver>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Introduce a new 'virtiofs' driver type for filesystem.
<filesystem type='mount' accessmode='passthrough'>
<driver type='virtiofs'/>
<source dir='/path'/>
<target dir='mount_tag'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</filesystem>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Now that this file no longer transitively includes
domain_conf.h, it can be included here.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make the header easier to read and let the compiler inline
what it wants.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is pulled in via domain_conf.h somehow, but it is directly used.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The ParseNode function takes arguments with types
from libxml.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
After the split of enum functions into virenum.h,
this function does not contain anything worth including
in another header file.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Include virutil.h in all files that use it,
instead of relying on it being pulled in somehow.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Currently they live in util/virhostdev.
However the virhostdev module is wrongly placed
in util, which is below conf/ in our hierarchy.
Move the functions that are actually used in conf/
to conf/ and remove the include of virhostdev.h
from domain_conf.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The persistent alias name @persistent is allocated in
virDomainNetDefParseXML() but never freed.
==119642== 22 bytes in 2 blocks are definitely lost in loss record 178 of 671
==119642== at 0x483579F: malloc (vg_replace_malloc.c:309)
==119642== by 0x58F89F1: xmlStrndup (in /usr/lib64/libxml2.so.2.9.9)
==119642== by 0x4BA3B74: virXMLPropString (virxml.c:520)
==119642== by 0x4BDB0C5: virDomainNetDefParseXML (domain_conf.c:11876)
==119642== by 0x4BF9EF4: virDomainDefParseXML (domain_conf.c:21196)
==119642== by 0x4BFCD5B: virDomainDefParseNode (domain_conf.c:21943)
==119642== by 0x4BFCC36: virDomainDefParse (domain_conf.c:21901)
==119642== by 0x4BFCCCB: virDomainDefParseFile (domain_conf.c:21924)
==119642== by 0x114A9D: testCompareXMLToArgv (qemuxml2argvtest.c:452)
==119642== by 0x13894F: virTestRun (testutils.c:143)
==119642== by 0x11F46E: mymain (qemuxml2argvtest.c:1316)
==119642== by 0x13A60E: virTestMain (testutils.c:839
Fixes: fb0509d06a
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The privateData object is allocated in virDomainFSDefNew() but
never unref'd.
==119642== 480 bytes in 20 blocks are definitely lost in loss record 656 of 671
==119642== at 0x4837B86: calloc (vg_replace_malloc.c:762)
==119642== by 0x57806A0: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.7)
==119642== by 0x4AE7392: virAllocVar (viralloc.c:331)
==119642== by 0x4B64395: virObjectNew (virobject.c:241)
==119642== by 0x48F1464: qemuDomainFSPrivateNew (qemu_domain.c:1427)
==119642== by 0x4BBF004: virDomainFSDefNew (domain_conf.c:2307)
==119642== by 0x4BD859A: virDomainFSDefParseXML (domain_conf.c:11217)
==119642== by 0x4BF9DD1: virDomainDefParseXML (domain_conf.c:21179)
==119642== by 0x4BFCD5B: virDomainDefParseNode (domain_conf.c:21943)
==119642== by 0x4BFCC36: virDomainDefParse (domain_conf.c:21901)
==119642== by 0x4BFCCCB: virDomainDefParseFile (domain_conf.c:21924)
==119642== by 0x114A9D: testCompareXMLToArgv (qemuxml2argvtest.c:452)
Fixes: 5120577ed7
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
During the hypervisor-agnostic validation of network devices, verify
that the interface type is either "network" or "bridge", and that if
there is any <virtualport>, that it doesn't have any type associated
with it.
This needs to be done both for the parse-time validation and for
runtime validation (after a port has been acquired from any associated
network), because an interface with type='network' could have an
actual type at runtime of "hostdev" or "direct", neither of which
support isolated='true' (yet). Likewise, if an interface is
type='network', then at runtime a <virtualport> with a type that
doesn't support isolated='yes' (e.g. "openvswitch", "802.1Qbh" -
currently *none* of the available virtualport types support it)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch pushes the isolatedPort setting from the <interface> down
all the way to the callers of virNetDevBridgeAddPort(), and sets
BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after
the port has been successfully added to the bridge.
Signed-off-by: Laine Stump <laine@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similar to the way that the <vlan>, <bandwidth>, and <virtualport>
elements and the trustGuestRxFilters attribute in a <network> (or in
the appropriate <portgroup> element of a <network> can be applied to a
port when it is allocated for a domain's network interface, this patch
checks for a configured value of <port isolated="yes|no"/> in
either the domain <interface> or in the network, setting isolatedPort
in the <networkport> to the first one it finds (the setting from the
domain's <interface> is preferred). This, in turn, is passed back to
the domain when a port is allocated, so that the domain will use that
setting.
(One difference from <vlan>, <bandwidth>, <virtualport>, and
trustGuestRxFilters, is that all of those can be set in a <portgroup>
so that they can be applied only to a subset of interfaces connected
to the network. This didn't really make sense for the isolated setting
due to the way that it's implemented in Linux - the BR_ISOLATED flag
will prevent traffic from passing between two ports that both have
BR_ISOLATED set, but traffic can still go between those ports and
other ports that *don't* have BR_ISOLATED. (It would be nice if all
traffic from a BR_ISOLATED port could be blocked except traffic going
to/from a designated egress port or ports, but instead the entire
feature is implemented as a single flag. Because of this, it's really
only useful if all the ports on a network are isolated, so setting it
for a subset has no practical utility.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is a very simple thing to parse and format, but needs to be done
in 4 places, so two trivial utility functions have been made that can
be called from all the higher level parser/formatters:
<domain><interface>
<domain><interface><actual> (only in domain status)
<network>
<networkport>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In commit v5.9.0-400-gaf8e39921a I removed printing model's fallback and
vendor_id attributes when no model is specified. However, vendor_id
makes sense even without a specific CPU model (for host-model CPUs).
https://bugzilla.redhat.com/show_bug.cgi?id=1804549
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Even if an interface of type 'network', setting 'floor' is only supported
if the network's forward type is nat, route, open or none.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This compound condition will be useful in several places so it
makes sense to give it a name for better readability.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This new timer model will be used to control the behavior of the
virtual timer for KVM ARM/virt guests.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we accept full backing chains on input nothing should prevent
users from also using disk type 'VOLUME' for specifying the backing
images.
Do the translation for the whole backing chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract all the code setting up one storage source from the rest which
sets up the whole disk. This will allow us to prepare the whole backing
chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virStringSplitCount instead of virStringSplit so that we can drop
the call to virStringListLength and use VIR_AUTOSTRINGLIST to declare
it and allow removal of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch adds support for the tpm-spapr device model for ppc64. The XML for
this type of TPM looks as follows:
<tpm model='tpm-spapr'>
<backend type='emulator'/>
</tpm>
Extend the documentation.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Introduce VIR_DOMAIN_TPM_MODEL_DEFAULT as a default model which we use
in case the user does not provide a model in the device XML. It has
the TIS's previous value of '0'. In the post parsing function
we change this default value to 'TIS' to have the same model as before.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Technically, there is no memleak here, since the only
allocations are filled by virDomainDeviceInfoParseXML,
which cleans up after itself.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simple g_autofree is not enough if we put allocated
data into the device structure.
Define the AUTOPTR_CLEANUP function and use it here.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Xu Yandong <xuyandong2@huawei.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
All our supported Linux distros now have this header.
It has never existed on FreeBSD / macOS / Mingw.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Tweak the return value expectation comment so that it doesn't
necessarily require to allocate memory and refactor the implementations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Always trim the full specified suffix.
All of the callers outside of tests were passing either
strlen or the actual length of the string.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Previous patch used 'g_autofree' to eliminate instances of
VIR_FREE(), making some cleanup labels obsolete. This
patch removes them.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree in strings when possible to spare a VIR_FREE()
call. Unneeded 'cleanup' labels will be taken care of in the
next patch.
The 'str' string in virDomainVirtioSerialAddrReserve() was
never used by the logic, only being used in cleanup by
VIR_FREE(). Let's remove it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are two calls to virHashNew which check the return value. It's not
necessary any more as virHashNew always returns a valid pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This module has last two direct checks whether the value returned by
virHashCreateFull is NULL. Remove them so that static analyzers don't
get the false idea that checking the value is necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The subelement <teaming> of <interface> devices is used to configure a
simple teaming association between two interfaces in a domain. Example:
<interface type='bridge'>
<source bridge='br0'/>
<model type='virtio'/>
<mac address='00:11:22:33:44:55'/>
<alias name='ua-backup0'/>
<teaming type='persistent'/>
</interface>
<interface type='hostdev'>
<source>
<address type='pci' bus='0x02' slot='0x10' function='0x4'/>
</source>
<mac address='00:11:22:33:44:55'/>
<teaming type='transient' persistent='ua-backup0'/>
</interface>
The interface with <teaming type='persistent'/> is assumed to always
be present, while the interface with type='transient' may be be
unplugged and later re-plugged; the persistent='blah' attribute (and
in the one currently available implementation, also the matching MAC
addresses) is what associates the two devices with each other. It is
up to the hypervisor and the guest network drivers to determine what
to do with this information.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are a large number of different header files that
are related to the sockets APIs. The virsocket.h header
includes all of the relevant headers for Windows and UNIX
in one convenient place. If virsocketaddr.h is already
included, then there's no need for virsocket.h
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently it is possible to start a domain which have disks
in same iotune group and at the same time having different iotune
params. Both params set are passed to qemu in command line and the one
that is passed later down command line is get actually set.
Let's prohibit such configurations.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, if only iotune group name is given for some disk and
no any params then later start of domain will fail. I guess it
will be convenient to allow such configuration if there is
another disk in the same iotune group with iotune params set. The
meaning is that the first disk have same iotunes and the latter.
Thus one can easily add a disk to iotune group - just add group
name parameter and no need to copy all the params.
Also let's expand iotunes params in the described case so we don't
need to refer to another disk to know iotunes and this will make
logic in many places simple.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
And introduce virDomainBlockIoTuneInfoHasAny.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some *ParseXML functions have comments stating what kind of device
they parse with an outdated list of parameters, with the exception
of virDomainFSDefParseXML which claims to parse a disk.
Remove them, assuming the function names are descriptive enough.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the virXMLFormatElement helper to format the driver element
to simplify adding further sub-elements.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The 'builtin' rng backend model can be used as following:
<rng model='virtio'>
<backend model='builtin'/>
</rng>
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If we get a user reporting this error message being shown it's pretty
useless in terms of actually debugging it since we don't know which hash
and which key are actually subject to the error.
This patch adds a new hash table callback which formats the
user-readable version of the hash key and reports it in the new message
which will look like:
"Duplicate hash table key 'blah'"
That way we will at least have an anchor point where to start the
search.
There are two special implementations of keys which are numeric so we
add specific printer functions for them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If users wish to use different name for exported disks or bitmaps
the new fields allow to do so. Additionally they also document the
current settings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
QEMU driver has two functions: qemuGetDHCPInterfaces() and
qemuARPGetInterfaces() that are being used inside only one single
function. They can be turned into generic functions that other drivers
can use. This commit move both from QEMU driver tree to domain conf
tree.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simplify function logic by using g_autofree to free local variables so
that we can remove some goto statements that are used for cleanup.
Introduce a g_autoptr cleanup function for virNodeDeviceDef.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
gmtime_r/localtime_r are mostly used in combination with
strftime to format timestamps in libvirt. This can all
be replaced with GDateTime resulting in simpler code
that is also more portable.
There is some boundary condition problem in parsing POSIX
timezone offsets in GLib which tickles our test suite.
The test suite is hacked to avoid the problem. The upsteam
GLib bug report is
https://gitlab.gnome.org/GNOME/glib/issues/1999
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
G_STATIC_ASSERT() is a drop-in functional equivalent of
the GNULIB verify() macro.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't need all the platforms gnulib deals with, so
this is a cut down version of GNULIB's physmem.c
code. This also allows us to integrate libvirt's
error reporting functions closer to the error cause.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Update the host CPU code to report the die_id in the NUMA topology
capabilities. On systems with multiple dies, this fixes the bug
where CPU cores can't be distinguished:
<cpus num='12'>
<cpu id='0' socket_id='0' core_id='0' siblings='0'/>
<cpu id='1' socket_id='0' core_id='1' siblings='1'/>
<cpu id='2' socket_id='0' core_id='0' siblings='2'/>
<cpu id='3' socket_id='0' core_id='1' siblings='3'/>
</cpus>
Notice how core_id is repeated within the scope of the same socket_id.
It now reports
<cpus num='12'>
<cpu id='0' socket_id='0' die_id='0' core_id='0' siblings='0'/>
<cpu id='1' socket_id='0' die_id='0' core_id='1' siblings='1'/>
<cpu id='2' socket_id='0' die_id='1' core_id='0' siblings='2'/>
<cpu id='3' socket_id='0' die_id='1' core_id='1' siblings='3'/>
</cpus>
So core_id is now unique within a (socket_id, die_id) pair.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Recently CPU hardware vendors have started to support a new structure
inside the CPU package topology known as a "die". Thus the hierarchy
is now:
sockets > dies > cores > threads
This adds support for "dies" in the XML parser, with the value
defaulting to 1 if not specified for backwards compatibility.
For example a system with 64 logical CPUs might report
<topology sockets="4" dies="2" cores="4" threads="2"/>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is only a theoretical leak, but in virChrdevAlloc() we
initialize a mutex and if creating a hash table fails,
then virChrdevFree() is called which because of incorrect check
doesn't deinit the mutex.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When opening a console to a domain, we put a tuple of {path,
virStreamPtr} into a hash table that's private to the domain.
This is to ensure only one client at most has the console stream
open. Later, when the console is closed, the tuple is removed
from the hash table and freed. Except, @path won't be freed.
==234102== 60 bytes in 5 blocks are definitely lost in loss record 436 of 651
==234102== at 0x4836753: malloc (vg_replace_malloc.c:307)
==234102== by 0x5549110: g_malloc (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102== by 0x5562D1E: g_strdup (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102== by 0x4A5A917: virChrdevOpen (virchrdev.c:412)
==234102== by 0x17B64645: qemuDomainOpenConsole (qemu_driver.c:17309)
==234102== by 0x4BC8031: virDomainOpenConsole (libvirt-domain.c:9662)
==234102== by 0x13F854: remoteDispatchDomainOpenConsole (remote_daemon_dispatch_stubs.h:9211)
==234102== by 0x13F72F: remoteDispatchDomainOpenConsoleHelper (remote_daemon_dispatch_stubs.h:9178)
==234102== by 0x4AB0685: virNetServerProgramDispatchCall (virnetserverprogram.c:430)
==234102== by 0x4AB01F0: virNetServerProgramDispatch (virnetserverprogram.c:302)
==234102== by 0x4AB700B: virNetServerProcessMsg (virnetserver.c:136)
==234102== by 0x4AB70CB: virNetServerHandleJob (virnetserver.c:153)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
As of systemd commit:
commit d65652f1f21a4b0c59711320f34266c635393c89
Author: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
CommitDate: 2018-12-10 09:56:56 +0100
Partially unify hostname_is_valid() and dns_name_is_valid()
Dashes are no longer allowed at the end of machine names.
Trim the trailing dashes from the generated name before passing
it to machined.
https://bugzilla.redhat.com/show_bug.cgi?id=1790409
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When trying to specify an input device on s390x without bus like this:
<input type='keyboard'/>
... then libvirt currently complains:
error: unsupported configuration: USB is disabled for this domain,
but USB devices are present in the domain XML
This is somewhat confusing since the user did not specify an USB
device here. Since USB is not available on s390x, we should default
to the "virtio" bus here instead.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1790189
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically there are two places where we format authentication and
encryption for a disk. The logich which formats it for backing files was
flawed though and didn't format it at all. This worked if the image
became a backing file through the means of a snapshot but not directly.
Force formatting of the source and encryption for any non-disk case to
fix the issue.
This caused problems in many places as we use the formatter to copy the
definition. Effectively any copy lost the secret definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1789310https://bugzilla.redhat.com/show_bug.cgi?id=1788898
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The point of this function is to translate virDomainOsDefFirmware
enum to qemuFirmwareOSInterface enum. However, with my commit
v5.10.0-507-g8e1804f9f6 we are passing a variable type of
virDomainLoader enum. Make the function accept both enums and
make the enum members correspond to each other.
This fixes clang build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add an object to hold the private data and call the
allocation function if it's present in xmlopt.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This will be needed in the future for allocating private data.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
These functions are meant to replace verbose check for the old
style of specifying UEFI with a simple function call.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The g_date_time_new_from_iso8601() function was introduced as
a replacement for strptime in
commit 810613a60e
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon Dec 23 15:37:26 2019 +0000
src: replace strptime()/timegm()/mktime() with GDateTime APIs set
Unfortunately g_date_time_new_from_iso8601 isn't available until
glib 2.56, and backporting it requires alot of code copying and
poking at private glib structs.
This reverts domain_conf.c back to its original parsing logic prior
to 810613a60e, but using g_date_time_new()
instead of gmtime(). The other files are then adapted to follow a
similar approach.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All places where we use strptime/timegm()/mktime() are handling
conversion of dates in a format compatible with ISO 8601, so we
can use the GDateTime APIs to simplify code.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The last_component() method is a GNULIB custom function
that returns a pointer to the base name in the path.
This is similar to g_path_get_basename() but without the
malloc. The extra malloc is no trouble for libvirt's
needs so we can use g_path_get_basename().
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
g_get_real_time() returns the time since epoch in microseconds.
It uses gettimeofday() internally while libvirt used clock_gettime
because it is declared async signal safe. In practice gettimeofday
is also async signal safe *provided* the timezone parameter is
NULL. This is indeed the case in g_get_real_time().
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
It will be used to represent the type of a filesystem pool in ESXi.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Recent changes removed the virCapsPtr, but didn't adjust/remove the
corresponding ATTRIBUTE_NONNULL resulting in a build failure to build
in my Coverity environment.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch introduces a new PCI hostdev address type called
'unassigned'. This new type gives users the option to add
PCI hostdevs to the domain XML in an 'unassigned' state, meaning
that the device exists in the domain, is managed by Libvirt
like any regular PCI hostdev, but the guest does not have
access to it.
This adds extra options for managing PCI device binding
inside Libvirt, for example, making all the managed PCI hostdevs
declared in the domain XML to be detached from the host and bind
to the chosen driver and, at the same time, allowing just a
subset of these devices to be usable by the guest.
Next patch will use this new address type in the QEMU driver to
avoid adding unassigned devices to the QEMU launch command line.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
If the host OS doesn't have NUMA present, we fallback to
populating fake NUMA info and the code thus assumes only a
single NUMA node.
Unfortunately we also fallback to fake NUMA if numactl-devel
was not present, and in this case we can still have multiple
NUMA nodes. In this case we create all CPUs, but only the
CPUs in the first node have any data filled in, resulting in
capabilities like:
<topology>
<cells num='1'>
<cell id='0'>
<memory unit='KiB'>15977572</memory>
<cpus num='48'>
<cpu id='0' socket_id='0' core_id='0' siblings='0'/>
<cpu id='1' socket_id='0' core_id='0' siblings='1'/>
<cpu id='2' socket_id='0' core_id='1' siblings='2'/>
<cpu id='3' socket_id='0' core_id='1' siblings='3'/>
<cpu id='4' socket_id='0' core_id='2' siblings='4'/>
<cpu id='5' socket_id='0' core_id='2' siblings='5'/>
<cpu id='6' socket_id='0' core_id='3' siblings='6'/>
<cpu id='7' socket_id='0' core_id='3' siblings='7'/>
<cpu id='8' socket_id='0' core_id='4' siblings='8'/>
<cpu id='9' socket_id='0' core_id='4' siblings='9'/>
<cpu id='10' socket_id='0' core_id='5' siblings='10'/>
<cpu id='11' socket_id='0' core_id='5' siblings='11'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
<cpu id='0'/>
</cpus>
</cell>
</cells>
</topology>
With this new code we get something slightly less broken
<topology>
<cells num='4'>
<cell id='0'>
<memory unit='KiB'>15977572</memory>
<cpus num='12'>
<cpu id='0' socket_id='0' core_id='0' siblings='0-1'/>
<cpu id='1' socket_id='0' core_id='0' siblings='0-1'/>
<cpu id='2' socket_id='0' core_id='1' siblings='2-3'/>
<cpu id='3' socket_id='0' core_id='1' siblings='2-3'/>
<cpu id='4' socket_id='0' core_id='2' siblings='4-5'/>
<cpu id='5' socket_id='0' core_id='2' siblings='4-5'/>
<cpu id='6' socket_id='0' core_id='3' siblings='6-7'/>
<cpu id='7' socket_id='0' core_id='3' siblings='6-7'/>
<cpu id='8' socket_id='0' core_id='4' siblings='8-9'/>
<cpu id='9' socket_id='0' core_id='4' siblings='8-9'/>
<cpu id='10' socket_id='0' core_id='5' siblings='10-11'/>
<cpu id='11' socket_id='0' core_id='5' siblings='10-11'/>
</cpus>
</cell>
<cell id='0'>
<memory unit='KiB'>15977572</memory>
<cpus num='12'>
<cpu id='12' socket_id='0' core_id='0' siblings='12-13'/>
<cpu id='13' socket_id='0' core_id='0' siblings='12-13'/>
<cpu id='14' socket_id='0' core_id='1' siblings='14-15'/>
<cpu id='15' socket_id='0' core_id='1' siblings='14-15'/>
<cpu id='16' socket_id='0' core_id='2' siblings='16-17'/>
<cpu id='17' socket_id='0' core_id='2' siblings='16-17'/>
<cpu id='18' socket_id='0' core_id='3' siblings='18-19'/>
<cpu id='19' socket_id='0' core_id='3' siblings='18-19'/>
<cpu id='20' socket_id='0' core_id='4' siblings='20-21'/>
<cpu id='21' socket_id='0' core_id='4' siblings='20-21'/>
<cpu id='22' socket_id='0' core_id='5' siblings='22-23'/>
<cpu id='23' socket_id='0' core_id='5' siblings='22-23'/>
</cpus>
</cell>
</cells>
</topology>
The topology at least now reflects what 'virsh nodeinfo' reports.
The main bug is that the CPU "id" values won't match what the Linux
host actually uses.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The 'caps' object is already allocated when the fake NUMA
initialization takes place.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Fortunately, this is not causing any problems now because glib
does this check for us when calling this function via attribute
cleanup. But in a future commit we will explicitly call this
function over a struct member that might be NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This function will return true if any of disks (or their backing
chain) for given domain contains an NVMe disk.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
To simplify implementation, some restrictions are added. For
instance, an NVMe disk can't go to any bus but virtio and has to
be type of 'disk' and can't have startupPolicy set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Next patch will validate QEMU_CAPS_NUMA_DIST in a new qemu_domain.c
function. The code to verify if a NUMA node distance is being
set will still be needed in qemuBuildNumaArgStr() though.
To avoid code repetition, let's put this logic in a helper to be
used in qemuBuildNumaArgStr() and in the new function.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Following domain configuration changes create two memory bandwidth
monitors: one is monitoring the bandwidth consumed by vCPU 0,
another is for vCPU 5.
```
<cputune>
<memorytune vcpus='0-4'>
<node id='0' bandwidth='20'/>
<node id='1' bandwidth='30'/>
+ <monitor vcpus='0'/>
</memorytune>
+ <memorytune vcpus='5'>
+ <monitor vcpus='5'/>
+ </memorytune>
</cputune>
```
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Huaqiang <huaqiang.wang@intel.com>
We learned that the hardware features of CAT, CMT, MBA and MBM
are orthogonal ones, if CAT or MBA is not supported in system,
but CMT or MBM are supported, then the cache monitor or
memoryBW monitor features may not be correctly displayed in
host capabilities through command 'virsh capabilites'.
Showing the cache/memoryBW monitor capabilities even there is
no support of cache allocation or memoryBW allocation features.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Huaqiang <huaqiang.wang@intel.com>
Replace all the uses passing a single parameter as the length.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
My hesitation to remove VIR_STRDUP without VIR_STRNDUP resulted
in these being able to sneak in.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This flag will allow figuring out whether the hypervisor supports the
incremental backup and checkpoint features.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need a place to store stats of completed sub-jobs so that we can
later report accurate stats.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A backup job may consist of many backup sub-blockjobs. Add the new
blockjob type and add all type converter strings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Accept XML describing a generic block job, and output it again as
needed. This may still need a few tweaks to match the documented XML
and RNG schema.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Annoyingly there was no existing constructor, and identifying all the
places which do a VIR_ALLOC(cpu) is a bit error prone. Hopefully this
has found & converted them all.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The NUMA cells are stored directly in the virCapsHostPtr
struct. This moves them into their own struct allowing
them to be stored independantly of the rest of the host
capabilities. The change is used as an excuse to switch
the representation to use a GPtrArray too.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This parameter is now unused and can be removed entirely.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
None of the impls of this callback require the virCapsPtr param.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
None of the impls of this callback require the virCapsPtr param.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
No impl of this callback requires the virCapsPtr anymore.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The only user of this callback did not require the virCapsPtr parameter.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU impl of the callback can directly use the QEMU capabilities
cache to resolve the emulator binary name, allowing virCapsPtr to be
dropped.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virCapsPtr param is not used by any of the virt drivers providing
this callback.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of using the virCapsPtr to get the default security model,
pass this in via the parser config.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the disk and chardev seclabels are validated immediately at
the time their data is parsed. This forces the parser to fill in the
top level secmodel at time of parsing which is an undesirable thing.
This validation conceptually should be done in the post-parse phase
instead.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of using the virCapsPtr information, pass the driver specific
netprefix in the domain parser struct. This eliminates one more use of
virCapsPtr from the XML parsing/formatting code.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The XML parser currently calls virCapabilitiesDomainDataLookup during
parsing to find the domain capabilities matching the triple
(virt type, os type, arch)
This is, however, bogus with the QEMU driver as it assumes that there
is an emulator known to the default driver capabilities that matches
this triple. It is entirely possible for the driver to be parsing an
XML file with a custom emulator path specified pointing to a binary
that doesn't exist in the default driver capabilities. This will,
for example be the case on a RHEL host which only installs the host
native emulator to /usr/bin. The user can have built a custom QEMU
for non-native arches into $HOME and wish to use that.
Aside from validation, this call is also used to fill in a machine type
for the guest if not otherwise specified. Again, this data may be
incorrect for the QEMU driver because it is not taking account of
the emulator binary that is referenced.
To start fixing this, move the validation to the post-parse callbacks
where more intelligent driver specific logic can be applied.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When parsing the guest XML we must fill in the default guest arch if it
is not already present because later parts of the parsing process need
this information.
If no arch is specified we lookup the first guest in the capabilities
data matching the os type and virt type. In most cases this will result
in picking the host architecture but there are some exceptions...
- The test driver is hardcoded to always use i686 arch
- The VMWare/ESX drivers will always place i686 guests ahead
of x86_64 guests in capabilities, so effectively they always
use i686
- The QEMU driver can potentially return any arch at all
depending on what combination of QEMU binaries are installed.
The domain XML hardware configurations are inherently architecture
specific in many places. As a result whomever/whatever created the
domain XML will have had a particular architecture in mind when
specifying the config. In pretty much any sensible case this arch
will have been the native host architecture. i686 on x86_64 is
the only sensible divergance because both these archs are
compatible from a domaain XML config POV.
IOW, although the QEMU driver can pick an almost arbitrary arch as its
default, in the real world no application or user is likely to be
relying on this default arch being anything other than native.
With all this in mind, it is reasonable to change the XML parser to
allow the default architecture to be passed via the domain XML options
struct. If no info is explicitly given then it is safe & sane to pick
the host native architecture as the default for the guest.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Moving their instance parameter to be the first one, and give consistent
ordering of other parameters across all functions. Ensure that the xml
options are passed into both functions in prep for future work.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Our normal practice is for the object type to be the name prefix, and
the object instance be the first parameter passed in.
Rename these to virDomainObjSave and virDomainDefSave moving their
primary parameter to be the first one. Ensure that the xml options
are passed into both functions in prep for future work.
Finally enforce checking of the return type and mark all parameters
as non-NULL.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With this patch users can cold unplug some sound devices.
use "virsh detach-device vm sound.xml --config" command.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jidong Xia <xiajidong@cmss.chinamobile.com>
Introduced in c8007fdc5d, it should use 'greater than max' instead of
'equal or greater than max' for the condition of checking invalid scsi
unit.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In the past the network driver was (mistakenly) being called for all
interfaces, not just those of type='network', and so it had a chance
to validate all interface configs after the actual type of the
interface was known.
But since the network driver has been more completely/properly
separated from qemu, the network driver isn't called during the
startup of any interfaces except those with type='network', so this
validation no longer takes place for, e.g. <interface type='bridge'>
(or direct, etc). This in turn meant that a config could erroneously
specify a vlan tag, or bandwidth settings, for a type of interface
that didn't support it, and the domain would start without complaint,
just silently ignoring those settings.
This patch moves those validation checks out of the network driver,
and into virDomainActualNetDefValidate() so they will be done for all
interfaces, not just type='network'.
https://bugzilla.redhat.com/1741121
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
<interface> devices (virDomainNetDef) are a bit different from other
types of devices in that their actual type may come from a network (in
the form of a port connection), and that doesn't happen until the
domain is started. This means that any validation of an <interface> at
parse time needs to be a bit liberal in what it accepts - when
type='network', you could think that something is/isn't allowed, but
once the domain is started and a port is created by the configured
network, the opposite might be true.
To solve this problem hypervisor drivers need to do an extra
validation step when the domain is being started. I recently (commit
3cff23f7, libvirt 5.7.0) added a function to peform such validation
for all interfaces to the QEMU driver -
qemuDomainValidateActualNetDef() - but while that function is a good
single point to call for the multiple places that need to "start" an
interface (domain startup, device hotplug, device update), it can't be
called by the other hypervisor drivers, since 1) it's in the QEMU
driver, and 2) it contains some checks specific to QEMU. For
validation that applies to network devices on *all* hypervisors, we
need yet another interface validation function that can be called by
any hypervisor driver (not just QEMU) right after its network port has
been created during domain startup or hotplug. This patch adds that
function - virDomainActualNetDefValidate(), in the conf directory,
and calls it in appropriate places in the QEMU, lxc, and libxl
drivers.
This new function is the place to put all network device validation
that 1) is hypervisor agnostic, and 2) can't be done until we know the
"actual type" of an interface.
There is no framework for validation at domain startup as there is for
post-parse validation, but I don't want to create a whole elaborate
system that will only be used by one type of device. For that reason,
I just made a single function that should be called directly from the
hypervisors, when they are initializing interfaces to start a domain,
right after conditionally allocating the network port (and regardless
of whether or not that was actually needed). In the case of the QEMU
driver, qemuDomainValidateActualNetDef() is already called in all the
appropriate places, so we can just call the new function from
there. In the case of the other hypervisors, we search for
virDomainNetAllocateActualDevice() (which is the hypervisor-agnostic
function that calls virNetworkPortCreateXML()), and add the call to our
new function right after that.
The new function itself could be plunked down into many places in the
code, but we already have 3 validation functions for network devices
in 2 different places (not counting any basic validation done in
virDomainNetDefParseXML() itself):
1) post-parse hypervisor-agnostic
(virDomainNetDefValidate() - domain_conf.c:6145)
2) post-parse hypervisor-specific
(qemuDomainDeviceDefValidateNetwork() - qemu_domain.c:5498)
3) domain-start hypervisor-specific
(qemuDomainValidateActualNetDef() - qemu_domain.c:5390)
I placed (3) right next to (2) when I added it, specifically to avoid
spreading validation all over the code. For the same reason, I decided
to put this new function right next to (1) - this way if someone needs
to add validation specific to qemu, they go to one location, and if
they need to add validation applying to everyone, they go to the
other. It looks a bit strange to have a public function in between a
bunch of statics, but I think it's better than the alternative of
further fragmentation. (I'm open to other ideas though, of course.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
These all just return a scalar value, so there's no daisy-chained
fallout from changing them, and they can easily be combined in a
single patch.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This also isn't required (due to the vportprofile being stored in the
NetDef as a pointer rather than being directly contained), but it
seemed dishonest to not mark it as const (and thus permit users to
modify its contents)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
In this case, the virNetDevBandwidthPtr that is returned is not to a
region within the virDomainNetDef arg, but points elsewhere (the
NetDef has the pointer, not the entire object), so technically it's
not necessary to make the return value a const, but it's a bit
disingenuous to *not* do it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This is needed if we want to call the function when the
virDomainNetDef* we have is a const.
Since virDomainNetGetActualVlan returns a pointer to memory that is
within the virDomainNetDefPtr arg, the returned pointer must also be
made const. This leads to a cascade of other virNetDevVlanPtr's that
must be changed to "const virNetDevVlan *".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Most likely for historical reasons our CPU def formatting code is
happily adding useless <model fallback='allow'/> for host-model CPUs. We
can just drop it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit v0.8.4-66-g95ff6b18ec (9 years ago) changed the default value for
the cpu/@match attribute to 'exact' in a rather complicated way. It did
so only if <model> subelement was present and set -1 otherwise (which is
not expected to ever happen). Thus the following two equivalent XML
elements:
<cpu mode='host-model'/>
and
<cpu mode='host-model'>
<model/>
</cpu>
would be parsed differently. The former would end up with match == -1
while the latter would have match == 1 ('exact'). This is not a big deal
since the match attribute is ignored for host-model CPUs, but we can
simplify the code and make it a little bit saner anyway.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This previous commit introduced a simpler free callback for
hash data with only 1 arg, the value to free:
commit 49288fac96
Author: Peter Krempa <pkrempa@redhat.com>
Date: Wed Oct 9 15:26:37 2019 +0200
util: hash: Add possibility to use simpler data free function in virHash
It missed two functions in the hash table code which need
to call the alternate data free function, virHashRemoveEntry
and virHashRemoveSet.
After the previous patch though, there is no code that
makes functional use of the 2nd key arg in the data
free function. There is merely one log message that can
be dropped.
We can thus purge the current virHashDataFree callback
entirely, and rename virHashDataFreeSimple to replace
it.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virChrdevHashEntryFree method uses the hash 'key'
as the name of the logfile it has to remove. By storing
a struct as the value which contains the stream and
the dev path, we can avoid relying on the hash key
when free'ing entries.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Historically we've only supported the <backingStore> as an output-only
element for domain disks. The documentation states that it may become
supported on input. To allow management apps detectin once that happens
add a domain capability which will be asserted if the hypervisor driver
will be able to obey the <backingStore> as configured on input.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 5751a0b6b1 added a helper function
called virDomainCapsFeaturesInitUnsupported which initialized all domain
capability features as unsupported.
When adding a new feature this would initialize it as unsupported also
for hypervisor drivers which the original author possibly didn't intend
to modify. To prevent accidental wrong value being reported in such case
revert back to initializing individual features in the hypervisor
drivers themselves.
This is not a straight revert as additonal patches modified how we store
the capabilities.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The qemu_domain_monitor_event_msg struct in qemu_protocol.x
defines event as a nonnull_string and qemuMonitorJSONIOProcessEvent
also errors out on a non-NULL event.
Drop the check to fix the build with static analysis.
This essentially reverts commit d343e8203d
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Both virDomainCapsCPUModelsAdd and virDomainCapsCPUModelsAddSteal are so
simple we can just squash the code in a single function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than returning a direct pointer the list stored in qemuCaps the
function now creates a new copy of the CPU models list.
The main purpose of this seemingly useless change is to update callers
to free the result returned by virQEMUCapsGetCPUDefinitions because the
internals of this function will change significantly in the following
patches.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ensure that both x and y are non-zero when resolution is specified for a
video device.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Since this function is now only called when an 'acceleration' element is
present in the xml, any failure to parse the element will be considered
an error.
Previously, we detected some types of errors, but we would only log an
error (virReportError()), but still return a partially-specified accel
object to the caller. This patch returns NULL for all parsing errors and
reports that error back up to the caller.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
The current code doesn't properly handle errors when parsing a video
device's resolution. We were returning a NULL structure for the case
where 'x' or 'y' were missing. But for the other error cases, we were
logging an error (virReportError()), but still returning an
under-specified structure. That under-specified structure was used by
the calling function rather than properly reporting an error.
This patch changes the parse function to return NULL on any parsing
error and changes the calling function to report an error when NULL is
returned.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Previously, we were passing the video "model" node to the "acceleration"
and "resolution" parsing functions and requiring them to iterate over
the children to discover and parse the appropriate node. It makes more
sense to move this responsibility up to the parent function and just
pass these functions the node that needs to be parsed.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
The 'ramfb' attribute provides a framebuffer to the guest that can be
used as a boot display for the vgpu
For example, the following configuration can be used to provide a vgpu
with a boot display:
<hostdev mode='subsystem' type='mdev' model='vfio-pci' display='on' ramfb='on'>
<source>
<address uuid='$UUID'/>
</source>
</hostdev>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
As suggested by Cole, this patch uses the domain capabilities to
validate the supported video model types. This allows us to remove the
model type validation from qemu_process.c and qemu_domain.c and
consolidates it all in a single place that will automatically adjust
when new domain capabilities are added.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Declare the capabilities as enum values and store them in an array. This
makes adding new features more straightforward and simplifies the
formatter which now doesn't require changing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For future extensions of the domain caps it's useful to have a single
point that initializes all capabilities as unsupported by a driver. The
driver then can enable specific ones.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract it to virDomainCapsFormatFeatures so that the main function does
not get so bloated over time.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce qemuDomainCapsFeatureFormatSimple which does exactly the same
thing but it's a function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A function virStringParseYesNo was added to convert
string 'yes' to true and 'no' to false, so use this
helper to replace 'STREQ(.*, \"yes\")' and
'STREQ(.*, \"no\")' as it allows us to drop several
repetitive if-then-else string->bool conversion blocks.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
This helper performs a conversion from a "yes|no" string
to a corresponding boolean, and several conversions were
already done, but there are still some omissions.
For most of the remaining usages in domain_conf.c only
"yes" is explicitly checked for. This means all other
values are implicitly handled as 'false'. In this case,
use virStringParseYesNo to handle the conversion and
reserve the original logic of not raise an error, so
ignore the return value of helper.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
In preparation for some other improvements, switch to using glib
allocation and g_autofree when parsing the 'acceleration' and
'resolution' properties of the video device.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Just above in the function, we return from the function if either x or y
are NULL, so there's no need to re-check whether x or y are NULL.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Commit 72862797 introduced resolution settings for QEMU video drivers.
It includes a new structure inside video definition. So, the code needs
to clear pointer allocation for that structure into clear function
virDomainVideoDefClear(). This commit adds this missing VIR_FREE().
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Use the new helper to initialize child XML element buffers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new helper to initialize child XML element buffers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new helper to initialize child XML element buffers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the need to pass around strings and switch to the enum values
instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The capabilities are declared in the XML schema so passing feature names
as strings from hypervisor drivers makes no sense.
Additionally some of the features expose so called 'toggles' while
others not. This knowledge was encoded by a bunch of 'STREQ's in the
formatter.
Change all of this by declaring the features as an enum and use it
instead of a dynamically allocated array.
Presence of 'toggles' is encoded together with the conversion strings
rather than in the formatter directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement and the automatic memory handlers to simplfy the
code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Use automatic memory freeing and use virXMLFormatElement instead of open
coding it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The enum name sounds too generic. It in fact describes the capabilities
of the process, thus add 'Process' to the name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code formatting storage capabilities faithfully copied the wrong use
of 'const' from domain capabilities.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
'virBlahPtr const blah' results into modification to the value of 'blah'
triggering compilation error rather than the modification of the virBlah
struct the pointer points to.
All of the domain capability formatting code was broken in this regard.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 17561eb36 modified the logic to check "if (!event)" for an
attribute that was not supposed to be passed as NULL. This causes
the static checker/Coverity build to fail. Since the check is made,
alter the header.
Also add an error message since returning -1 without some sort of
error message as previously would have happened with the failed
VIR_STRDUP so that the eventual error doesn't get the default
for some reason message.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that we don't have to deal with errors of virBuffer we can also make
this function void.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function now does not return an error so we can drop it fully.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function now does not return an error so we can drop it fully.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After the conversion of all callers that would pass true as @dynamic to
a different function we can remove the unused argument now.
Additionally modify the return type to 'size_t' as indentation can't be
negative and remove checks whether @buf is passed as it's caller's duty
to do so.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If we are updating the current checkpoint when redefining by mentioning
the current checkpoint as a parent of the newly redefined one we don't
have to clear it first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's no point in clearing the current checkpoint when we are just
changing the definition of the current checkpoint as by the virtue of the
'update_current' flag the same checkpoint would become current in
qemuCheckpointCreateXML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'other' variable was used to store the parent of the redefined
checkpoint and then the existing version of the currently redefined
checkpoint. Make it less confusing by adding a 'parent' variable for the
first case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's no point in clearing the current snapshot when we are just
changing the definition of the current snapshot as by the virtue of the
'update_current' flag the same snapshot would become current in
qemuDomainSnapshotCreateXML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In few places we have the following code pattern:
int ret;
... /* @ret is not accessed here */
ret = f(...);
return ret;
This pattern can be written less verbose:
...
return f(...);
This patch was generated with following coccinelle spatch:
@@
type T;
constant C;
expression f;
identifier ret;
@@
-T ret = C;
... when != ret
-ret = f;
-return ret;
+return f;
Afterwards I needed to fix a few places, e.g. comment in
virDomainNetIPParseXML() was removed too because coccinelle
thinks it refers to @ret while in fact it doesn't. Also in few
places it replaced @ret declaration with a few spaces instead of
removing the line. But nothing terribly wrong.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
In domain_conf.c we have virDomainSCSIDriveAddressIsUsed()
function which returns true or false if given drive address is
already in use for given domain config or not. However, it also
takes a shortcut and returns true (meaning address in use) if the
unit number equals 7. This is because for some controllers this
is reserved address. The limitation comes mostly from vmware and
applies to lsilogic, buslogic, spapr-vscsi and vmpvscsi models.
On the other hand, we were not checking for the maximum unit
number (aka LUN number) which is also relevant and differs from
model to model.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
So far, the virDomainDeviceFindSCSIController() takes
virDomainDeviceInfo structure which is an overkill. It assumes
that the passed structure is type of
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE which is not obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce a new type virHashDataFreeSimple which has only a void * as
argument for cases when knowing the name of the entry when freeing the
hash entry is not required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
ACKed-by: Eric Blake <eblake@redhat.com>
Previous commit removed last use of this function so we can get rid of
it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Introduce a simpler replacement for virDomainDiskByName when looking up
by disk target.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The disk type is not part of source and thus it's parsed earlier. This
bypasses the checks when parsing a disk type='network' if it's
completely missing the source.
Since there are possible active users of this (it was reported as a
problem with openstack) fix it by resetting the disk type to '_FILE' for
an empty cdrom which is handled correctly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In some places we need to check if a hostdev has VFIO backend.
Because of how complicated virDomainHostdevDef structure is, the
check consists of three lines. Move them to a function and
replace all checks with the function call.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Replace all occurrences of
if (VIR_STRDUP(a, b) < 0)
/* effectively dead code */
with:
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace:
if (!s && VIR_STRDUP(s, str) < 0)
goto;
with:
if (!s)
s = g_strdup(str);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace all the occurrences of
ignore_value(VIR_STRDUP(a, b));
with
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use a temporary variable to allow copying from the
currently set source.
Always return 0 since none of the callers distinguishes
between 0 and 1 propagated from VIR_STRDUP.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit adds resolution element with parameters 'x' and 'y' into video
XML domain group definition. Both, properties were added into an element
called 'resolution' and it was added inside 'model' element. They are set
as optional. This element does not follow QEMU properties 'xres' and
'yres' format. Both HTML documentation and schema were changed too. This
commit includes a simple test case to cover resolution for QEMU video
models. The new XML format for resolution looks like:
<model ...>
<resolution x='800' y='600'/>
</model>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.
Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Back in July 2009, in the days before libvirt supported explicitly
assigning a PCI address to every device, code was added to save the
PCI addresses of hotplugged network, disk, and hostdevs in the domain
status with this XML element:
<state devaddr='domain🚌slot'/>
This was added in commits 4e21a95a, 01654107, in v0.7.0, and 0c5b7b93
in v0.7.1.
Then just a few months later, in November 2009, The code that actually
formatted the "devaddr='blah'" into the status XML was removed by
commit 1b0cce7d3 (which "introduced a standardized data structure for
device addresses"). The code to *parse* the devaddr from the status
was left in for backward compatibility though (it just parses it into
the "standard" PCI address).
At the time the devaddr attribute was added, a few other attributes
already existed in the <state> element for network devices, and these
were removed over time (I haven't checked the exact dates of this),
but 10 years later, in libvirt v5.8.0, we *still* maintain code to
parse <state devaddr='blah'/> from the domain status.
In the meantime, even distros so old that we no longer support them in
upstream libvirt are using a libvirt new enough that it doesn't ever
write <state devaddr='blah'/> to the domain status XML.
Since the only way a current libvirt would ever encounter this element
would be if someone was upgrading directly from libvirt <= v0.7.5 with
running guests, it seems safe to finally remove the code that parses it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we no longer use any of the macros from this file, remove it.
This also removes a typo.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that all the types using VIR_AUTOUNREF have a cleanup func defined
to virObjectUnref, use g_autoptr instead of VIR_AUTOUNREF.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOPTR aliases to g_autoptr. Replace all of its use by the GLib
macro version.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOPTR aliases to g_autoptr. Replace all uses of VIR_DEFINE_AUTOPTR_FUNC
with G_DEFINE_AUTOPTR_CLEANUP_FUNC in preparation for replacing the
rest.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOFREE is just an alias for g_autofree. Use the GLib macros
directly instead of our custom aliases.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOCLEAN is just an alias for g_auto. Use the GLib macros
directly instead of our custom aliases.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Also define the macro for building with GLib older than 2.60
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace use of the gnulib base64 module with glib's own base64 API family.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Libvirt currently uses the VIR_AUTOUNREF macro for auto cleanup of
virObject instances. GLib approaches things differently with GObject,
reusing their g_autoptr() concept.
This introduces support for g_autoptr() with virObject, to facilitate
the conversion to GObject.
Only virObject classes which are currently used with VIR_AUTOREF are
updated. Any others should be converted to GObject before introducing
use of autocleanup.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This patch adds the implementation of the ccf-assist pSeries
feature, based on the QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST
capability that was added in the previous patch.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This device is a very simple framebuffer device supported by qemu that
is mostly intended to use as a boot framebuffer in conjunction with a
vgpu. However, there is also a standalone ramfb device that can be used
as a primary display device and is useful for e.g. aarch64 guests where
different memory mappings between the host and guest can prevent use of
other devices with framebuffers such as virtio-vga.
https://bugzilla.redhat.com/show_bug.cgi?id=1679680 describes the
issues in more detail.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
It is the only user. Rename it to match the local style
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
This reverts commit 2d8721e260.
This fix was both incomplete and too general. It only fixed domain
startup, but libvirt would still report empty list of supported CPU
models with recent QEMU for ppc64. On the other hand, while ppc64 QEMU
ignores case when looking up CPU model names, x86_64 QEMU does case
sensitive lookup. Without reverting this patch, libvirt could happily
accept CPU model names which are not supported by QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We're using gnulib to get ffs, ffsl, rotl32, count_one_bits,
and count_leading_zeros. Except for rotl32 they can all be
replaced with gcc/clangs builtins. rotl32 is a one-line
trivial function.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Implement an XML to virCPUDefPtr helper that handles the ctxt
prerequisite for virCPUDefParseXML.
This does not alter any functionality.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com>
Message-Id: <1568924706-2311-14-git-send-email-walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
'vm' is passed in which contains the definition which contains the UUID
so we don't need another parameter for this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'vm' is passed in which contains the definition which contains the UUID
so we don't need another parameter for this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Before the refactoring that properly separated the network driver from
the hypervisor driver and forced all interaction to go through public
APIs, all network usage counters were zeroed when the network driver
was initialized, and the network driver's now-deprecated
"semi-private" API networkNotifyActualDevice() was called for every
interface of every domain as each hypervisor "reconnected" its domains
during a libvirtd restart, and this would refresh the usage count for
each network.
Post-driver-split, during libvirtd restart/reconnection of the running
domains, the function virDomainNetNotifyActualDevice() is called by
each hypervisor driver for every interface of every domain restart,
and this function has code to re-register interfaces, but it only
calls into the network driver to re-register those ports that don't
already have a valid portid (ie. one that is not simply all 0),
assuming that those with valid portids are already known (and counted)
by the network driver.
commit 7ab9bdd47 recently modified the network driver so that, in most
cases, it properly resyncs each network's connection count during
libvirtd (or maybe virtnetworkd) restart by iterating through the
network's port list. This doesn't account for the case where a network
is destroyed and restarted while there are running domains that have
active ports on the network. In that case, the entire port list and
connection count for that network is lost, and now even a restart of
libvirtd/virtnetworkd/virtqemud, which in the past would resync the
connection count, doesn't help (the network driver thinks there are no
active ports, while the hypervisor driver knows about all the active
ports, but mistakenly believes that the network driver also knows).
The solution to this is to not just bypass valid portids during the
call to virDomainNetworkNotifyActualDevice(). Instead, we query the
network driver about the portid that was preserved in the domain
status, and if it is not registered, we register it.
(NB: while it would technically be correct to just generate a new
portid for these cases, it makes for less churn in portids (and thus
may make troubleshooting simpler) if we make the small fix to
virDomainNetDefActualToNetworkPort() that preserves existing valid
portids rather than unconditionally generating a new one.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
define a VIR_DEFINE_AUTOPTR_FUNC() to autofree virNetworkPortDefs, and
convert all uses of virNetworkPortDefPtr that are appropriate to use
it.
This coincidentally fixes multiple potential memory leaks (in failure
cases) in networkPortCreateXML()
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A virDomainNetDef object in a domain's nets array might contain a
virDomainHostdevDef, and when this is the case, the domain's hostdevs
array will also have a pointer to this embedded hostdev (this is done
so that internal functions that need to perform some operation on all
hostdevs won't leave out the type='hostdev' network interfaces).
When a network device was updated with virDomainUpdateDeviceFlags(),
we were replacing the entry in the nets array (and free'ing the
original) but forgetting about the pointer in the hostdevs array
(which would then point to the now-free'd hostdev contained in the old
net object.) This often resulted in a libvirtd crash.
The solution is to add a function, virDomainNetUpdate(), called by
qemuDomainUpdateDeviceConfig(), that updates the hostdevs array
appropriately along with the nets array.
Resolves: https://bugzilla.redhat.com/1558934
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The private data for video definition is created in
virDomainVideoDefNew() and we attempt to free it in
virDomainVideoDefFree(). This seems to work, except
the free function calls clear function which zeroes
out the whole structure and thus virObjectUnref()
which is called on private data does nothing.
2,568 bytes in 107 blocks are definitely lost in loss record 207 of 213
at 0x4A35476: calloc (vg_replace_malloc.c:752)
by 0x50A6048: virAllocVar (viralloc.c:346)
by 0x513CC5A: virObjectNew (virobject.c:243)
by 0x4DC1DEE: qemuDomainVideoPrivateNew (qemu_domain.c:1337)
by 0x51A6BD6: virDomainVideoDefNew (domain_conf.c:2831)
by 0x51B9F06: virDomainVideoDefParseXML (domain_conf.c:15541)
by 0x51CB761: virDomainDefParseXML (domain_conf.c:21158)
by 0x51C5973: virDomainDefParseNode (domain_conf.c:21708)
by 0x51C583A: virDomainDefParse (domain_conf.c:21663)
by 0x51C58AE: virDomainDefParseFile (domain_conf.c:21688)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In bc1e924cf0 we've introduced video driver name and whilst
doing so we've utilized VIR_ENUM_IMPL() macro. Then, in domain
XML parsing code the generated
virDomainVideoBackendTypeFromString() is called and its return
value is assigned directly to an unsigned int variable which is
wrong. Also, the video driver enum has 'default' value which is
not formatted into domain XML but is accepted during parsing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.
qemuVhostUserFillDomainGPU() will find the first matching GPU helper
with the required capabilities and set the associated
vhost_user_binary.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
vhost-user-gpu helper takes --render-node option to specify on which
GPU should the renderning be done.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Accept a new driver name attribute to specify usage of helper process, ex:
<video>
<driver name='vhostuser'/>
<model type='virtio'/>
</video>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
