Commit Graph

20188 Commits

Author SHA1 Message Date
Michal Privoznik
4a8bd26039 qemuDomainUndefineFlags: Grab QEMU_JOB_MODIFY
This API is definitely modifying state of @vm. Therefore it
should grab a job.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2017-08-29 11:18:34 +02:00
Michal Privoznik
9115dcd83e qemu: Introduce and use qemuDomainRemoveInactiveJob
At some places we either already have synchronous job or we just
released it. Also, some APIs might want to use this code without
having to release their job. Anyway, the job acquire code is
moved out to qemuDomainRemoveInactiveJob so that
qemuDomainRemoveInactive does just what it promises.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2017-08-29 11:18:34 +02:00
Martin Kletzander
265d0bed0b conf: Clean up and report error in virDomainGenerateMachineName
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Martin Kletzander
2a544e3130 conf: Clean up and report error in virDomainCapsFormat
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Martin Kletzander
323f7c476e util: Use virBufferCheckError to its full potential.
We can now check for the error and not care about the return value as
it will be properly handled in virBufferContentAndReset() anyway.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Martin Kletzander
7addfa195b util: Umark virBufferCheckErrorInternal as ATTRIBUTE_RETURN_CHECK
The function is useful even without using the return value.  And if
needed, the return value can be obtained by other calls as well.  The
potential for clean-up can be seen in the following patch.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Martin Kletzander
f5ef291bdb qemu: Use short domain name in qemuDomainGetPreservedMountPath
Otherwise longer domain names might generate paths that are too long
to be created.  This follows what other parts of the code do as well.

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

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Martin Kletzander
5102be52d8 conf: Properly truncate wide character names in virDomainObjGetShortName
We always truncated the name at 20 bytes instead of characters.  In
case 20 bytes were in the middle of a multi-byte character, then the
string became invalid and various parts of the code would error
out (e.g. XML parsing of that string).  Let's instead properly
truncate it after 20 characters instead.

We cannot test this in our test suite because we would need to know
what locales are installed on the system where the tests are ran and
if there is supported one (most probably there will be, but we cannot
be 100% sure), we could initialize gettext in qemuxml2argvtest, but
there would still be a chance of getting two different (both valid,
though) results.

In order to test this it is enough to start a machine with a name for
which trimming it after 20 bytes would create invalid sequence (e.g.
1234567890123456789č where č is any multi-byte character).  Then start
the domain and restart libvirtd.  The domain would disappear because
such illegal sequence will not go through the XML parser.  And that's
not a bug of the parser, it should not be in the XML in the first
place, but since we don't use any sophisticated formatter, just
mash some strings together, the formatting succeeds.

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

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Martin Kletzander
fb61407501 qemu: Add support for virtio-vga/gpu's max_outputs= parameter
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449712

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Martin Kletzander
66c4e15335 qemu: Add capabilities for virtio-vga/gpu's max_outputs= parameter
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-29 11:17:16 +02:00
Pavel Hrdina
3ba6b532d1 qemu: implement chardev source reconnect
The reconnect attribute for chardev devices in QEMU is used to
configure the reconnect timeout in seconds.  Setting '0' value disables
the reconnect functionality thus we don't allow to set '0' for QEMU.
To disable the reconnect user should use <reconnect enabled='no'/>.

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

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-29 10:30:05 +02:00
Pavel Hrdina
9aa72a6dd5 conf: introduce reconnect element for chardev source
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-29 10:30:05 +02:00
Pavel Hrdina
5f7da98b3f qemu: introduce QEMU_CAPS_CHARDEV_RECONNECT
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-29 10:30:05 +02:00
John Ferlan
5aaa304f8d network: Fix virNetworkObjBridgeInUse return type
Rather than an int, it returns a bool - so let's define it that way

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-28 14:02:22 -04:00
John Ferlan
c447086f01 Revert "interface: Consume @def in virInterfaceObjNew"
This reverts commit 92840eb3a7.

More recent reviews/changes don't have the vir*ObjNew APIs
consuming the @def, so remove from Interface as well. Changes
needed to also deal with conflicts from commit id '46f5eca4'.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-28 14:02:22 -04:00
Erik Skultety
b988f794e3 daemon: logging: Fix --verbose option being ignored by the daemon
Commit 94c465d0 refactored the logging setup phase but introduced an
issue, where the daemon ignores verbose mode when there are no outputs
defined and the default must be used. The problem is that the default
output was determined too early, thus ignoring the potential '--verbose'
option taking effect. This patch postpones the creation of the default
output to the very last moment when nothing else can change. Since the
default output is only created during the init phase, it's safe to leave
the pointer as NULL for a while, but it will be set eventually, thus not
affecting runtime.
Patch also adjusts both the other daemons.

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

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-08-28 16:42:13 +02:00
Andrea Bolognani
1f43393283 qemu: Handle host devices not being available better
We can't retrieve the isolation group of a device that's not present
in the system. However, it's very common for VFs to be created late
in the boot, so they might not be present yet when libvirtd starts,
which would cause the guests using them to disappear.

Moreover, for other architectures and even ppc64 before isolation
groups were introduced, it's considered perfectly fine to configure a
guest to use a device that's not yet (or no longer) available to the
host, with the obvious caveat that such a guest won't be able to
start before the device is available.

In order to be consistent, when a device's isolation group can't be
determined fall back to not isolating it rather than erroring out or,
worse, making the guest disappear.

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

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2017-08-28 16:16:12 +02:00
Pavel Hrdina
e02ff020ca conf: don't close the source element inside different function
While formatting disk or chardev element they both uses
virDomainDiskSourceDefFormatSeclabel() function which also closes
the source element.  This is not extendable.

Use the new virXMLFormatElement() to properly format the source
element with possible child elements.

As a side effect it fixes a bug in disk source formatting.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-28 14:02:50 +02:00
Pavel Hrdina
4e56a3e793 util: introduce virXMLFormatElement helper
This helper allows you to better structurize the code if some element
may or may not contains attributes and/or child elements.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-28 14:02:44 +02:00
Pavel Hrdina
ef3ea17f62 util: introduce virBufferSetChildIndent macro
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-28 14:02:34 +02:00
Cole Robinson
ac87932ee3 conf: add virDomainVideoDefNew
To handle setting a default heads value. Convert callers that were
doing it by hand

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2017-08-27 09:38:12 -04:00
Cole Robinson
5db046211f qemu: domain: Move some validation out of DeviceDefPostParse
And into DeviceDefValidate which is the expected place

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2017-08-27 09:38:12 -04:00
Cole Robinson
42845acb0b qemu: parse: drop redundant video config
The ram/vram = 0 bits aren't needed, and PostParse will fill in the
needed QXL default

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2017-08-27 09:38:12 -04:00
Cole Robinson
1f17ce215f qemu: Remove remnants of xenner support
Both of these are dead code: qemu_command.c explicitly rejects
VIRT_XEN earlier in the call chain, and qemu_parse_command.c
will never set VIRT_XEN anymore

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2017-08-27 09:38:05 -04:00
Scott Garfinkle
457f8f33c9 qemu: Implement virDomainMigrateGetMaxDowntime
Add code to support querying maximum allowable downtime during live migration.
2017-08-26 07:55:17 -04:00
Scott Garfinkle
4601a546b7 Add virDomainMigrateGetMaxDowntime public API
Add virDomainMigrateGetMaxDowntime to support querying maximum allowable
downtime during live migration.
2017-08-26 07:55:17 -04:00
Andrea Bolognani
ed9db36b42 qemu: Report error on failure to set isolation group
This is more user-friendly because the error will be
displayed directly instead of being buried in the log.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-25 10:05:45 +02:00
George Dunlap
92b6c99223 libxl: Avoid a variable named 'stat'
Using a variable named 'stat' clashes with the system function
'stat()' causing compiler warnings on some platforms:

libxl/libxl_driver.c: In function 'libxlDomainBlockStatsVBD':
libxl/libxl_driver.c:5387: error: declaration of 'stat' shadows a global declaration [-Wshadow]
/usr/include/sys/stat.h:455: error: shadowed declaration is here [-Wshadow]

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2017-08-24 16:22:27 +02:00
Ján Tomko
d9fc08d103 vmx: do not treat controllers as implicit devices
When parsing the config, we look for the SCSI controllers one by one,
remembering their models, then let virDomainDefAddImplicitDevices
add them if any SCSI disk is using them.

Since these controllers are not really implicit (they are present
in the source config), add them explicitly.

This patch maintains the behavior of not adding a controller
if it was present in the config, but no disk was using it.

This also resolves the memory leak of virVMXParseConfig overwriting
the video device added by calling virDomainDefAddImplicitDevices
before the parsing is finished.

