After 27726d8c21 a privateData is allocated in
virDomainHostdevDefAlloc(). However, the counter part - freeing
them in Free() is missing which leads to the following memory
leak:
==6489== 24 bytes in 1 blocks are definitely lost in loss record 684 of 1,003
==6489== at 0x4C2C070: calloc (vg_replace_malloc.c:623)
==6489== by 0x54B7C94: virAllocVar (viralloc.c:560)
==6489== by 0x5517BE6: virObjectNew (virobject.c:193)
==6489== by 0x1B400121: qemuDomainHostdevPrivateNew (qemu_domain.c:798)
==6489== by 0x5557B24: virDomainHostdevDefAlloc (domain_conf.c:2152)
==6489== by 0x5575578: virDomainHostdevDefParseXML (domain_conf.c:12709)
==6489== by 0x5582292: virDomainDefParseXML (domain_conf.c:16995)
==6489== by 0x5583C98: virDomainDefParseNode (domain_conf.c:17470)
==6489== by 0x5583B07: virDomainDefParse (domain_conf.c:17417)
==6489== by 0x5583B95: virDomainDefParseFile (domain_conf.c:17441)
==6489== by 0x55A3F24: virDomainObjListLoadConfig (virdomainobjlist.c:465)
==6489== by 0x55A43E6: virDomainObjListLoadAllConfigs (virdomainobjlist.c:596)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Libxl is the last user and I don't have the toolchain prepared to
compile the libxl driver. Move it to the libxl driver to avoid having to
refactor the code.
This is place as a sub-element of <source>, where other aspects of the
host-side connection to the network device are located (network or
bridge name, udp listen port, etc). It's a bit odd that the interface
we're configuring with this info is itself named in <target dev='x'/>,
but that ship sailed long ago:
<interface type='ethernet'>
<mac address='00:16:3e:0f:ef:8a'/>
<source>
<ip address='192.168.122.12' family='ipv4'
prefix='24' peer='192.168.122.1'/>
<ip address='192.168.122.13' family='ipv4' prefix='24'/>
<route family='ipv4' address='0.0.0.0'
gateway='192.168.122.1'/>
<route family='ipv4' address='192.168.124.0' prefix='24'
gateway='192.168.124.1'/>
</source>
</interface>
In practice, this will likely only be useful for type='ethernet', so
its presence in any other type of interface is currently forbidden in
the generic device Validate function (but it's been put into the
general population of virDomainNetDef rather than the
ethernet-specific union member so that 1) we can more easily add the
capability to other types if needed, and 2) we can retain the info
when set to an invalid interface type all the way through to
validation and report a proper error, rather than just ignoring it
(which is currently what happens for many other type-specific
settings).
(NB: The already-existing configuration of IP info for the guest-side
of interfaces is in subelements directly under <interface>, and the
name of the guest-side interface (when configurable) is in <guest
dev='x'/>).
(This patch had been pushed earlier in
commit fe6a77898a, but was reverted in
commit d658456530 because it had been
accidentally pushed during the freeze for release 2.0.0)
The peer attribute is used to set the property of the same name in the
interface IP info:
<interface type='ethernet'>
...
<ip family='ipv4' address='192.168.122.5'
prefix='32' peer='192.168.122.6'/>
...
</interface>
Note that this element is used to set the IP information on the
*guest* side interface, not the host side interface - that will be
supported in an upcoming patch.
(This patch now has quite a history: it was originally pushed in
commit 690969af, which was subsequently reverted in commit 1d14b13f,
then reworked and pushed (along with a lot of other related/supporting
patches) in commit 93135abf1; however *that* commit had been
accidentally pushed during dev. freeze for release 2.0.0, so it was
again reverted in commit f6acf039f0).
Signed-off-by: Vasiliy Tolstov <v.tolstov@selfip.ru>
Signed-off-by: Laine Stump <laine@laine.org>
Add a new secret type known as "passphrase" - it will handle adding the
secret objects that need a passphrase without a specific username.
The format is:
<secret ...>
<uuid>...</uuid>
...
<usage type='passphrase'>
<name>mumblyfratz</name>
</usage>
</secret>
Signed-off-by: John Ferlan <jferlan@redhat.com>
Since the virSecretDefParseUsage ensures each of the fields is present,
no need to check during virSecretDefFormatUsage (also virBufferEscapeString
is a no-op with a NULL argument).
Signed-off-by: John Ferlan <jferlan@redhat.com>
A helper that will execute a callback on every USB device
in the domain definition.
With an ability to skip USB hubs, since we will want to treat
them differently in some cases.
virTypedParameterAssign steals the string rather than copying it into
the typed parameter and thus freeing it leads to a crash when attempting
to serialize the results.
This was introduced in commit 9f50f6e2 and later made an universal
helper in 32e6339c.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1351473
Some code paths already assume that it is allocated since it was always
allocated by virDomainPerfDefParseXML. Make it member of virDomainDef
directly so that we don't have to allocate it all the time.
This fixes crash when attempting to connect to an existing process via
virDomainQemuAttach since we would not allocate it in that code path.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1350688
This is place as a sub-element of <source>, where other aspects of the
host-side connection to the network device are located (network or
bridge name, udp listen port, etc). It's a bit odd that the interface
we're configuring with this info is itself named in <target dev='x'/>,
but that ship sailed long ago:
<interface type='ethernet'>
<mac address='00:16:3e:0f:ef:8a'/>
<source>
<ip address='192.168.122.12' family='ipv4'
prefix='24' peer='192.168.122.1'/>
<ip address='192.168.122.13' family='ipv4' prefix='24'/>
<route family='ipv4' address='0.0.0.0'
gateway='192.168.122.1'/>
<route family='ipv4' address='192.168.124.0' prefix='24'
gateway='192.168.124.1'/>
</source>
</interface>
In practice, this will likely only be useful for type='ethernet', so
its presence in any other type of interface is currently forbidden in
the generic device Validate function (but it's been put into the
general population of virDomainNetDef rather than the
ethernet-specific union member so that 1) we can more easily add the
capability to other types, and 2) we can retain the info when set to
an invalid interface type all the way through to validation and report
a proper error, rather than just ignoring it (which is currently what
happens for many other type-specific settings).
(NB: The already-existing configuration of IP info for the guest-side
of interfaces is in subelements directly under <interface>, and the
name of the guest-side interface (when configurable) is in <guest
dev='x'/>).
The peer attribute is used to set the property of the same name in the
interface IP info:
<interface type='ethernet'>
...
<ip family='ipv4' address='192.168.122.5'
prefix='32' peer='192.168.122.6'/>
...
</interface>
Note that this element is used to set the IP information on the
*guest* side interface, not the host side interface - that will be
supported in an upcoming patch.
(This is an updated *re*-commit of commit 690969af, which was
subsequently reverted in commit 1d14b13f).
Signed-off-by: Vasiliy Tolstov <v.tolstov@selfip.ru>
Signed-off-by: Laine Stump <laine@laine.org>
virDomainNetIPInfoParseXML() and virDomainNetIPInfoFormat() are no
longer "unused", so we can now remove the "ATTRIBUTE_UNUSED" from
their definitions, since virDomainNetIPInfoFormat() is now the only
caller of virDomainNetIPsFormat() and virDomainNetRoutesFormat(),
those two functions can simply be subsumed into
virDomainNetIPInfoFormat().
a.k.a. <hostdev mode='capabilities' type='net'>.
This replaces the existing nips, ips, nroutes, and routes with a
single virNetDevIPInfo, and simplifies the code by calling that
object's parse/format/clear functions instead of open coding.
There are currently two places in the domain where this combination is
used, and there is about to be another. This patch puts them together
for brevity and uniformity.
As with the newly-renamed virNetDevIPAddr and virNetDevIPRoute
objects, the new virNetDevIPInfo object will need to be accessed by a
utility function that calls low level Netlink functions (so we don't
want it to be in the conf directory) and will be called from multiple
hypervisor drivers (so it can't be in any hypervisor directory); the
most appropriate place is thus once again the util directory.
The parse and format functions are in conf/domain_conf.c because only
the domain XML (i.e. *not* the network XML) has this exact combination
of IP addresses plus routes. Note that virDomainNetIPInfoFormat() will
end up being the only caller to virDomainNetRoutesFormat() and
virDomainNetIPsFormat(), so it will just subsume those functions in a
later patch, but we can't do that until they are no longer called.
(It would have been nice to include the interface name within the
virNetDevIPInfo object (with a slight name change), but that can't
be done cleanly, because in each case the interface name is provided
in a different place in the XML relative to the routes and IP
addresses, so putting it in this object would actually make the code
more confused rather than simpler).
These functions all need to be called from a utility function that
must be located in the util directory, so we move them all into
util/virnetdevip.[ch] now that it exists.
Function and struct names were appropriately changed for the new
location, but all code is unchanged aside from motion and renaming.
When support for <interface type='ethernet'> was added in commit
9a4b705f back in 2010, it erroneously looked at <source dev='blah'/>
for a user-specified guest-side interface name. This was never
documented though. (that attribute already existed at the time in the
data.ethernet union member of virDomainNetDef, but apparently had no
practical use - it was only used as a storage place for a NetDef's
bridge name during qemuDomainXMLToNative(), but even then that was
never used for anything).
When support for similar guest-side device naming was added to the lxc
driver several years later, it was put in a new subelement <guest
dev='blah'/>.
In the intervening years, since there was no validation that
ethernet.dev was NULL in the other drivers that didn't actually use
it, innocent souls who were adding other features assuming they needed
to account for non-NULL ethernet.dev when really they didn't, so
little bits of the usual pointless cargo-cult code showed up.
This patch not only switches the openvz driver to use the documented
<guest dev='blah'/> notation for naming the guest-side device (just in
case anyone is still using the openvz driver), and logs an error if
anyone tries to set <source dev='blah'/> for a type='ethernet'
interface, it also removes the cargo-cult uses of ethernet.dev and
<source dev='blah'/>, and eliminates if from the RNG and from
virDomainNetDef.
NB: I decided on this course of action after mentioning the
inconsistency here:
https://www.redhat.com/archives/libvir-list/2016-May/msg02038.html
and getting encouragement do eliminate it in a later IRC discussion
with danpb.
Rearrange this function to be better organized and more correct:
* the error codes were changed from the incorrect INVALID_ARG to
XML_ERROR
* prefix still isn't required, but if present it must be valid or an
error will be logged.
* don't emit a debug log just because prefix is missing - this
is valid.
* group everything related to setting prefix in one place rather than
scattered through the function.
I'm tired of mistyping this all the time, so let's do it the same all
the time (similar to how we changed all "Pci" to "PCI" awhile back).
(NB: I've left alone some things in the esx and vbox drivers because
I'm unable to compile them and they weren't obviously *not* a part of
some API. I also didn't change a couple of variables named,
e.g. "somethingIptables", because they were derived from the name of
the "iptables" command)
These had been declared in conf/device_conf.h, but then used in
util/virnetdev.c, meaning that we had to #include conf/device_conf.h
in virnetdev.c (which we have for a long time said shouldn't be done.
This caused a bigger problem when I tried to #include util/virnetdev.h
in a file in src/conf (which is allowed) - for some reason the
"device_conf.h: File not found" error.
The solution is to move the data types and functions used in util
sources from conf to util. Some names were adjusted during the move
("virInterface" --> "virNetDevIf", and "VIR_INTERFACE" -->
"VIR_NETDEV_IF")
The VIR_STORAGE_POOL_EVENT_REFRESHED constant does not
reflect any change in the lifecycle of the storage pool.
It should thus not be part of the storage pool lifecycle
event set, but rather be a top level event in its own
right. Thus we introduce VIR_STORAGE_POOL_EVENT_ID_REFRESH
to replace it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Disallowing them broke a use case of testing multipath configurations
for storage. Originally this was added as it was impossible to
use certain /dev/disk-by... links but the disks worked properly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1349895
Move the enum into a new src/util/virsecret.h, rename it to be
virSecretLookupType. Add a src/util/virsecret.h in order to perform
a couple of simple operations on the secret XML and virSecretLookupTypeDef
for clearing and copying.
This includes quite a bit of collateral damage, but the goal is to remove
the "virStorage*" and replace with the virSecretLookupType so that it's
easier to to add new lookups that aren't necessarily storage pool related.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This code was attempting to handle some implicit <console> XML
formatting for manually assembled DomainDef, since previously the
console<->serial compat copying was only done at XML parse time.
Nowadays it's done via virDomainDefPostParse ->
virDomainDefAddConsoleCompat, which all manual DomainDef builders
already call, so we can drop this workaround.
In the case of chassisNr (used to set chassis_nr of a pci-bridge
controller), 0 is reserved for / used by the pci[e]-root bus. In the
base of busNr, a value of 0 would mean that the root bus had no places
available to plug in new buses, including the pxb itself (the
documentation I wrote for pxb even noted the limit of busNr as 1.254).
NB: oddly, the "chassis" attribute, which is used for pcie-root-port
and pcie-switch-downstream-port *can* be set to 0, since it's the
combination of {chassis, slot} that needs to be unique, not chassis by
itself (and slot 0 of pcie-root is reserved, while pcie-*-port can use
*only* slot 0).
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1342962
When loading status XMLs with following graphics definition:
<graphics type='spice' port='5900' autoport='yes' listen='127.0.0.1'>
<listen type='address' address='127.0.0.1' fromConfig='1'/>
<image compression='off'/>
</graphics>
libvirtd would leak a few bytes:
10 bytes in 1 blocks are definitely lost in loss record 71 of 1,127
at 0x4C2C000: malloc (vg_replace_malloc.c:299)
by 0x6789298: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4)
by 0x552AB0A: virXMLPropString (virxml.c:479)
by 0x5539536: virDomainGraphicsListensParseXML (domain_conf.c:11171)
by 0x553DD5E: virDomainGraphicsDefParseXMLSpice (domain_conf.c:11414)
by 0x553DD5E: virDomainGraphicsDefParseXML (domain_conf.c:11749)
by 0x5566061: virDomainDefParseXML (domain_conf.c:16939)
by 0x556953F: virDomainObjParseXML (domain_conf.c:17348)
by 0x556953F: virDomainObjParseNode (domain_conf.c:17513)
by 0x5569902: virDomainObjParseFile (domain_conf.c:17532)
by 0x5571E02: virDomainObjListLoadStatus (virdomainobjlist.c:514)
by 0x5571E02: virDomainObjListLoadAllConfigs (virdomainobjlist.c:596)
by 0x26E0BDC8: qemuStateInitialize (qemu_driver.c:911)
by 0x55B1FDB: virStateInitialize (libvirt.c:770)
by 0x122039: daemonRunStateInit (libvirtd.c:960)
This is going to be important later when we received
DEVICE_DELETED event on the qemu monitor. If we do,
virDomainDefFindDevice() is called to find the device for given
device alias in the virDomainDef tree. When we enable removal for
redirdevs we need to include them in the lookup process too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Basically, there are just two functions introduced here:
virDomainRedirdevDefFind which looks up given redirdev in domain
definition, and virDomainRedirdevDefRemove which removes the
device at given index in the array of devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There's currently just one limitation: redirdevs that want to go
on USB bus require a USB controller, surprisingly.
At the same time, since I'm using virDomainDefHasUSB() in this
new validator function, it has to be moved a few lines up and
also its header needed to be changed a bit: it is now taking a
const pointer to domain def since it's not changing anything in
there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While we need to know the difference between the total memory stored in
<memory> and the actual size not included in the possible memory modules
we can't pre-calculate it reliably. This is due to the fact that
libvirt's XML is copied via formatting and parsing the XML and the
initial memory size can be reliably calculated only when certain
conditions are met due to backwards compatibility.
This patch removes the storage of 'initial_memory' and fixes the helpers
to recalculate the initial memory size all the time from the total
memory size. This conversion is possible when we also make sure that
memory hotplug accounts properly for the update of the total memory size
and thus the helpers for inserting and removing memory devices need to
be tweaked too.
This fixes a bug where a cold-plug and cold-remove of a memory device
would increase the size reported in <memory> in the XML by the size of
the memory device. This would happen as the persistent definition is
copied before attaching the device and this would lead to the loss of
data in 'initial_memory'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1344892
Implement storage pool event callbacks for START, STOP, DEFINE, UNDEFINED
and REFRESHED in functions when a storage pool is created/started/stopped
etc. accordingly
This option allows or disallows detection of zero-writes if it is set to
"on" or "off", respectively. It can be also set to "unmap" in which
case it will try discarding that part of image based on the value of the
"discard" option.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
In libxl driver we do virObjectRef in libxlDomainObjBeginJob,
If virCondWaitUntil failed, it goes to error, do virObjectUnref,
There's a chance that someone undefine the vm at the same time,
and refs unref to zero, vm is freed in libxlDomainObjBeginJob.
But the vm outside function is not Null, we do virObjectUnlock(vm).
That's how we overwrite the vm memory after it's freed. I fix it.
Signed-off-by: Wang Yufei <james.wangyufei@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
../../src/conf/domain_conf.c:10949: error: declaration of 'socket'
shadows a global declaration [-Wshadow]
../../src/conf/domain_conf.c:24373: error: declaration of 'listen'
shadows a global declaration [-Wshadow]
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This new listen type is currently supported only by spice graphics.
It's introduced to make it easier and clearer specify to not listen
anywhere in order to start a guest with OpenGL support.
The old way to do this was set spice graphics autoport='no' and don't
specify any ports. The new way is to use <listen type='none'/>. In
order to be able to migrate to old libvirt the migratable XML will be
generated without the listen element and with autoport='no'. Also the
old configuration will be automatically converted to the this listen
type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
VNC graphics already supports sockets but only via 'socket' attribute.
This patch coverts that attribute into listen type 'socket'.
For backward compatibility we need to handle listen type 'socket' and 'socket'
attribute properly to support old XMLs and new XMLs. If both are provided they
have to match, if only one of them is provided we need to be able to parse that
configuration too.
To not break migration back to old libvirt if the socket is provided by user we
need to generate migratable XML without the listen element and use only 'socket'
attribute.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Even though it's auto-generated it's based on qemu.conf option and listen type
address already uses "fromConfig" to carry this information. Following commits
will convert the socket to listen element so this rename is required because
there will be also an option to get socket auto-generated independently on the
qemu.conf option.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Since it will not be called from outside of conf we can unexport it too
if we move it to the appropriate place.
Test suite change is necessary since the error will be reported sooner
now.
Similarly to the domain definition validator add a device validator. The
change to the prototype of the domain validator is necessary as
virDomainDeviceInfoIterateInternal requires a non-const pointer.
Until now we weren't able to add checks that would reject configuration
once accepted by the parser. This patch adds a new callback and
infrastructure to add such checks. In this patch all the places where
rejecting a now-invalid configuration wouldn't be a good idea are marked
with a new parser flag.
Remove the live attribute and mark the definition as transient
whether the domain is runing or not.
There were only two callers left calling with live=false:
* testDomainStartState, where the domain already is active
because we assigned vm->def->id just a few lines above the call
* virDomainObjGetPersistentDef, which now only calls
virDomainObjSetDefTransient for an active domain
Calling virDomainObjSetDefTransient with live=false is a no-op
on an inactive domain.
Only call it on an active domain, since this is the only place using
the live bool.
There's this problem on the recent gcc-6.1:
In file included from conf/domain_conf.c:37:0:
conf/domain_conf.c: In function 'virDomainChrPreAlloc':
conf/domain_conf.c:14109:35: error: potential null pointer dereference [-Werror=null-dereference]
return VIR_REALLOC_N(*arrPtr, *cntPtr + 1);
^~
./util/viralloc.h:158:73: note: in definition of macro 'VIR_REALLOC_N'
# define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count), \
^~~~~
conf/domain_conf.c: In function 'virDomainChrRemove':
conf/domain_conf.c:14133:21: error: potential null pointer dereference [-Werror=null-dereference]
for (i = 0; i < *cntPtr; i++) {
^~~~~~~
GCC basically fails to see, that the
virDomainChrGetDomainPtrsInternal will never actually return NULL
because it's never called over a domain char device with _LAST
type. But to make it shut up, lets turn this function into
returning an integer and check in the callers if a zero value
value was returned.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Okay, I admit that our code here is complex. It's not easy to
spot that NULL deref can't really happen here. So it's no wonder
that a dumb compiler fails to see all the connections and
produces the following errors:
CC conf/libvirt_conf_la-domain_conf.lo
conf/domain_conf.c: In function 'virDomainDefFormatInternal':
conf/domain_conf.c:22162:22: error: potential null pointer dereference [-Werror=null-dereference]
if (sched->policy == i)
~~~~~^~~~~~~~
<snip/>
cc1: all warnings being treated as errors
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Hand-entering indexes for 20 PCI controllers is not as tedious as
manually determining and entering their PCI addresses, but it's still
annoying, and the algorithm for determining the proper index is
incredibly simple (in all cases except one) - just pick the lowest
unused index.
The one exception is USB2 controllers because multiple controllers in
the same group have the same index. For these we look to see if 1) the
most recently added USB controller is also a USB2 controller, and 2)
the group *that* controller belongs to doesn't yet have a controller
of the exact model we're just now adding - if both are true, the new
controller gets the same index, but in all other cases we just assign
the lowest unused index.
With this patch in place and combined with the automatic PCI address
assignment, we can define a PCIe switch with several ports like this:
<controller type='pci' model='pcie-root-port'/>
<controller type='pci' model='pcie-switch-upstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
...
These will each get a unique index, and PCI addresses that connect
them together appropriately with no pesky numbers required.
Make virDomainControllerFindUnusedIndex() a global function so that it
can be used outside domain_conf.c (as well as higher up in
domain_conf.c itself)/ Also make its DomainDef arg a const* so that
functions which only have a const* to the domain can use it.
IS_USB2_CONTROLLER() is useful in more places aside from just when
assigning PCI addresses in QEMU, and is checking for enum values that
are all defined in conf/domain_conf.h anyway, so define it there
instead.
Add a new element to <domain> XML:
<os>
<acpi>
<table type="slic">/path/to/acpi/table/file</table>
</acpi>
</os>
To supply a path to a SLIC (Software Licensing) ACPI
table blob.
https://bugzilla.redhat.com/show_bug.cgi?id=1327537
Instead of setting the flag before parsing the PCI address, set
it afterwards. This ensure we can never end up in a situation
where the flag has been set but pci_dev.physical_function has
not been filled in.
Rather than only assigning a PCI address when no address is given at
all, also do it when the config says that the address type is 'pci',
but it gives no address (virDeviceInfoPCIAddressWanted()).
There are also several places after parsing but prior to address
assignment where code previously expected that any info with address
type='pci' would have a *valid* PCI address, which isn't always the
case - now we check not only for type='pci', but also for a valid
address (virDeviceInfoPCIAddressPresent()).
The test case added in this patch was directly copied from Cole's patch titled:
qemu: Wire up address type=pci auto_allocate
Prior to this, <address type='pci'/> wasn't allowed when parsing
(domain+bus+slot+function needed to be a "valid" PCI address, meaning
that at least one of domain/bus/slot had to be non-0), the RNG
required bus to be specified, and if type was set to PCI when
formatting, domain+bus+slot+function would always be output.
This makes all the address attributes optional during parse and RNG
validation, and suppresses domain+bus+slot+function if domain+bus+slot
are all 0 (NB: if d+b+s are all 0, any value for function is
nonsensical as that will never happen in the real world, and after
the next patch we will always assign a real working address to any
empty PCI address before it is ever output to anywhere).
Note that explicitly setting all attributes to 0 is equivalent to
setting none of them, which is okay, since 0000:00:00 is reserved in
any PCI bus setup, and can't be used anyway.
In order to allow <address type='pci'/> with no other attributes to
mean "I want a PCI address, but any PCI address will do" (just as
having no <address> at all usually indicates), we will need to change
several places in the code from a simple "info->type == (or !=)
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_(PCI|NONE)" into something slightly
more complex, this patch adds to new functions that take a
virDomainDeviceInfoPtr and return true/false depending on 1) whether
the current state of the info indicates that we "want" a PCI address
for this device (virDeviceInfoPCIAddressWanted()) and 2) whether this
device already has a valid PCI address
(virDeviceInfoPCIAddressPresent()).
Both of these functions required the simpler check for whether a pci
address is "empty" (i.e. all of its attributes are 0, which can never
happen in a real PCI address, since slot 0 of bus 0 of domain 0 is
always reserved), so that function is also added.
Also moves all the subordinate structs. This is necessary due to a new
inline function that will be defined in device_conf.h, and also makes
sense, because it is the *device* info that's in the struct. (Actually
a lot more stuff from domain_conf.h could move to this newer file, but
I didn't want to disturb any more than necessary).
Move code that decide whether we print the 'listen' attribute or not
into virDomainGraphicsListenDefFormatAddr() function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Name the validation function distinctively since it's called in the
parser. Later patches will add function that will validate disk
definitions that are invalid but need to be parsed to avoid losing
domains.
We support omitting listen attribute of graphics element so we should
also support omitting address attribute of listen element. This patch
also updates libvirt to always add a listen element into domain XML
except for VNC graphics if socket attribute is specified.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Move the compatibility code out of virDomainGraphicsListensParseXML()
into virDomainGraphicsListenDefParseXML(). This also fixes a small
inconsistency between the code and error message itself.
Before this patch we would search first listen element that is
type='address' to validate listen and address attributes. After this
patch we always take the first listen element regardless of the type.
This shouldn't break anything since all drivers supports only one
listen.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
If socket attribute is present we start VNC that listens only on that
unix socket. This makes the parser behave the same way as we actually
use the socket attribute.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Commit 82ba41108a made possible to use direct mapped iSCSI
volumes in qemu as disk sources but didn't remove the define time check.
Rework the check by simplifying the condition and allow any volumes to
be used with disk type='lun'.
Move filling out the default video (v)ram to DeviceDefPostParse.
This means it can be removed from virDomainVideoDefParseXML
and qemuParseCommandLine. Also, we no longer need to special case
VIR_DOMAIN_VIRT_XEN, since the per-driver callback gets called
before the generic one.
Commit 6879be48 moved adding of an implicit video device after XML
parsing. As a result, libxlDomainDeviceDefPostParse() is no longer
called to set the default vram when adding an implicit device.
Commit 6879be48 assumes virDomainVideoDefaultRAM() will set the
default vram, but it returns 0 if the domain virtType is
VIR_DOMAIN_VIRT_XEN. Attempting to start an HVM domain with vram=0
results in
error: unsupported configuration: videoram must be at least 4MB for CIRRUS
The default vram setting for Xen HVM domains depends on the device
model used (qemu-xen vs qemu-traditional), hence setting the
default is deferred to libxlDomainDeviceDefPostParse().
Call the device post-parse callback even for implicit video,
to fill out the default vram even for VIR_DOMAIN_VIRT_XEN.
https://bugzilla.redhat.com/show_bug.cgi?id=1334557
Most-of-commit-message-by: Jim Fehlig <jfehlig@suse.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1318993
Commit id 'dd519a294' caused a regression cloning a volume into a
logical pool by removing just the 'allocation' adjustment during
storageVolCreateXMLFrom. Combined with the change to not require the
new volume input XML to have a capacity listed (commit id 'e3f1d2a8')
left the possibility that a zero allocation value (e.g., not provided)
would create a thin/sparse logical volume. When a thin lv becomes fully
populated, then LVM sets the partition 'inactive' and the subsequent
fdatasync() fails.
Add a new 'has_allocation' flag to be set at XML parse time to indicate
that allocation was provided. This is done so that if it's not provided
the create-from code uses the capacity value since we document that if
omitted, the volume will be fully allocated at time of creation.
For a logical backend, that creation time is 'createVol', while for a
file backend, creation doesn't set the size, but the 'createRaw' called
during buildVolFrom will decide whether the file is sparse or not based
on the provided capacity and allocation value.
For volume clones that provide different allocation and capacity values
to allow for sparse files, there is no change.
Usage of this keyword in front of function declaration that is exported via a
header file is unnecessary, since internally, this has been the default for most
compilers for quite some time.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Commit 5ed235c6 added unnecessary redifinition of
virDomainCapsDeviceHostdev in conf/domain_capabilities.h. This breaks
build with clang 3.4:
In file included from conf/domain_capabilities.c:25:
conf/domain_capabilities.h:88:44: error: redefinition of typedef
'virDomainCapsDeviceHostdev' is a C11 feature
[-Werror,-Wtypedef-redefinition]
typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev;
^
conf/domain_capabilities.h:86:44: note: previous definition is here
typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev;
So drop one of those.
If the call to virXPathNodeSet to set naddresses fails, Coverity notes
that the subsequent VIR_ALLOC_N cannot have a negative value (well it
probably wouldn't be negative per se).
Signed-off-by: John Ferlan <jferlan@redhat.com>
Requires adding the plumbing for <device><video>
The value is <enum name='modelType'> to match the associated domain
XML of <video><model type='XXX'/>
Wire it up for qemu too
Commin 36785c7e refactored the code for input devices but introduced a
bug where we removed all keyboard from migratable XML. We have to
remove only implicit keyboards like PS2 or XEN.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Add the ability to add an 'iothread' to the controller which will be how
virtio-scsi-pci and virtio-scsi-ccw iothreads have been implemented in qemu.
Describe the new functionality and add tests to parse/validate that the
new attribute can be added.
This adds a ports= attribute to usb controller XML, like
<controller type='usb' model='nec-xhci' ports='8'/>
This maps to:
qemu -device nec-usb-xhci,p2=8,p3=8
Meaning, 8 ports that support both usb2 and usb3 devices. Gerd
suggested to just expose them as one knob.
https://bugzilla.redhat.com/show_bug.cgi?id=1271408
If a panic device is being defined without a model in a domain
the default value is always overwritten with model ISA. An ISA
bus does not exist on S390 and therefore specifying a panic device
results in an unsupported configuration.
Since the S390 architecture inherently provides a crash detection
capability the panic device should be defined in the domain xml.
This patch adds an s390 panic device model and prevents setting a
device address on it.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
libvirt-daemon-config-nwfilter will put a bunch of xml configs
into /etc/libvirt/nwfilter. These configs don't hardcode a UUID
and depends on libvirt to generate one. However the generated UUID
is never saved to disk, unless the user manually calls Define.
This makes daemon reload quite noisy with many errors like:
error : virNWFilterObjAssignDef:3101 : operation failed: filter 'allow-incoming-ipv4' already exists with uuid 50def3b5-48d6-46a3-b005-cc22df4e5c5c
Because a new UUID is generated every time the config is read from
disk, so libvirt constantly thinks it's finding a new nwfilter.
Detect if we generated a UUID when the config file is loaded; if so,
resave the new contents to disk to ensure the UUID is persisteny.
This is similar to what was done in commit a47ae7c0 with virtual
networks and generated MAC addresses
In virNWFilterObjLoad we can still fail after virNWFilterObjAssignDef,
but we don't unlock and free the created virNWFilterObjPtr in the
cleanup path.
The bit we are trying to do after AssignDef is just STRDUP in the
configFile path. However caching the configFile in the NWFilterObj
is largely redundant and doesn't follow the same pattern we use
for domain and network objects.
So just remove all the configFile caching which fixes the latent
bug as a side effect.
We historically format runtime seclabel selinux/apparmor values,
however we skip formatting runtime DAC values. This was added in
commit 990e46c454
Author: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
Date: Fri Aug 31 13:40:41 2012 +0200
conf: Avoid formatting auto-generated DAC labels
to maintain migration compatibility with libvirt < 0.10.0.
However the formatting was skipped unconditionally. Instead only
skip formatting in the VIR_DOMAIN_DEF_FORMAT_MIGRATABLE case.
https://bugzilla.redhat.com/show_bug.cgi?id=1215833
Trying to define a pool name containing an embedded '/'
will immediately fail when trying to write the XML to disk.
This patch explicitly rejects names containing a '/'
Besides our stateful driver, there are two other storage impls:
esx and phyp. esx doesn't support pool creation, so this should
doesn't apply.
phyp does support pool creation, and the name is passed to the
'mksp' tool, which google doesn't reveal whether it accepts '/'
or not. IMO the likeliness of this impacting any users is near zero
Trying to define a network name containing an embedded '/'
will immediately fail when trying to write the XML to disk.
This patch explicitly rejects names containing a '/'
Besides the network bridge driver, the only other network
implementation is a very thin one for virtualbox, which seems to
use the network name as a host interface name, which won't
accept '/' anyways, so I think this is fine to do unconitionally.
https://bugzilla.redhat.com/show_bug.cgi?id=787604
Trying to define a domain name containing an embedded '/'
will immediately fail when trying to write the XML to disk for
our stateful drivers. This patch explicitly rejects names
containing a '/', and provides an xmlopt feature for drivers
to avoid this validation check, which is enabled in every
non-stateful driver that already has xmlopt handling wired up.
(Technically this could reject a previously accepted vmname like
'/foo', however at least for the qemu driver that falls over
later when starting qemu)
https://bugzilla.redhat.com/show_bug.cgi?id=639923
We were lacking tests that are checking for the completeness of our
nodedev XMLs and also whether we output properly formatted ones. This
patch adds parsing for the capability elements inside the <capability
type='pci'> element. Also bunch of tests are added to show everything
works properly.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
We had both and the only difference was that the latter also included
information about multifunction setting. The problem with that was that
we couldn't use functions made for only one of the structs (e.g.
parsing). To consolidate those two structs, use the one in virpci.h,
include that in domain_conf.h and add the multifunction member in it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Modeled after the qemuDomainDiskPrivatePtr logic, create a privateData
pointer in the _virDomainHostdevDef to allow storage of private data
for a hypervisor in order to at least temporarily store auth/secrets
data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the auth/secret data, there's no need to add
code to handle this new structure there.
Updated copyrights for modules touched. Some didn't have updates in a
couple years even though changes have been made.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This reverts commit 690969af9c, which
added the domain config parts to support a "peer" attribute in domain
interface <ip> elements.
It's being removed temporarily for the release of libvirt 1.3.4
because the feature doesn't work, and there are concerns that it may
need to be modified in an externally visible manner which could create
backward compatibility problems.
Similarly to what commit 7140807917 did with some internal paths,
clear vnc socket paths that were generated by us. Having such path in
the definition can cause trouble when restoring the domain. The path is
generated to the per-domain directory that contains the domain ID.
However, that ID will be different upon restoration, so qemu won't be
able to create that socket because the directory will not be prepared.
To be able to migrate to older libvirt, skip formatting the socket path
in migratable XML if it was autogenerated. And mark it as autogenerated
if it already exists and we're parsing live XML.
Best viewed with '-C'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326270
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Add virDomainObjGetShortName() and use it. For now that's used in one
place, but we should expose it so that future patches can use it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Currently we only allow /dev/random and /dev/hwrng as host input
for <rng><backend model='random'/> device. This was added after
various upstream discussions in commit 4932ef45
However this restriction has generated quite a few complaints over
the years, so a new discussion was initiated:
http://www.redhat.com/archives/libvir-list/2016-April/msg00987.html
Several people suggested removing the restriction, and nobody really
spoke up to defend it. So this patch drops the path restriction
entirely
https://bugzilla.redhat.com/show_bug.cgi?id=1074464
Introduce the final accessor's to _virSecretObject data and move the
structure from virsecretobj.h to virsecretobj.c
The virSecretObjSetValue logic will handle setting both the secret
value and the value_size. Some slight adjustments to the error path
over what was in secretSetValue were made.
Additionally, a slight logic change in secretGetValue where we'll
check for the internalFlags and error out before checking for
and erroring out for a NULL secret->value. That way, it won't be
obvious to anyone that the secret value wasn't set rather they'll
just know they cannot get the secret value since it's private.
Move and rename the secretRewriteFile, secretSaveDef, and secretSaveValue
from secret_driver to virsecretobj
Need to make some slight adjustments since the secretSave* functions
called secretEnsureDirectory, but otherwise mostly just a move of code.
Move and rename secretDeleteSaved from secret_driver into virsecretobj and
split it up into two parts since there is error path code that looks to
just delete the secret data file
Move to secret_conf.c and rename to virSecretLoadAllConfigs. Also includes
moving/renaming the supporting virSecretLoad, virSecretLoadValue, and
virSecretLoadValidateUUID.
This patch replaces most of the guts of secret_driver.c with recently
added secret_conf.c APIs in order manage secret lists and objects
using the hashed virSecretObjList* lookup API's.
Add function to return a "match" filtered list of secret objects. This
function replaces the guts of secretConnectListAllSecrets.
Need to also move and make global virSecretUsageIDForDef since it'll
be used by both secret_driver.c and secret_conf.c
Add the functions to add/remove elements from the hashed secret obj list.
These will replace secret_driver functions secretAssignDef and secretObjRemove.
The virSecretObjListAddLocked will perform the necessary lookups and
decide whether to replace an existing hash entry or create a new one.
This includes setting up the configPath and base64Path as well as being
able to support the caller's need to restore from a previous definition
in case something goes wrong in the caller.
New API's including unlocked and Locked versions in order to be able
to use in either manner.
Support for searching hash object lists instead of linked lists will
replace existing secret_driver functions secretFindByUUID and
secretFindByUsage
Move virSecretObj from secret_driver.c to virsecretobj.h
To support being able to create a hashed secrets list, move the
virSecretObj to virsecretobj.h so that the code can at least find
the definition.
This should be a temporary situation while the virsecretobj.c code
is patched in order to support a hashed secret object while still
having the linked list support in secret_driver.c. Eventually, the
goal is to move the virSecretObj into virsecretobj.c, although it
is notable that the existing model from which virSecretObj was
derived has virDomainObj in src/conf/domain_conf.h and virNetworkObj
in src/conf/network_conf.h, so virSecretObj wouldn't be unique if
it were to remain in virsecretobj.h Still adding accessors to fetch
and store hashed object data will be the end goal.
Add definitions and infrastucture in virsecretobj.c to create and
handle a hashed virSecretObj and virSecretObjList including the class,
object, lock setup, and disposal API's. Nothing will call these yet.
This infrastructure will replace the forward linked list logic
within the secret_driver, eventually.
VIR_ERR_NO_SUPPORT maps to the error string
this function is not supported by the connection driver
and is largely only used for when a driver doesn't have any
implementation for a public API. So its usage with invalid
net-update requests is a bit out of place. Instead use
VIR_ERR_OPERATION_UNSUPPORTED which maps to:
Operation not supported
And is what qemu's hotplug routines use in similar scenarios
The struct contains a single boolean field, 'supported':
the meaning of this field is too generic to be limited to
devices only, and in fact it's already being used for
other things like loaders and OSs.
Instead of trying to come up with a more generic name just
get rid of the struct altogether.
Prior to this patch we didn't make any attempt to prevent two entries
in the array of interfaces/PCI devices from pointing to the same
device.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1002423
So in glibc-2.23 sys/sysmacros.h is no longer included from sys/types.h
and we don't build because of the usage of major/minor/makedev macros.
Autoconf already has AC_HEADER_MAJOR macro that check where exactly
these functions/macros are defined, so let's use that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Ploop image consists of directory with two files: ploop image itself,
called root.hds and DiskDescriptor.xml that contains information about
ploop device: https://openvz.org/Ploop/format.
Such volume are difficult to manipulate in terms of existing volume types
because they are neither a single files nor a directory.
This patch introduces new volume type - ploop. This volume type is used
by ploop volume's exclusively.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This controller provides a single PCIe port on a new root. It is
similar to pci-expander-bus, intended to provide a bus that can be
associated with a guest-identifiable NUMA node, but is for
machinetypes with PCIe rather than PCI (e.g. q35-based machinetypes).
Aside from PCIe vs. PCI, the other main difference is that a
pci-expander-bus has a companion pci-bridge that is automatically
attached along with it, but pcie-expander-bus has only a single port,
and that port will only connect to a pcie-root-port, or to a
pcie-switch-upstream-port. In order for the bus to be of any use in
the guest, it must have either a pcie-root-port or a
pcie-switch-upstream-port attached (and one or more
pcie-switch-downstream-ports attached to the
pcie-switch-upstream-port).
This is a standard PCI root bus (not a bridge) that can be added to a
440fx-based domain. Although it uses a PCI slot, this is *not* how it
is connected into the PCI bus hierarchy, but is only used for
control. Each pci-expander-bus provides 32 slots (0-31) that can
accept hotplug of standard PCI devices.
The usefulness of pci-expander-bus relative to a pci-bridge is that
the NUMA node of the bus can be specified with the <node> subelement
of <target>. This gives guest-side visibility to the NUMA node of
attached devices (presuming that management apps only assign a device
to a bus that has a NUMA node number matching the node number of the
device on the host).
Each pci-expander-bus also has a "busNr" attribute. The expander-bus
itself will take the busNr specified, and all buses that are connected
to this bus (including the pci-bridge that is automatically added to
any expander bus of model "pxb" (see the next commit)) will use
busNr+1, busNr+2, etc, and the pci-root (or the expander-bus with next
lower busNr) will use bus numbers lower than busNr.
There are two places in qemu_domain_address.c where we have a switch
statement to convert PCI controller models
(VIR_DOMAIN_CONTROLLER_MODEL_PCI*) into the connection type flag that
is matched when looking for an upstream connection for that model of
controller (VIR_PCI_CONNECT_TYPE_*). This patch makes a utility
function in conf/domain_addr.c to do that, so that when a new PCI
controller is added, we only need to add the new model-->connect-type
in a single place.
The flags used to determine which devices could be plugged into which
controllers were quite confusing, as they tried to create classes of
connections, then put particular devices into possibly multiple
classes, while sometimes setting multiple flags for the controllers
themselves. The attempt to have a single flag indicate, e.g. that a
root-port or a switch-downstream-port could connect was not only
confusing, it was leading to a situation where it would be impossible
to specify exactly the right combinations for a new controller.
The solution is for the VIR_PCI_CONNECT_TYPE_* flags to have a 1:1
correspondence with each type of PCI controller, plus a flag for a PCI
endpoint device and another for a PCIe endpoint device (the only
exception to this is that pci-bridge and pcie-expander-bus controllers
have their upstream connection classified as
VIR_PCI_CONNECT_TYPE_PCI_DEVICE since they can be plugged into
*exactly* the same ports as any endpoint device). Each device then
has a single flag for connect type (plus the HOTPLUG flag if that
device can e hotplugged), and each controller sets the CONNECT bits
for all controllers that can be plugged into it, as well as for either
type of endpoint device that can be plugged in (and the HOTPLUG flag
if it can accept hotplugged devices).
With this change, it is *slightly* easier to understand the matching
of connections (as long as you remember that the flag for a
device/upstream-facing connection of a controller is the same as that
device's type, while the flags for a controller's downstream
connections is the OR of all device types that can be plugged into
that controller). More importantly, it will be possible to correctly
specify what can be plugged into a pcie-switch-expander-bus, when
support for it is added.
When support for dmi-to-pci-bridge was added, it was assumed that,
just as with the pci-root bus, slot 0 was reserved. This is not the
case - it can be used to connect a device just like any other slot, so
remove the restriction and update the test cases that auto-assign an
address on a dmi-to-pci-bridge.
Every other maxSlot was either set to 0 or to
VIR_PCI_ADDRESS_SLOT_LAST, but this one was for some reason set to the
literal value 31 (which is the same as VIR_PCI_ADDRESS_SLOT_LAST).
This makes them all consistent.
GCC in RHEL-6 complains about listen:
../../src/conf/domain_conf.c:23718: error: declaration of 'listen' shadows a global declaration [-Wshadow]
/usr/include/sys/socket.h:204: error: shadowed declaration is here [-Wshadow]
This renames all the listen to gListen.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Since we didn't opt to use one single event for device lifecycle for a
VM we are missing one last event if the device removal failed. This
event will be emitted once we asked to eject the device but for some
reason it is not possible.
Instead of calling the virDomainGraphicsListensParseXML function for all
graphics types and ignore the wrong ones move the call only to graphics
types where we supports listen elements.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Those are the last two places that uses the getter functions. Use a
direct access instead and remove those getters.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Removes the check for graphics type, it's not a public API and developer
know what he's doing and this check makes no sense. It also removes
the ability to allocate a new array if there is none. This was used by
the virDomainGraphicsListenAdd* functions and isn't used anymore.
This is now a simple getter with simple check for listens array presence
and whether the index in out of bounds.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This effectively removes virDomainGraphicsListenSetAddress which was
used only to change the address of listen structure and possible change
the listen type. The new function will auto-expand the listens array
and append a new listen.
The old function was used on pre-allocated array of listens and in most
cases it only "add" a new listen. The two remaining uses can access the
listen structure directly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Rest of the fields of the iotune data structure did not check for
malformed integers. Use the previously defined macro to extract them
which will simplify the code and add error reporting.
Since the structure was pre-initialized to 0 we don't need to set every
single member to 0 if it's not present in the XML. Additionally if we
put the name of the field into the error message the code can be
simplified using a macro to parse the members.
If we encounter a video device with primary=yes, we insert it
at def->videos[0].
There is no need to record this in a separate variable,
just check if there already is a primary video at def->videos[0].
We call VIR_INSERT_ELEMENT_INPLACE either with 0 (for primary video)
or def->nvideos (for the rest).
Use a variable with more semantic name, since j is usually used
for iterating.
We start with both i and def->nvideos at 0 and increment both
after every successful iteration.
Use i directly, instead of passing the def->nvideos value through j.
