Laine Stump [Mon, 31 Jul 2017 05:55:49 +0000 (01:55 -0400)]
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.
Laine Stump [Mon, 31 Jul 2017 04:21:45 +0000 (00:21 -0400)]
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.
Laine Stump [Mon, 31 Jul 2017 03:32:43 +0000 (23:32 -0400)]
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.
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>
So the hostdev manager has some lists to keep track which devices
are active (=assigned to a domain) or inactive. The manager and
its lists are allocated in myInit and freed in myCleanup but one
of them (activeSCSIHostdevs) was missing. Also, the order in
which the cleanup was done doesn't make it easy to spot it,
therefore reoder it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
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>
Andrea Bolognani [Tue, 25 Jul 2017 07:34:53 +0000 (09:34 +0200)]
docs: Add "PCI topology and hotplug" guidelines
For all machine types except i440fx, making a guest hotplug
capable requires some sort of planning. Add some information
to help users make educated choices when defining the PCI
topology of guests.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
If there's no content in <script></script>, the XSTL generator
will turn it into <script/> which is not permitted in XHTML.
Adding a single whitespace is enough to guarantee an explicit
closing tag. Without this, the scripts never get loaded by
the browser.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Sri Ramanujam [Fri, 28 Jul 2017 16:19:19 +0000 (12:19 -0400)]
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.
The website does not look good in a mobile device as the text is
far too small and the layout assumes a wide screen.
Make the style dynamically adapt based on viewport size, so a
mobile device gets a layout more suited to its dimensions,
also changing "Learn" to "Docs"
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
* 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>
Some tests take already prepared domain from previous tests. In
this case, the domain is freed by the first test that doesn't
keep the domain. However, if there's no such test case domain is
leaked.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
virTestCompareToFile: Don't access memory we don't own
After reading the contents of a file some cleanup is performed.
However, the check for it might access a byte outside of the
string - if the file is empty in the first place. Then strlen()
is zero.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Tue, 1 Aug 2017 13:17:51 +0000 (15:17 +0200)]
libxl: Add a test suite for libxl_domain_config generator
The libxl library allows a libxl_domain_config object to be serialized
from/to a JSON string. Use this to allow testing of the XML to
libxl_domain_config conversion process. Test XML is converted to
libxl_domain_config, which is then serialized to json. A json template
corresponding to the test XML is converted to a libxl_domain_config
object using libxl_domain_config_from_json(), and then serialized
back to json using libxl_domain_config_to_json(). The two json
docs are then compared.
Using libxl to convert the json template to a libxl_domain_config
object and then back to json provides a simple way to account for
any changes or additions to the json representation across Xen
releases.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
[update to v3.5.0-rc1, improve error reporting, use /bin/true emulator] Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
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>
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>
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>
Peter Krempa [Wed, 2 Aug 2017 15:23:51 +0000 (17:23 +0200)]
tests: deterministichash: Make hash tables arch-independent
It turns out that our implementation of the hashing function is
endian-dependent and thus if used on various architectures the testsuite
may have different results. Work this around by mocking virHashCodeGen
to something which does not use bit operations instead of just setting a
deterministic seed.
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.
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.
The 'shape' attribute on <a> is used together with a 'coords'
attribute to create hot-zones in image maps. We're not using
image maps so our inclusion of a 'shape' attribute is bogus.
Furthermore this is forbidden in HTML5.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
docs: use UTF-8 instead of HTML entities for decorated letters
We have files which use HTML entities for decorating letters
with unlauts, accents, etc. Other files just use UTF-8
characters directly for this. Remove the HTML entities since
they have no benefit and use UTF-8 instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Some docs pages were using <p> </p> to add arbitrary whitespace
in the page. This is something that should be done by CSS if needed,
but it is not needed here, so delete it.
There was also use of <td> </td> which adds no value at all
when we have CSS to prettify tables.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We already require libxml to be installed, so it is not unreasonable
to require xmllint and xsltproc to be installed too - any platform
with the former will have the latter too.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The HTML pages are currently validated against an XHTML 1.0 DTD.
This makes it impossible to take advantage of features that are
introduced in HTML 5, because they'll fail validation.
There is intentionally no DTD defined for HTML 5, so there's no
alternative to XHTML 1.0 DTD that we could switch to. The only
options are to stick with XHTML 1.0 forever, or drop the DTD
validation, and we pick the latter.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
docs: switch to using 'id' attribute instead of 'name' for links
The 'name' attribute on <a...> elements is deprecated in favour
of the 'id' attribute which is allowed on any element. HTML5
drops 'name' support entirely.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Ján Tomko [Tue, 27 Jun 2017 14:33:22 +0000 (16:33 +0200)]
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.
Ján Tomko [Tue, 27 Jun 2017 14:24:11 +0000 (16:24 +0200)]
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.
Ján Tomko [Tue, 27 Jun 2017 12:04:52 +0000 (14:04 +0200)]
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.
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>
Michal Privoznik [Tue, 25 Jul 2017 15:04:11 +0000 (17:04 +0200)]
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>
Michal Privoznik [Thu, 27 Jul 2017 06:43:40 +0000 (08:43 +0200)]
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>
Julio Faracco [Fri, 28 Jul 2017 21:49:35 +0000 (18:49 -0300)]
tools: virsh: Adding unix socket support to 'domdisplay' command.
This commit adds the unix socket URL support to 'domdisplay' command.
Before, even if an user was using unix socket to define a spice graphics,
the command 'domdisplay' showed that the settings were not supported. Now,
the command shows the proper URL: spice+unix://foo/bar.sock.
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>
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>
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>
Peter Krempa [Wed, 26 Jul 2017 09:06:51 +0000 (11:06 +0200)]
tests: qemumonitorjson: Old and empty test case for node name detection
Prior to qemu 2.5 the node names would not be generated, thus would be
missing from 'query-blockstats' and 'query-named-block-nodes'. Test that
the code correctly detects nothing.
Additionally make sure that a VM without disks does not cause problems.
The test case change is necessary as our test file checker does not play
well with empty files.
Peter Krempa [Fri, 28 Jul 2017 10:37:48 +0000 (12:37 +0200)]
tests: utils: Allow NULL strings to be equal to empty file in virTestCompareToFile
Treat an NULL string equivalent to an empty string in
virTestCompareToFile so that callers don't need to add additional logic
in case when a test produces no output.
Peter Krempa [Fri, 28 Jul 2017 09:49:38 +0000 (11:49 +0200)]
tests: Run virt-aa-helper-test only when the helper was compiled
'virt-aa-helper' is compiled when both WITH_LIBVIRTD and
WITH_SECDRIVER_APPARMOR are defined. The test was run only when
WITH_SECDRIVER_APPARMOR was defined thus causing a build failure when
building without the daemon.
Erik Skultety [Wed, 26 Jul 2017 08:45:11 +0000 (10:45 +0200)]
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>
Peter Krempa [Wed, 26 Jul 2017 19:57:23 +0000 (21:57 +0200)]
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>
Pavel Hrdina [Fri, 21 Jul 2017 17:54:33 +0000 (19:54 +0200)]
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.
Peter Krempa [Wed, 26 Jul 2017 09:06:51 +0000 (11:06 +0200)]
tests: qemumonitorjson: Add test case for node name detection with blockjob
qemu 2.9 returns an extra layer in the backing data if a block job is
active. Add a test case to see whether our code properly detects and
ignores such layer.
The test data was prepared by creating a backing chain of qcow2 images
(with qemu-img and with libvirt's snapshot feature).
One of the layers was then merged back by doing a block-commit:
virsh blockcommit VM hda --top /var/lib/libvirt/images/b
and then a block-copy job was started and kept in synchronized phase:
virsh blockcopy VM hda /tmp/tgt.img --transient job