Reported-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-24 09:46:56 +02:00
Nikolay Shirokovskiy
c22f02cfdf vz: build fix
6e6faf6d changed vzDomObjAlloc signature in source but not in header file.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-08-24 09:40:34 +02:00
Ján Tomko
126e6f381a conf: check address type for USB hostdevs
https://bugzilla.redhat.com/show_bug.cgi?id=1225339
2017-08-24 09:28:53 +02:00
Ján Tomko
cdf72e19d9 conf: move hostdev address validation to virDomainHostdevDefValidate
For selected hostdev types, we validate that the address type
matches the subsystem type when parsing the XML.

Move it to the validation phase, to allow extending the checks
to other subsystem types without making existing domains disappear.
2017-08-24 09:28:53 +02:00
Nikolay Shirokovskiy
f3cdbf22e3 vz: support disabled items in vz boot order
At the time the check was written virtuozzo did not use disabled items in boot
order configuration. Boot items were always enabled. Now they can be disabled
as well. Supporting such items is easy - they just should be ignored.
2017-08-23 09:03:27 -04:00
Ján Tomko
a530078cd2 conf: do not count per-device boot elements when parsing <os><boot>
When parsing bootable devices, we maintain a bitmap of used
<boot order=""> elements. Use it in the post-parse function
to figure out whether the user tried to mix per-device and
per-domain boot elements.

This removes the need to count them twice.
2017-08-22 17:47:24 +02:00
Ján Tomko
4b759a4041 conf: create a thin wrapper above virDomainDefPostParse
Rename the original function to virDomainDefPostParseInternal
to allow adding arguments that will be only used by the internal
version.
2017-08-22 17:47:24 +02:00
Ján Tomko
c4480eef08 conf: rename virDomain*PostParseInternal to virDomain*PostParseCommon
These functions contain the post-parse steps common for all drivers.
Rename it to use the 'Common' prefix, instead of the vagueness
of 'Internal', leaving 'Internal' available for other vague uses.
2017-08-22 17:47:23 +02:00
Pavel Hrdina
1e4cc18890 conf: move chardev validation into virDomainDeviceDefValidateInternal
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
a4ba59ff4b conf: separate PTY chardev source parsing
There is no reason why to share the same code for PTY and other file
based chardev source types.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
9b341bbf41 conf: move FILE chardev source parsing to separate function
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
65f4e2ff27 conf: assign parsed strings directly into chardev source definition
Since the source element is parsed only once for these type of
character devices we don't have to use temporary variable and
check whether the variable was already set.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
2c9b97d842 conf: move UNIX chardev source parsing to separate function
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
34e0eb7dc4 conf: move UDP chardev source parsing to separate function
The extra check whether (connect|bind)(Host|Service) was set is
required because for UDP chardev there can be two source elements.
Without the check there could be a memory leak.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
40b0f40b54 conf: move TCP chardev source parsing to separate function
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
9ed225e910 conf: move mode parsing of chardev source to separate function
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
3672fc99f4 conf: move chardev log parsing to separate function
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:44:36 +02:00
Pavel Hrdina
f2be5b38f0 conf: move chardev protocol parsing to separate function
In order to ensure that the default protocol is RAW, explicitly
assigning VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW = 0.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:43:51 +02:00
Pavel Hrdina
209fa11b02 conf: error out for multiple protocol elements while parsing chardev
Remove check whether a variable was already set because the element
is parsed only once now.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:28:40 +02:00
Pavel Hrdina
23482e4b1e conf: error out for multiple log elements while parsing chardev
Remove check whether a variable was already set because the element
is parsed only once now.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:28:40 +02:00
Pavel Hrdina
21bfd1e9b9 conf: error out for multiple source elements while parsing chardev
Currently we accept and correctly parse this chardev XML:

  ...
  <channel type='tcp'>
    <source mode='connect'/>
    <source mode='bind' host='localhost'/>
    <source service='4567'/>
    <target type='virtio' name='test'/>
  </channel>
  ...

The parsed formatted XML is:

  ...
  <channel type='tcp'>
    <source mode='connect' host='localhost' service='4567'/>
    <target type='virtio' name='test'/>
  </channel>
  ...

That behavior is super wrong and should not be allowed.  If you notice
the current parse takes the first found attribute and uses that value,
so for example from the "<source mode='bind' host='localhost'/>" only
the "host" attribute is used.  It works the same way for all possible
attributes that we are able to parse for source element.

This patch enforces providing only one source element for all character
devices, only for UDP type we allow to provide two source elements
since you can specify both modes.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:28:40 +02:00
Pavel Hrdina
7d0ff9430b conf: switch from while to for loop for chardev parsing
This removes one level of indentation.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-22 17:28:40 +02:00
Ján Tomko
bad14fbbed conf: useserial: drop useless check for serial devices
Since its introduction in commit 874e65aa, if someone requests:
<os><bios useserial="yes"/><os/>
we report an error if we cannot successfully count the number
of serial devices via an XPath query.

Instead of fixing the check (and moving it to the validation phase,
to prevent existing domains from disappearing), drop it completely.
For QEMU, the number of serials is checked when building the command
line.
2017-08-22 09:44:28 +02:00
Jim Fehlig
0f1993aa15 Don't autogenerate seclabels of type 'none'
When security drivers are active but confinement is not enabled,
there is no need to autogenerate <seclabel> elements when starting
a domain def that contains no <seclabel> elements. In fact,
autogenerating the elements can result in needless save/restore and
migration failures when the security driver is not active on the
restore/migration target.

This patch changes the virSecurityManagerGenLabel function in
src/security_manager.c to only autogenerate a <seclabel> element
if none is already defined for the domain *and* default
confinement is enabled. Otherwise the needless <seclabel>
autogeneration is skipped.

Resolves: https://bugzilla.opensuse.org/show_bug.cgi?id=1051017
2017-08-21 09:22:26 -06:00
Andrea Bolognani
64357c3f93 conf: Use the correct limit for the number of PHBs
I mistakenly thought pSeries guests supported 32 PHBs,
but it turns out they only support 31. Validate the
target index accordingly.

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

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2017-08-21 13:11:49 +02:00
Andrea Bolognani
c9d75d655a conf: Move target index validation
Validation should happen after parsing, so the proper
location for it is virDomainControllerDefValidate()
rather than virDomainControllerDefParseXML().

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2017-08-21 13:11:48 +02:00
Peter Krempa
7726d1581f qemu: Implement postParse callback skipping on config reload
Use the new facility which allows to ignore failures in post parse
callbacks if they are not fatal so that VM configs are not lost if the
emulator binary is missing.

If qemuCaps can't be populated on daemon restart skip certain portions
of the post parse callbacks during config reload and re-run the callback
during VM startup.

This fixes VMs vanishing if the emulator binary was broken or
uninstalled and libvirtd was restarted.
2017-08-18 15:07:44 +02:00
Peter Krempa
7808884808 qemu: domain: Don't set default USB model if qemuCaps is missing
qemuDomainControllerDefPostParse assigns the default USB controller
model when it was not specified by the user. Skip this step if @qemuCaps
is missing so that we don't fill wrong data. This will then be fixes by
re-running the post parse callback.
2017-08-18 15:07:44 +02:00
Peter Krempa
fde772cf82 qemu: domain: Don't return default NIC model if @qemuCaps are missing
Return NULL in qemuDomainDefaultNetModel if qemuCaps is missing and the
network card model would be determined by the capabilities.
2017-08-18 15:07:44 +02:00
Peter Krempa
6df29d0816 qemu: capabilities: Tolerate missing @qemuCaps in virQEMUCapsSupportsGICVersion
Report the given GIC version as unsupported if @qemuCapsi is NULL. This
will be helpful to run post parse callbacks even if qemu is not
currently installed.
2017-08-18 15:07:44 +02:00
Peter Krempa
9088d42da1 qemu: capabilities: Tolerate missing @qemuCaps in virQEMUCapsGetCanonicalMachine
If qemuCaps are not present, just return the original machine type name.

This will help in situations when qemuCaps is not available in the post
parse callback.
2017-08-18 15:07:44 +02:00
Peter Krempa
7c5cf4983c conf: add infrastructure for tolerating certain post parse callback failures
Some failures of the post parse callback can be tolerated. This is
specifically desired when loading the configs of existing VMs. In such
case the post parse callback should not really be modifying anything
in the definition.

This patch adds a parse flag VIR_DOMAIN_DEF_PARSE_ALLOW_POST_PARSE_FAIL
which will allow the callbacks to report non-fatal failures by returning
a positive return value. In such case the field 'postParseFailed' in the
domain definition is set to true, to notify the drivers that the
callback failed and possibly needs to be re-run.
2017-08-18 15:07:44 +02:00
Peter Krempa
e159ddfab8 conf: Return any non-zero value from virDomainDeviceInfoIterateInternal callback
Post parse callbacks will need to be able to signal that they failed
non-fatally. This means that we need to return the value returned by the
callback without modification.
2017-08-18 15:07:44 +02:00
Peter Krempa
18a8c36610 qemu: domain: Don't re-allocate qemuCaps in post parse callbacks
The domain post parse callback, domain address callback and the domain
device callback (for every single device) would each grab qemuCaps for
the current emulator. This is quite wasteful. Use the new callback to do
this just once.
2017-08-18 15:07:44 +02:00
Peter Krempa
e168bc8a72 conf: Add callbacks that allocate per-def private data
Some drivers use def-specific private data across callbacks (e.g.
qemuCaps in the qemu driver). Currently it's mostly allocated in every
single callback. This is rather wasteful, given that every single call
to the device callback allocates it.

The new callback will allocate the data (if not provided externally) and
then use it for the VM, address and device post parse callbacks.
2017-08-18 15:07:44 +02:00
Peter Krempa
03132bf487 qemu: Move assignment of default emulator to the basic post parse callback 2017-08-18 15:07:44 +02:00
Peter Krempa
2f19c9325e conf: Add 'basic' post parse callback
Add yet another post parse callback, which is executed prior the real
one without @parseOpaque. This is meant to set basics before
@parseOpaque (in case of the qemu driver qemuCaps) can be allocated.

This callback will allow to optimize passing of custom parseOpaque
through the callbacks.
2017-08-18 15:07:44 +02:00
Peter Krempa
7a5f68a1f2 conf: domainlist: Explicitly report failure to load domain config
When dropping a domain report which one was dropped so that it's not
necessary to rummage through the logs.
2017-08-18 15:07:44 +02:00
Pavel Hrdina
abab46a29b qemu: don't check whether offline migration is safe
Offline migration transfers only the domain definition.

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

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-18 14:45:48 +02:00
Peter Krempa
c0ce84ca98 util: string: Introduce virStringHasChars
The helper returns true if a string contains any of the given chars.
virStringHasControlChars can be reimplemented using that helper.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-18 14:20:37 +02:00
Erik Skultety
ad97fecee7 nodedev: Introduce udevHandleOneDevice
Let this new method handle the device object we obtained from the
monitor in order to enhance readability.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-08-17 16:50:47 +02:00
Erik Skultety
d3f2820ff7 nodedev: udev: Remove the udevEventHandleCallback on fatal error
So we have a sanity check for the udev monitor fd. Theoretically, it
could happen that the udev monitor fd changes (due to our own wrongdoing,
hence the 'sanity' here) and if that happens it means we are handling an
event from a different entity than we think, thus we should remove the
handle if someone somewhere somehow hits this hypothetical case.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-08-17 16:50:47 +02:00
Erik Skultety
6167d8494c nodedev: mdev: Report an error when mdev path resolution fails
It might happen that virFileResolveLinkHelper fails on the lstat system
call. virFileResolveLink expects the caller to report an error when it
fails, however this wasn't the case for udevProcessMediatedDevice.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-08-17 16:50:47 +02:00
Christian Ehrhardt
03fb4e3dea virt-aa-helper: locking loader/nvram for qemu 2.10
Testing qemu-2.10-rc3 shows issues like:
  qemu-system-aarch64: -drive file=/home/ubuntu/vm-start-stop/vms/
  7936-0_CODE.fd,if=pflash,format=raw,unit=1: Failed to unlock byte 100

There is an apparmor deny due to qemu now locking those files:
 apparmor="DENIED" operation="file_lock" [...]
 name="/home/ubuntu/vm-start-stop/vms/7936-0_CODE.fd"
 name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
 [...] comm="qemu-system-aarch64" requested_mask="k" denied_mask="k"

The profile needs to allow locking for loader and nvram files via
the locking (k) rule.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
2017-08-17 16:11:59 +02:00
Christian Ehrhardt
580cdaa7e7 virt-aa-helper: locking disk files for qemu 2.10
Testing qemu-2.10-rc2 shows issues like:
  qemu-system-x86_64: -drive file=/var/lib/uvtool/libvirt/images/kvmguest- \
  artful-normal.qcow,format=qcow2,if=none,id=drive-virtio-disk0:
  Failed to lock byte 100

It seems the following qemu commit changed the needs for the backing
image rules:

(qemu) commit 244a5668106297378391b768e7288eb157616f64
Author: Fam Zheng <famz@redhat.com>
    file-posix: Add image locking to perm operations

The block appears as:
 apparmor="DENIED" operation="file_lock" [...]
 name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow"
 [...] comm="qemu-system-x86" requested_mask="k" denied_mask="k"

With that qemu change in place the rules generated for the image
and backing files need the allowance to also lock (k) the files.

Disks are added via add_file_path and with this fix rules now get
that permission, but no other rules are changed, example:
  -  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rw,
  +  "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" rwk

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
2017-08-17 16:11:59 +02:00
Pavel Hrdina
b71946af5c conf: use virXMLPropString and virXMLNodeContentString for vcpu parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:48 +02:00
Pavel Hrdina
49991b74d9 conf: use virXMLNodeContentString for boot options parsing
Using XPath here doesn't add any benefit.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:47 +02:00
Pavel Hrdina
422cf16079 conf: use virXMLPropStringLimit where it makes sense
The XPath call for these cases is more expensive than accessing the
XML dom node directly.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:47 +02:00
Pavel Hrdina
d5b025bb15 conf: use virXMLPropString for actual network parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:47 +02:00
Pavel Hrdina
0b36d6cb08 conf: use virXMLPropString for boot parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:47 +02:00
Pavel Hrdina
8cf6eb4612 conf: use virXMLPropString for network parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:47 +02:00
Pavel Hrdina
dee01fc5a0 conf: use virXMLPropString for IOMMU def parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:47 +02:00
Pavel Hrdina
58bf9d1d37 conf: use virXMLPropString for virDomainVirtioOptionsParseXML
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:47 +02:00
Pavel Hrdina
561726cd7f util: introduce virXMLNodeContentString
It's equivalent of calling virXPathString("string(.)", ctxt) but it
doesn't have to use the XPath resolving and parsing.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:23 +02:00
Pavel Hrdina
827cf58d50 util: introduce virXMLPropStringLimit
The virXMLPropStringLimit is an equivalent of virXPathStringLimit
which should be preferred if you already have a XML dom node or
if you need to parse more than one property.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-17 15:42:08 +02:00
Michal Privoznik
361ff0a088 network: Use self inflating bitmap for class IDs
Back in the day when I was implementing QoS for networks there
were no self inflating virBitmaps. Only the static ones.
Therefore, I had to allocate the whole 8KB of memory in order to
keep track of used/unused class IDs. This is rather wasteful
because nobody is ever gonna use that much classes (kernel
overhead would drastically lower the bandwidth). Anyway, now that
we have self inflating bitmaps we can start small and allocate
more if there's need for it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2017-08-17 14:58:11 +02:00
John Ferlan
6862ad3cd7 network: Use @nnames instead of @got
To make it clearer, let's use @nnames instead of @got for counting the
names in the @names array.  Keeps things consistent and clear.
2017-08-17 07:28:17 -04:00
John Ferlan
1910abbf47 storage: Alter @aclfilter to just @filter
Rename the variable, recent review requested just use of @filter,
so be consistent throughout.
2017-08-16 14:24:38 -04:00
John Ferlan
8ce0556fc5 secret: Alter @aclfilter to just @filter
Rename the variable, recent review requested just use of @filter,
so be consistent throughout.
2017-08-16 14:24:38 -04:00
John Ferlan
59310792a6 nwfilter: Alter @aclfilter to just @filter
Rename the variable, recent review requested just use of @filter,
so be consistent throughout.

NB: Also change the virNWFilterPtr to be @nwfilter to not conflict
with the renamed variable.
2017-08-16 14:24:38 -04:00
John Ferlan
f4b5844050 nodedev: Alter @aclfilter to just @filter
Rename the variable, recent review requested just use of @filter,
so be consistent throughout.
2017-08-16 14:24:38 -04:00
John Ferlan
63d84f18fa network: Privatize virNetworkObj
Move from virnetworkobj.h into virnetworkobj.c

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
80ba4142e1 network: Modify naming for virNetworkObjList* fetching APIs
Use the structure names in the @data setup - makes it easier than
going back to find the struct fields to make sure the order of the
data is correct.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
8473859a47 network: Use @maxnames instead of @nnames
To be consistent with the API definition, use the @maxnames instead
of @nnames when describing/comparing against the maximum names to
be provided for the *ConnectList[Defined]Networks APIs.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
178ac3d133 network: Move virObjectRef during AssignDef processing
Move the virObjectRef in virNetworkObjAssignDefLocked to after
the virHashAddEntry to make it "clearer" why the @ref is being
incremented. Upon return from the ObjNew we will have 1 ref on
the object already, adding it to the hash table requires the
increment.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
7beef70871 network: Have virNetworkObjNew lock the returned object
Forces callers to use the virNetworkObjEndAPI properly.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
26c8fe4990 network: Consistent use of @obj for virnetworkobj
Consistently use @obj for a virNetworkObjPtr.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
bc9868aaf2 network: Introduce virNetworkObjIsPersistent
In preparation to privatize the virNetworkObj - create an accessor function
to get the current @persistent value.  Also change the value to a bool rather
than an unsigned int (since that's how it's generated anyway).

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
9e8227d76b network: Introduce virNetworkObj{Is|Set}Active
In order to privatize the virNetworkObj create accessors in virnetworkobj
in order to handle the get/set of the active value.

Also rather than an unsigned int, convert it to a boolean to match other
drivers representation and the reality of what it is.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
f57d8a7b32 network: Introduce virNetworkObj{Is|Set}Autostart
In preparation for privatizing the virNetworkObj structure, create
accessors for the obj->autostart.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
db207a6233 network: Add virNetworkObj Get/Set API's for @def and @newDef
In preparation for making the object private, create a couple of API's
to get the obj->def & obj->newDef and set the obj->def.

While altering networkxml2conftest.c to use the virNetworkObjSetDef
API, fix the name of the variable from @dev to @def

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
062c38ce75 network: Add virNetworkObj Get/Set API's for @floor_sum
In preparation for making the object private, create a couple of API's
to get/set the obj->floor_sum.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
7435404fd5 network: Introduce virNetworkObjGetClassIdMap
In preparation for privatizing virNetworkObj, create accessor function to
fetch the @classIdMap.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
d5d699ec18 network: Alter virNetworkObj @class_id to be @classIdMap
Change the variable name to be a bit more descriptive and less confusing
when used with the data.network.actual->class_id.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
8489d31cee network: Add virNetworkObj Get/Set API's for @dnsmasqPid and @radvdPid
In preparation for making the object private, create/use a couple of API's
to get/set the obj->dnsmasqPid and obj->radvdPid.

NB: Since the pid's can sometimes changed based on intervening functions,
be sure to always fetch the latest value.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
4f5e32d305 network: Unconditionally initialize macmap when stopping virtual network
Since we can only ever have one reference to obj->macmap, rather
than only clearing obj->macmap during virNetworkObjUnrefMacMap
(e.g. virtual network from networkShutdownNetwork), let's just
unconditionally clear the obj->macmap to ensure that some future
change that created it's own reference to obj->macmap wouldn't
have that reference disappear if virNetworkObjDispose got called.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
f4adeae09d network: Move macmap mgmt from bridge_driver to virnetworkobj
In preparation for having a private virNetworkObj - let's create/move some
API's that handle the obj->macmap. The API's will be renamed to have a
virNetworkObj prefix to follow conventions and the arguments slightly
modified to accept what's necessary to complete their task.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
070b6f7f55 network: Move and rename networkMacMgrFileName
Move networkMacMgrFileName into src/util/virmacmap.c and rename to
virMacMapFileName. We're about to move some more MacMgr processing
files into virnetworkobj and it doesn't make sense to have this helper
in the driver or in virnetworkobj.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
8e51b30148 network: Use consistent naming in bridge_driver for virNetwork objects
Use @obj for a virNetworkObjPtr

Use @net for a virNetworkPtr

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
f8f59d39cb network: Perform some formatting cleanup in bridge_driver.h
Alter prototypes to utilize some more recent guidelines.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-16 14:17:57 -04:00
John Ferlan
fdab78b574 qemu: Fix bug assuming usage of default UUID for certificate passphrase
If an environment specific _tls_x509_cert_dir is provided, then
do not VIR_STRDUP the defaultTLSx509secretUUID as that would be
for the "default" environment and not the vnc, spice, chardev, or
migrate environments. If the environment needs a secret to decode
it's certificate, then it must provide the secret. If the secrets
happen to be the same, then configuration would use the same UUID
as the default (but we cannot assume that nor can we assume that
the secret would be necessary).
2017-08-15 16:10:27 -04:00
John Ferlan
d3e17259e1 util: Add object checking for virObject{Ref|Unref}
Rather than assuming that what's passed to virObject{Ref|Unref}
would be a virObjectPtr as long as it's not NULL, let's do the
similar checks virObjectIsClass in order to prevent a possible
increment or decrement to some field at the obj->u.s.refs offset.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
dfa0efbb77 util: Add magic number check for object validity
The virObjectIsClass API has only ever checked object validity
based on if the @obj is not NULL and it was derived from some class.
While this has worked well in general, there is one additional
check that could be made prior to calling virClassIsDerivedFrom
which loops through the classes checking the magic number against
the klass expected magic number.

If by chance a non virObject is passed, rather than assuming the
void * @obj is a _virObject and thus offsetting to obj->klass,
obj->magic, and obj->parent, let's check that the void * @obj
has at least the "base part" of the magic number in the right
place and generate a more specific VIR_WARN message if not.

There are many consumers to virObjectIsClass, include the locking
primitives virObject{Lock|Unlock}, virObjectRWLock{Read|Write},
and virObjectRWUnlock. For those callers, the locking call will
not fail, but it also will not attempt a virMutex* call which
will "most likely" fail since the &obj->lock is used.

In order to avoid some possible future wrap on the 0xCAFExxxx
value, add a check during initialization that some new class
won't cause the wrap. Should be good for a few years at least!

It is still left up to the caller to handle the failed API calls
just as it would be if it passed a NULL opaque pointer anyobj.
2017-08-14 21:41:59 -04:00
John Ferlan
19f4395230 util: Create common error path for invalid object
If virObjectIsClass fails "internally" to virobject.c, create a
macro to generate the VIR_WARN describing what the problem is.
Also improve the checks and message a bit to indicate which was
the failure - whether the obj was NULL or just not the right class

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
045d712c4b util: Introduce and use virObjectRWUnlock
Rather than overload virObjectUnlock as commit id '77f4593b' has
done, create a separate virObjectRWUnlock API that will force the
consumers to make the proper decision regarding unlocking the
RWLock's. Similar to the RWLockRead and RWLockWrite, use the
virObjectGetRWLockableObj helper. This restores the virObjectUnlock
code to using the virObjectGetLockableObj.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
bf09f002b1 util: Introduce virObjectGetRWLockableObj
Introduce a helper to handle the error path more cleanly. The same
as virObjectGetLockableObj in order to essentially follow the original
logic of commit 'b545f65d' to ensure that the input argument at least
has some validity before using.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
8b03a609dd util: Only have virObjectLock handle virObjectLockable
Now that virObjectRWLockWrite exists to handle the virObjectRWLockable
objects, let's restore virObjectLock to only handle virObjectLockable
class locks. There still exists the possibility that the input @anyobj
isn't a valid object and the resource isn't truly locked, but that
also exists before commit id '77f4593b'.

This also restores some logic that commit id '77f4593b' removed
with respect to a common code path that commit id '10c2bb2b' had
introduced as virObjectGetLockableObj. This code path merely does
the same checks as the original virObjectLock commit 'b545f65d',
but in callable/reusable helper to ensure the @obj at least has
some validity before using.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
908b33644e util: Introduce and use virObjectRWLockWrite
Instead of making virObjectLock be the entry point for two
different types of locks, let's create a virObjectRWLockWrite API
which will only handle the virObjectRWLockableClass objects.

Use the new virObjectRWLockWrite for the virdomainobjlist code
in order to handle the Add, Remove, Rename, and Load operations
that need to be very synchronous.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
John Ferlan
99a72b3eb4 util: Rename virObjectLockRead to virObjectRWLockRead
Since the class it represents is based on virObjectRWLockableClass
and in order to make sure we differentiate just in case anyone somehow
believes they could use virObjectLockRead for a virObjectLockableClass,
let's rename the API to use the RW in the name. Besides the RW locks
refer to pthread_rwlock_{init|rdlock|wrlock|unlock|destroy} while the
other locks refer to pthread_mutex_{init|lock|unlock|destroy}.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-14 21:41:59 -04:00
Pavel Hrdina
40cc355c92 qemu: fix nwfilter deadlock in qemuProcessReconnect
The correct lock order is:

  nwfilter driver lock (not used in this code path)
  nwfilter update lock
  virt driver lock (not used in this code path)
  domain object lock

but the current code have this order:

  domain object lock
  nwfilter update lock

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 18:23:56 +02:00
Pavel Hrdina
231c710460 qemu: fix nwfilter deadlock while reverting to snapshot
Introduced by commit <41127244fb90f08cf5032a5d7553f5f0390d925e>.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 18:23:21 +02:00
Pavel Hrdina
771002669b conf: use virXMLPropString for Domain def parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:22 +02:00
Pavel Hrdina
4785a401f9 conf: use virXMLPropString for IOThread pin parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:21 +02:00
Pavel Hrdina
954b52cd8b conf: use virXMLPropString for IOThread parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:21 +02:00
Pavel Hrdina
db755227ae conf: use virXMLPropString for disk geometry parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:46:21 +02:00
Pavel Hrdina
aa99ea747e conf: use virXMLPropString for KeyWrapCipherDef parsing
XPath is good for random search of elements, not for accessing
attributes of one node.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 17:42:11 +02:00
Pavel Hrdina
78ac13860a conf: cleanup virDomainChrSourceDef parsing
The @remaining variable is leftover from old code and it's not
used anymore.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 14:31:52 +02:00
Pavel Hrdina
068606f85c use virXMLNodeNameEqual instead of xmlStrEqual
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 14:31:52 +02:00
Pavel Hrdina
5bd8a1c5d8 util: introduce virXMLNodeNameEqual
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-08-14 14:31:52 +02:00
Martin Kletzander
a045317680 util: Rename virResctrl to virResctrlInfo
This way later patches can add another structures with virResctrl
prefix without the meaning being even more confusing than it needs to
be.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Martin Kletzander
7c4b4f8905 util: Make virResctrlGetCacheControlType() behave like other functions
That means that returning negative values means error and non-negative
values differ in meaning, but are all successful.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Martin Kletzander
af4270400a Move resctrl-related code from conf/capabilities to util/virresctrl
It doesn't access anything from conf/ and ti will be needed to use
from other util/ places.  This split makes the separation clearer.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Martin Kletzander
62146d8532 virxml: Fix indentation
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-14 10:01:12 +02:00
Laine Stump
f5bc8b5436 util: eliminate superfluous saveVlan check in virNetDevSetNetConfig()
Commit 81fb440b further qualified an if statement by adding the
boolean saveVlan to the condition. Coverity pointed out that this
change in the logic eliminated the need to check saveVlan in an
argument to virAsprintf().
2017-08-13 23:07:13 -04:00
Laine Stump
83074cc917 util: fix improper assignment of return value in virHostdevReadNetConfig()
Commit 9a94af6d restructured virHostdevReadNetConfig() so that it
would manually set ret = 0 after successfully reading the device's
config, but Coverity pointed out that "ret = 0" was erroneously placed
outside of an "else" clause, meaning that the the value of ret set in
the "if" clause was unnecessarily and incorrectly overwritten.

This patch moves ret = 0 into the else clause, which should silence
Coverity.
2017-08-13 23:07:13 -04:00
Guido Günther
cd3c4eece2 vbox: fix typo in warning message
Acked-by: Laine Stump <laine@laine.org>
2017-08-14 02:56:35 +02:00
Laine Stump
489a937eb4 util: check for PF online status earlier in guest startup
When using a VF from an SRIOV-capable network card in a guest (either
in macvtap passthrough mode, or via VFIO PCI device assignment), The
associated PF netdev must be online in order for the VF to be usable
by the guest. The guest, however, is not able to change the state of
the PF. And libvirt *could* set the PF online as needed, but that
could lead to the host receiving unexpected IPv6 traffic (since the
default for an unconfigured interface is to participate in IPv6
autoconf). For this reason, before assigning a VF to a guest, libvirt
verifies that the related PF netdev is online - if it isn't, then we
log an error and don't allow the guest startup to continue.

Until now, this check was done during virNetDevSetNetConfig(). This
works nicely because the same function is called both for macvtap
passthrough and for VFIO device assignment. But in the case of VFIO,
the VF has already been unbound from its netdev driver by the time we
get to virNetDevSetNetConfig(), and in the case of dual port Mellanox
NICs that have their VFs setup in single port mode, the *only* way to
determine the proper PF netdev to query for online status is via the
"phys_port_id" file that is in the VF netdev's sysfs directory. *BUT*
if we've unbound the VF from the netdev driver, then it doesn't *have*
a netdev sysfs directory.

So, in order to check the correct PF netdev for online status, this
patch moved the check earlier in the setup, into
virNetDevSaveNetConfig(), which is called *before* unbinding the VF
from its netdev driver.

(Note that this implies that if you are using VFIO device assignment
for the VFs of a Mellanox NIC that has the VFs programmed in single
port mode, you must let the VFs be bound to their net driver and use
"managed='yes'" in the device definition. To be more specific, this is
only true if the VFs in single port mode are using port *2* of the PF
- if the VFs are using only port 1, then the correct PF netdev will be
arrived at by default/chance))

  This resolves: https://bugzilla.redhat.com/267191
2017-08-11 19:13:33 -04:00
Laine Stump
9a08168301 util: restructure virNetDevReadNetConfig() to eliminate false error logs
virHostdevRestoreNetConfig() calls virNetDevReadNetConfig() to try and
read the "original config" of a netdev, and if that fails, it tries
again with a different directory/netdev name. This achieves the
desired effect (we end up finding the config wherever it may be), but
for each failure, virNetDevReadNetConfig() places a nice error message
in the system logs. Experience has shown that false-positive error
logs like this lead to erroneous bug reports, and can often mislead
those searching for *real* bugs.

This patch changes virNetDevReadNetConfig() to explicitly check if the
file exists before calling virFileReadAll(); if it doesn't exist,
virNetDevReadNetConfig() returns a success, but leaves all the
variables holding the results as NULL. (This makes sense if you define
the purpose of the function as "read a netdev's config from its config
file *if that file exists*).

To take advantage of that change, the caller,
virHostdevRestoreNetConfig() is modified to fail immediately if
virNetDevReadNetConfig() returns an error, and otherwise to try the
different directory/netdev name if adminMAC & vlan & MAC are all NULL
after the preceding attempt.
2017-08-11 19:09:49 -04:00
Laine Stump
b67eaa6351 util: save the correct VF's info when using a dual port SRIOV NIC in single port mode
Mellanox ConnectX-3 dual port SRIOV NICs present a bit of a challenge
when assigning one of their VFs to a guest using VFIO device
assignment.

These NICs have only a single PCI PF device, and that single PF has
two netdevs sharing the single PCI address - one for port 1 and one
for port 2. When a VF is created it can also have 2 netdevs, or it can
be setup in "single port" mode, where the VF has only a single netdev,
and that netdev is connected either to port 1 or to port 2.

When the VF is created in dual port mode, you get/set the MAC
address/vlan tag for the port 1 VF by sending a netlink message to the
PF's port1 netdev, and you get/set the MAC address/vlan tag for the
port 2 VF by sending a netlink message to the PF's port 2 netdev. (Of
course libvirt doesn't have any way to describe MAC/vlan info for 2
ports in a single hostdev interface, so that's a bit of a moot point)

When the VF is created in single port mode, you can *set* the MAC/vlan
info by sending a netlink message to *either* PF netdev - the driver
is smart enough to understand that there's only a single netdev, and
set the MAC/vlan for that netdev. When you want to *get* it, however,
the driver is more accurate - it will return 00:00:00:00:00:00 for the
MAC if you request it from the port 1 PF netdev when the VF was
configured to be single port on port 2, or if you request if from the
port 2 PF netdev when the VF was configured to be single port on port
1.

Based on this information, when *getting* the MAC/vlan info (to save
the original setting prior to assignment), we determine the correct PF
netdev by matching phys_port_id between VF and PF.

(IMPORTANT NOTE: this implies that to do PCI device assignment of the
VFs on dual port Mellanox cards using <interface type='hostdev'>
(i.e. if you want the MAC address/vlan tag to be set), not only must
the VFs be configured in single port mode, but also the VFs *must* be
bound to the host VF net driver, and libvirt must use managed='yes')

By the time libvirt is ready to set the new MAC/vlan tag, the VF has
already been unbound from the host net driver and bound to
vfio-pci. This isn't problematic though because, as stated earlier,
when a VF is created in single port mode, commands to configure it can
be sent to either the port 1 PF netdev or the port 2 PF netdev.

When it is time to restore the original MAC/vlan tag, again the VF
will *not* be bound to a host net driver, so it won't be possible to
learn from sysfs whether to use the port 1 or port 2 PF netdev for the
netlink commands. And again, it doesn't matter which netdev you
use. However, we must keep in mind that we saved the original settings
to a file called "${PF}_${VFNUM}". To solve this problem, we just
check for the existence of ${PF1}_${VFNUM} and ${PF2}_${VFNUM}, and
use whichever one we find (since we know that only one can be there)
2017-08-11 19:05:20 -04:00
Laine Stump
39d136b67b util: match phys_port_id when converting PF-netdev to/from VF-netdev
This patch updates functions in netdev.c to pay attention to
phys_port_id. It uses the new function virNetDevGetPhysPortID() to
learn the phys_port_id of a VF or PF, then sends that info to
virPCIGetNetName(), which has newly been modified to take an optional
phys_port_id.
2017-08-11 18:55:25 -04:00
Laine Stump
b3b5aa75ed util: make virPCIGetNetName() more versatile
A single PCI device may have multiple netdevs associated with it. Each
of those netdevs will have a different phys_port_id entry in
sysfs. This patch modifies virPCIGetNetName() to allow selecting one
of the potential many netdevs in two different ways:

1) by setting the "idx" argument, the caller can select the 1st (0),
2nd (1), etc. netdev from the PCI device's net subdirectory.

2) If the physPortID arg is set (to a null-terminated string) then
virPCIGetNetName() returns the netdev that has that phys_port_id in
the sysfs file of the same name in the netdev's directory.
2017-08-11 18:35:09 -04:00
Laine Stump
0dc67e6d2d util: Fix const'ness of 1st arg to virPCIGetNetName()
The first arg isn't modified in the function, so it should be const.
2017-08-11 18:30:14 -04:00
Laine Stump
48f33bb5df util: new function virNetDevGetPhysPortID()
On Linux each network device *can* (but not necessarily *does*) have
an attribute called phys_port_id which can be read from the file of
that name in the netdev's sysfs directory. The examples I've seen have
been a many-digit hexadecimal number (as an ASCII string).

This value can be useful when a single PCI device is associated with
multiple netdevs (e.g a dual port Mellanox SR-IOV NIC - this card has
a single PCI Physical Function (PF), and that PF has two netdevs
associated with it (the "net" subdirectory of the PF in sysfs has two
links rather than the usual single link to a netdev directory). Each
of the PF netdevs has a different phys_port_id. The Virtual Functions
(VF) are similar - the PF (a PCI device) has "n" VFs (also each of
these is a PCI device), each VF has two netdevs, and each of the VF
netdevs points back to the VF PCI device (with the "device" entry in
its sysfs directory) as well as having a phys_port_id matching the PF
netdev it is associated with.

virNetDevGetPhysPortID() simply attempts to read the phys_port_id for
the given netdev and return it to the caller. If this particular
netdev driver doesn't support phys_port_id, it returns NULL (*not* a
NULL-terminated string, but a NULL pointer) but still counts it as a
success.
2017-08-11 18:25:00 -04:00
Michal Privoznik
e255cf02b2 qemuBuildMemoryBackendStr: Handle one more corner case
https://bugzilla.redhat.com/show_bug.cgi?id=1458638

This code is so complicated because we allow enabling the same
bits at many places. Just like in this case: huge pages can be
enabled by global <hugepages/> element under <memoryBacking> or
on per <memory/> basis. To complicate things a bit more, users
are allowed to omit the page size which case the default page
size is used. And this is what is causing this bug. If no page
size is specified, @pagesize is keeping value of zero throughout
whole function. Therefore we need yet another boolean to hold
[use, don't use] information as we can't sue @pagesize for that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-10 17:26:30 +02:00
Michal Privoznik
667a37a1ff virDomainNetDefClear: Free @coalesce
In virDomainNetDefParseXML() the def->coalesce is parsed and
allocated by virDomainNetDefCoalesceParseXML() but in fact it's
never freed .

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-10 09:33:11 +02:00
Michal Privoznik
e488ebb31d qemuDomainUndefineFlags: unlink nvram file regardless of domain state
https://bugzilla.redhat.com/show_bug.cgi?id=1467245

Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). One of the solutions is to remove if
regardless of the domain state and rely on qemu having the file
opened. This still has a downside that if the domain is defined
back the NVRAM store file is going to be new, any changes to the
current one are lost (just like with any other file that is
deleted while a process has it opened). But is it really a
downside?

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-10 09:30:15 +02:00
Ján Tomko
e9f3222705 introduce virConfReadString
Rewrite virConfReadMem to take a null-terminated string.
All the callers were calling strlen on it anyway.
2017-08-08 12:19:17 +02:00
Ján Tomko
5d218156a8 conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT
Make the comparison explicit.
2017-08-08 12:07:52 +02:00
Sri Ramanujam
e756333dac hyperv: Silence clang alignment warnings in serialization code
Slight refactor of the WMI serialization code to minimize mixing
openwsman and libxml2 APIs that triggered clang alignment warnings.

The only usage of libxml2 APIs now is in creating CDATA blocks,
because the openwsman API does not provide that functionality. The
clang alignment warning in this case is silenced by casting to a
void pointer first.
2017-08-07 23:14:01 +02:00
Sri Ramanujam
97197ce243 hyperv: Correct number of milliseconds in five minutes 2017-08-07 21:34:51 +02:00
Daniel P. Berrange
10e277a432 Remove bogus warning about vir$OBJECTGetConnect functions
The API docs for the various vir$OBJECTGetConnect functions
contain a warning

  WARNING: When writing libvirt bindings in other languages, do
  not use this function.  Instead, store the connection and
  the domain object together.

There is no reason why language bindings should not use this
method, and indeed the Perl, Python, and Go bindings all use
these methods.

This warning was originally added back in

  commit 3edb4bc9fb
  Author: Daniel Veillard <veillard@redhat.com>
  Date:   Tue Jul 24 15:32:55 2007 +0000

    * libvirt.spec.in NEWS docs/* po/*: preparing release 0.3.1
    * src/libvirt.c python/generator.py: some cleanup and warnings
      from Richard W.M. Jones

IIUC, the rational was that these APIs do not need to be
directly exposed to the non-C language, as the language
can expose the same concept itself by storing the original
virConnectPtr object alongside the virDomainPtr.  There's
no reason to mandate such an approach though - it is valid
for languages to expose this directly if that suits their
needs better.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2017-08-07 13:39:28 +01:00
Michal Privoznik
e0a4eaa913 qemuDomainObjPrivateFree: Free @machineName
We're storing the machine name in @priv but free it just in
qemuProcessStop, Therefore this may leak.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-07 10:44:06 +02:00
Michal Privoznik
2d533297a0 virNodeDevCapCCWParseXML: Free temporary variables
Again, we are using @cssid, @ssid and @devno to store some
temporary strings, but never free it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-07 10:44:06 +02:00
Michal Privoznik
65cb40a07e virDomainDefParseXML: Free @tmp
When parsing <ioapic> feature we're using @tmp to store some
temporary string but never free it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-07 10:44:06 +02:00
Michal Privoznik
5619c93ce7 virDomainDefFree: Don't leak initenv name and value
When parsing boot options from domain XML in
virDomainDefParseBootOptions() initenv id stored to:

    def->os.initenv[i]->name
    def->os.initenv[i]->value

But these are never freed.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-07 10:44:05 +02:00
Alexander Nusov
ed210660d6 bhyve: Add support for VNC autoport
This patch adds support for automatic VNC port assignment for bhyve guests.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2017-08-06 09:24:26 +04:00
Daniel P. Berrange
b7dba9336f remote: increase max storage pools, nwfilters & snapshots to 16384
Most other top level objects have already had their limits increased
to 16384. Increase the storage pool, nwfilter & snapshot object
limits to match. For snapshots at least, we have seen hosts which
exceeded the current limit

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2017-08-04 16:24:03 +01:00
Marek Marczykowski-Górecki
1be213e295 cpu: define sub-leaf 0 for leaf 7 in cpu_map.xml
CPUID leaf 7 is sub-leaf aware. Add missing attribute.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2017-08-04 16:45:11 +02:00
Cole Robinson
64665fa82f qemu: command: explicitly error for non-x86 default CPU
The code only currently handles writing an x86 default -cpu
argument, and doesn't know anything about other architectures.
Let's make this explicit rather than leaving ex. qemu ppc64 to
throw an error about -cpu qemu64

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2017-08-03 11:54:42 -04:00
Cole Robinson
05583fcb37 qemu: command: rework adding of default cpu model
Certain XML features that aren't in the <cpu> block map to -cpu
flags on the qemu cli. If one of these is specified but the user
didn't explicitly pass an XML <cpu> model, we need to format a
default model on the command line.

The current code handles this by sprinkling this default cpu handling
among all the different flag string formatting. Instead, switch it
to do this just once.

This alters some test output slightly: the previous code would
write the default -cpu in some cases when no flags were actually
added, so the output was redundant.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2017-08-03 11:54:37 -04:00
Ján Tomko
b4f7793ce2 conf: fix formatting of smartcard devices
My commit 0c1d863 broke formatting of passthrough smartcard devices:
<smartcard mode='passthrough' type='spicevmc'/>

resulted in invalid XML:
    <smartcard mode='passthrough'>
       type='spicevmc'>
      <address type='ccid' controller='0' slot='0'/>
    </smartcard>

Split out chardev source formatting function into two -
one formatting the attributes and other formatting the subelements.

Reported-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2017-08-03 17:19:04 +02:00
Peter Krempa
0b1ecf7b53 util: hash: Make virHashCodeGen mockable
Export the function from the util module so that dynamic linking can
override it.
2017-08-03 09:49:15 +02:00
Peter Krempa
8982f3ab20 util: hash: Include stdbool.h in the header file
The functions declared in virhash.h return bool, but stdbool.h was not
included.
2017-08-03 09:49:15 +02:00
Nikolay Shirokovskiy
cc6d43bb49 qemu: command: align disk serial check to schema
Disk serial schema has extra '.+' allowed characters in comparison
with check in code. Looks like there is no reason for that as qemu
allows any character AFAIK for serial. This discrepancy is originated
in commit id '85d15b51' where the ability to add serial was added.

Alter the disk-serial test to add a disk with all the possible
characters listed as the serial value.
2017-08-02 19:19:55 -04:00
John Ferlan
dc4c2f75ab qemu: Check for existence of provided *_tls_x509_cert_dir
https://bugzilla.redhat.com/show_bug.cgi?id=1458630

Introduce virQEMUDriverConfigTLSDirResetDefaults in order to check
if the defaultTLSx509certdir was changed, then change the default
for any other *TLSx509certdir that was not set to the default default.

Introduce virQEMUDriverConfigValidate to validate the existence of
any of the *_tls_x509_cert_dir values that were uncommented/set,
incuding the default.

Update the qemu.conf description for default to describe the consequences
if the default directory path does not exist.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-08-02 15:51:50 -04:00
Ján Tomko
ba54acd3c7 Turn virDomainDeviceInfoFormat into void
The rombar attribute was already validated at the time of parsing
the XML.
2017-08-02 15:49:30 +02:00
Ján Tomko
7330a36579 conf: check for buffer errors before virBufferUse
After an OOM error, virBuffer* APIs set buf->use to zero.
Adding a buffer to the parent buffer only if use is non-zero
would quietly drop data on error.

Check the error beforehand to make sure buf->use is zero
because we have not attempted to add anything to it.
2017-08-02 15:49:23 +02:00
Ján Tomko
338c74dacd Use a separate buffer for <hub> subelements
Switch virDomainHubDefFormat to use a separate buffer for subelements.
2017-08-02 15:45:53 +02:00
Ján Tomko
cc7368e962 Use a separate buffer for <watchdog> subelements
Convert virDomainWatchdogDefFormat to use a separate
buffer for subelements.
2017-08-02 15:45:53 +02:00
Ján Tomko
a0d43268ca Use a separate buffer for <sound> subelements
Convert virDomainSoundDefFormat to use a separate buffer
for subelements.
2017-08-02 15:45:53 +02:00
Ján Tomko
0c1d863232 Use a separate buffer for <smartcard> subelements
Convert virDomainSmartcardDefFormat to use a separate buffer
for possible subelements, to avoid the need for duplicated
formatting logic in virDomainDeviceInfoNeedsFormat.
2017-08-02 15:45:52 +02:00
Ján Tomko
b301ed584c virDomainDeviceInfoFormat: delete outdated comments
This function has grown to format more than just the address.
Delete the comment completely to avoid failing to update it
in the future.

Also, the indentation is now handled by the virBuffer APIs,
so the comment about indentation no longer makes sense.
2017-08-02 15:45:52 +02:00
Ján Tomko
6233e7bb74 Remove superfluous usage of virDomainDeviceInfoNeedsFormat
This function returns false if virDomainDeviceInfoFormat
would not format anything.

Using it as the sole condition to decide whether to call
virDomainDeviceInfoFormat or not is pointless, since
the conditions are repeated in virDomainDeviceInfoFormat.
2017-08-02 15:45:52 +02:00
Ján Tomko
2403dcce51 virCapabilitiesFormatCaches: free cpus_str right after use
This will simplify the cleanup when we start checking for
buffer errors.
2017-08-02 15:44:34 +02:00
Michal Privoznik
2074ef6cd4 Add support for virtio-net.tx_queue_size
https://bugzilla.redhat.com/show_bug.cgi?id=1462653

Just like I've added support for setting rx_queue_size (in
c56cdf259 and friends), qemu just gained support for setting tx
ring size.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-02 15:37:09 +02:00
Daniel P. Berrange
8b9d017de3 driver: conditionalize use of dlopen functions & use mingw-dlfcn
Not every platform is guaranteed to have dlopen/dlsym, so we should
conditionalize its use. Suprisingly it is actually present for Win32
via the mingw-dlfcn add on, but we should still conditionalize it.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2017-08-02 13:51:28 +01:00
Michal Privoznik
d8447faa61 virnetworkobj: Make virNetworkObjFindBy{UUID,Name}Locked() static again
These functions were made exportable back in 3aa3e072 when I was
splitting network code into parsing and list management parts.
Since then the split is finished now and these two functions do
not need to be exported anymore.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-02 10:03:23 +02:00
Michal Privoznik
8e2a535cf4 virConnect: Update comment for @privateData
This member allows us to store a pointer to some private data.
However, the comment says it's used in both domain driver and
network driver. Well, it is not. It's just one pointer and domain
driver uses it directly. Network driver has a global driver
variable. Update the comment to not confuse others.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-02 10:03:23 +02:00
Martin Kletzander
848378c64c qemu: Split shmem preparation as it's supposed to be
Since the introduction of shmem, there was a split of preparation code
from the formatting code from qemuBuildCommandLine() into
qemuProcessPrepareDomain().  Let's fix shmem in this regard, so that
we can slowly get to a cleaner codebase.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-08-02 09:27:22 +02:00
Michal Privoznik
ace45e67ab virNetDaemonCallInhibit: Call virNetDaemonGotInhibitReply properly
So there are couple of issues here. Firstly, we never unref the
@pendingReply and thus it leaks.

==13279== 144 (72 direct, 72 indirect) bytes in 1 blocks are definitely lost in loss record 1,095 of 1,259
==13279==    at 0x4C2E080: calloc (vg_replace_malloc.c:711)
==13279==    by 0x781FA97: _dbus_pending_call_new_unlocked (in /usr/lib64/libdbus-1.so.3.14.11)
==13279==    by 0x7812A4C: dbus_connection_send_with_reply (in /usr/lib64/libdbus-1.so.3.14.11)
==13279==    by 0x56BEDF3: virNetDaemonCallInhibit (virnetdaemon.c:514)
==13279==    by 0x56BEF18: virNetDaemonAddShutdownInhibition (virnetdaemon.c:536)
==13279==    by 0x12473B: daemonInhibitCallback (libvirtd.c:742)
==13279==    by 0x1249BD: daemonRunStateInit (libvirtd.c:823)
==13279==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==13279==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==13279==    by 0x928DE3C: clone (in /lib64/libc-2.23.so)

Secondly, while we send the message, we are suspended ('cos we're
talking to a UNIX socket).  However, until we are resumed back
again the reply might have came therefore subsequent
dbus_pending_call_set_notify() has no effect and in fact the
virNetDaemonGotInhibitReply() callback is never called. Thirdly,
the dbus_connection_send_with_reply() has really stupid policy
for return values. To cite the man page:

  Returns
      FALSE if no memory, TRUE otherwise.

Yes, that's right. If anything goes wrong and it's not case of
OOM then TRUE is returned, i.e. you're trying to pass FDs and
it's not supported, or you're not connected, or anything else.
Therefore, checking for return value of
dbus_connection_send_with_reply() is not enoguh. We also have to
check if @pendingReply is not NULL before proceeding any further.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-01 11:37:51 +02:00
Michal Privoznik
3e609bf4e4 virCgroupValidateMachineGroup: Don't free @machinename
We are given a string in @machinename, we never allocate it, just
merely use it for reading. We should not free it otherwise it
leads to double free:

==32191== Thread 17:
==32191== Invalid free() / delete / delete[] / realloc()
==32191==    at 0x4C2D1A0: free (vg_replace_malloc.c:530)
==32191==    by 0x54BBB84: virFree (viralloc.c:582)
==32191==    by 0x2BC04499: qemuProcessStop (qemu_process.c:6313)
==32191==    by 0x2BC500FF: processMonitorEOFEvent (qemu_driver.c:4724)
==32191==    by 0x2BC502FC: qemuProcessEventHandler (qemu_driver.c:4769)
==32191==    by 0x5550640: virThreadPoolWorker (virthreadpool.c:167)
==32191==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==32191==    by 0x928DE3C: clone (in /lib64/libc-2.23.so)
==32191==  Address 0x31893d70 is 0 bytes inside a block of size 1,100 free'd
==32191==    at 0x4C2D1A0: free (vg_replace_malloc.c:530)
==32191==    by 0x54BBB84: virFree (viralloc.c:582)
==32191==    by 0x54C1936: virCgroupValidateMachineGroup (vircgroup.c:343)
==32191==    by 0x54C4B29: virCgroupNewDetectMachine (vircgroup.c:1550)
==32191==    by 0x2BBDDA29: qemuConnectCgroup (qemu_cgroup.c:972)
==32191==    by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:6822)
==32191==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==32191==    by 0x928DE3C: clone (in /lib64/libc-2.23.so)
==32191==  Block was alloc'd at
==32191==    at 0x4C2BE80: malloc (vg_replace_malloc.c:298)
==32191==    by 0x4C2E35F: realloc (vg_replace_malloc.c:785)
==32191==    by 0x54BB492: virReallocN (viralloc.c:245)
==32191==    by 0x54BEDF2: virBufferGrow (virbuffer.c:150)
==32191==    by 0x54BF3B9: virBufferVasprintf (virbuffer.c:408)
==32191==    by 0x54BF324: virBufferAsprintf (virbuffer.c:381)
==32191==    by 0x55BB271: virDomainGenerateMachineName (domain_conf.c:27078)
==32191==    by 0x2BBD5B8F: qemuDomainGetMachineName (qemu_domain.c:9595)
==32191==    by 0x2BBDD9B4: qemuConnectCgroup (qemu_cgroup.c:966)
==32191==    by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:6822)
==32191==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)

Moreover, make the @machinename 'const char *' to mark it
explicitly that we are not changing the passed string.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2017-08-01 11:37:51 +02:00
Erik Skultety
159be14d9c nodedev: Fix double unlock of the driver on udevEnumerateDevices failure
Commit @4cb719b2dc moved the driver locks around since these have become
unnecessary at spots where the code handles now self-lockable object
list, but missed the possible double unlock if udevEnumerateDevices
fails, because at that point the driver lock had been already dropped.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2017-07-28 10:52:28 +02:00
Peter Krempa
6cf7fc573f security: apparmor: load the storage driver dynamically
In commit 5e515b542d I've attempted to fix the inability to access
storage from the apparmor helper program by linking with the storage
driver. By linking with the .so the linker complains that it's not
portable. Fix this by loading the module dynamically as we are supposed
to do.

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
2017-07-27 12:00:35 +02:00
Peter Krempa
5aec02dc37 make: Drop building without driver modules
Driver modules proved to be reliable for a long time. Since support for
not building modules complicates the code and makefiles drop it.

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
2017-07-27 12:00:35 +02:00
Pavel Hrdina
ed4d1653ed qemu: properly handle '=' in the VNC socket path
If a domain name contains a '=' and the unix socket path is
auto-generated or socket path provided by user contains '=' QEMU
is unable to properly parse the command line.  In order to make it
work we need to use the new command line syntax for VNC if it's
available, otherwise we can use the old syntax.

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

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-07-27 10:49:55 +02:00
Pavel Hrdina
cb9e09b502 qemu: capabilities: introduce QEMU_CAPS_VNC_MULTI_SERVERS
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-07-27 10:49:55 +02:00
Peter Krempa
c03e52af57 qemu: block: store and test driver names for detected storage nodes
Store the 'drv' field both for the storage node and for the format node
and format them in the test case.

Reviewed-by: Eric Blake <eblake@redhat.com>
2017-07-27 09:56:41 +02:00
Peter Krempa
0175dc6ea0 qemu: block: Refactor node name detection code
Remove the complex and unreliable code which inferred the node name
hierarchy only from data returned by 'query-named-block-nodes'. It turns
out that query-blockstats contain the full hierarchy of nodes as
perceived by qemu so the inference code is not necessary.

In query blockstats, the 'parent' object corresponds to the storage
behind a storage volume and 'backing' corresponds to the lower level of
backing chain. Since all have node names this data can be really easily
used to detect node names.

In addition to the code refactoring the one remaining test case needed
to be fixed along.

Reviewed-by: Eric Blake <eblake@redhat.com>
2017-07-27 09:47:40 +02:00
Peter Krempa
c61d169327 util: storagefile: rename 'nodebacking' to 'nodestorage' in virStorageSource
Make it less confusing by naming the field which refers to the storage
object as 'nodestorage'.

Reviewed-by: Eric Blake <eblake@redhat.com>
2017-07-27 09:44:05 +02:00
Peter Krempa
4a58dfc24e qemu: block: Rename qemuBlockFillNodeData and move it to the top
The same operation will become useful in other places so rename the
function to be more generic and move it to the top so that it can be
reused earlier in the file.

Reviewed-by: Eric Blake <eblake@redhat.com>
2017-07-27 09:42:31 +02:00
Peter Krempa
3c60388591 util: buffer: Add virBufferStrcatVArgs
Split out the worker loop into a separate function and export it.

Reviewed-by: Eric Blake <eblake@redhat.com>
2017-07-27 09:31:14 +02:00
Peter Krempa
51d243bdbf qemu: monitor: Extract call of 'query-blockstats' and add new API for it
Allow getting the raw data from query-blockstats, so that we can use it
to detect the backing chain later on.

Reviewed-by: Eric Blake <eblake@redhat.com>
2017-07-27 09:25:26 +02:00
John Ferlan
43e6686c7f storage: Disallow usage of the HBA for a fc_host backing
Disallow providing the wwnn/wwpn of the HBA in the adapter XML:

  <adapter type='fc_host' [parent='scsi_hostN'] wwnn='HBA_wwnn'
    wwpn='HBA_wwpn'/>

This should be considered a configuration error since a vHBA
would not be created. In order to use the HBA as the backing the
following XML should be used:

  <adapter type='scsi_host' name='scsi_hostN'/>

So add a check prior to the checkParent call to validate that
the provided wwnn/wwpn resolves to a vHBA and not an HBA.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-07-26 15:28:06 -04:00
Martin Kletzander
e1bafb0099 qemu_cgroup: Remove unnecessary virQEMUDriverPtr arguments
Since commit 2e6ecba1bc, the pointer to the qemu driver is saved in
domain object's private data and hence does not have to be passed as
yet another parameter if domain object is already one of them.

This is a first (example) patch of this kind of clean up, others will
hopefully follow.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2017-07-26 17:47:25 +02:00
John Ferlan
5df1d0d417 test: Use consistent variable names for network test driver APIs
A virNetworkObjPtr will be an 'obj'.

A virNetworkPtr will be a 'net'.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2017-07-26 10:38:01 -04:00
Pavel Hrdina
f444e5b4a2 qemu: privatize _virQEMUCapsCachePriv struct
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2017-07-26 15:36:47 +02:00
Pavel Hrdina
d03de54e7e qemu: switch QEMU capabilities to use virFileCache
The switch contains considerable amount of changes:

  virQEMUCapsRememberCached() is removed because this is now handled
  by virFileCacheSave().

  virQEMUCapsInitCached() is removed because this is now handled by
  virFileCacheLoad().

  virQEMUCapsNewForBinary() is split into two functions,
  virQEMUCapsNewData() which creates new data if there is nothing
  cached and virQEMUCapsLoadFile() which loads the cached data.
  This is now handled by virFileCacheNewData().

  virQEMUCapsCacheValidate() is removed because this is now handled by
  virFileCacheValidate().

  virQEMUCapsCacheFree() is removed because it's no longer required.

  Add virCapsPtr into virQEMUCapsCachePriv because for each call of
  virFileCacheLookup*() we need to use current virCapsPtr.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2017-07-26 15:36:21 +02:00
Pavel Hrdina
f366cfed62 qemu: pass only host arch instead of the whole virCaps
This is a preparation for following patches where we switch to
virFileCache for QEMU capabilities cache

The host arch will always remain the same but virCaps may change.  Now
the host arch is stored while creating new qemu capabilities cache.
It removes the need to pass virCaps into virQEMUCapsCache*() functions.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2017-07-26 15:35:24 +02:00
Pavel Hrdina
731cfd5fe8 qemu: introduce struct _virQEMUCapsCachePriv
This will store private data that will be used by following patches
when switching to virFileCache.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2017-07-26 15:33:59 +02:00
Pavel Hrdina
ac3eb2ab24 util: introduce virFileCache
The new virFileCache will nicely handle the caching logic for any data
that we would like to cache.  For each type of data we will just need
to implement few handlers that will take care of creating, validating,
loading and saving the cached data.

The cached data must be an instance of virObject.

Currently we cache QEMU capabilities which will start using
virFileCache.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2017-07-26 15:31:25 +02:00
Pavel Hrdina
cc1329b627 qemu: we prefer C89 comment styles over C99
Introduced by commit 'a7bc2c8cfd6f'.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2017-07-25 23:10:00 +02:00
Scott Garfinkle
a7bc2c8cfd Generate unique socket file
It's possible to have more than one unnamed virtio-serial unix channel.
We need to generate a unique name for each channel. Currently, we use
".../unknown.sock" for all of them. Better practice would be to specify
an explicit target path name; however, in the absence of that, we need
uniqueness in the names we generate internally.

Before the changes we'd get /var/lib/libvirt/qemu/channel/target/unknown.sock
for each instance of
    <channel type='unix'>
        <source mode='bind'/>
        <target type='virtio'/>
    </channel>

Now, we get vioser-00-00-01.sock, vioser-00-00-02.sock, etc.

Signed-off-by: Scott Garfinkle <seg@us.ibm.com>
2017-07-25 22:38:35 +02:00