Commit Graph

31988 Commits

Author SHA1 Message Date
John Ferlan
2f92d59c78 conf: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStoragePoolDef
Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
d2ddcdc1a2 phyp: Resolve memory leak in phypStorageVolCreateXML
The @spdef would be leaked in the normal path.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
6bf74e3199 conf: Rework virStoragePoolDefParseXML
Rather than having an error path, let's rework the code to allocate
and fill into an @def variable and then steal that into @ret when we
are successful leaving just a cleanup: path.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
1f20da92d8 conf: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageVolDef
Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
d04fbe7dab conf: Rework virStorageVolDefParseXML
Rather than having an error path, let's rework the code to allocate
and fill into an @def variable and then steal that into @ret when we
are successful leaving just a cleanup: path.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
e687538d3a storage: Use VIR_STEAL_PTR for gluster volume processing
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
cf46075293 conf: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStoragePoolSource
Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
13395548b2 util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef
Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
6fcc344082 util: Rework virStorageAuthDefCopy
Rather than having an error path, let's rework the code to allocate
and fill into an @authdef variable and then steal that into @ret when
we are successful leaving just a cleanup: path.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
John Ferlan
a98d9daf07 conf,util,qemu: Use VIR_STEAL_PTR for authdef processing
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 08:51:23 -05:00
Ján Tomko
b4a4e8f71a qemu: do not format <usedQMP/> in qemu caps XML
Since commit a7424faff QMP is always used.

Also, commit 932534e8 removed the last use of this apart from:
* parsing/formatting this in the caps cache
* using it as a temporary variable to know when to report an error

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2019-02-12 14:02:38 +01:00
Ján Tomko
0f87356c51 virsh: allow empty targets in cmdDomFSInfo
Ever since the introduction of the guest-get-fsinfo command
in QEMU commit 46d4c572 qga/qapi-schema.json says that
the 'disks' array can possibly be empty. For example when getting
the target list is unsupported:
https://bugzilla.redhat.com/show_bug.cgi?id=1567041

Pass an empty string instead of NULL to vshTableRowAppend to prevent
a mismatched column number.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 13:50:10 +01:00
Ján Tomko
0e7707d830 virsh: use virBufferTrim in cmdDomFSInfo
Add comma after every string and trim the final one.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Ján Tomko
066d91ec79 virsh: do not report error on zero filesystems in cmdDomFSInfo
Use vshPrintExtra to report this message. It is a human-readable
explanation rather than an error.

Also, it is a very special system that runs with no filesystems.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Ján Tomko
023f0672d4 virsh: introduce 'ret' in cmdDomFSInfo
Failing to print the table is also a reason to return failure
and print the reported error.

Switch to the usual pattern where we fall through the cleanup
label right after setting ret to true instead of infering the
return value from the number of filesystems returned.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Ján Tomko
eb3f5a2c6d virsh: do not access uninitialized memory in cmdDomFSInfo
Initialize 'info' to prevent accessing random access memory.

Introduced by commit 3072ded released in 4.8.0.

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

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Ján Tomko
44993d312d virsh: rename ret to rc in cmdDomFSInfo
Leave the 'ret' variable for the current function's return value.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Ján Tomko
443fc056a0 virsh: introduce ninfos variable in cmdDomFSInfo
Do not use 'ret' throughout the whole function to avoid confusion
and comparison of unsigned 'i' against signed 'ret'.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Ján Tomko
b419ea6a90 vsh-table: allow empty columns
Trivially implement this by deleting the bogus check in
vshTableSafeEncode.

Now it returns an empty string for an empty string instead
of returning NULL without setting an error.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Ján Tomko
865b201479 vshtabletest: indent strings with expected output
Indent them by four spaces from the previous line, instead of starting
at columnn zero.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 10:14:53 +01:00
Michal Privoznik
1620bb0a30 tools: Drop support for pre-2.4.0 wireshark
The wireshark-2.4.0 is almost 2 years old now. Assuming anybody
interested in running latest libvirt doesn't run old wireshark,
it is safe to do this. It also simplifies the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 09:22:59 +01:00
Michal Privoznik
dc93997a83 m4: Put wireshark plugin into epan/ directory
Since wirshark-2.5.0 toplevel plugins are no longer loaded. Only
plugins from epan/, wiretap/ or codecs/ subdirs are. Update the
plugin dir we generate. This is safe to do even for older
wiresharks, since they load plugins from there too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 09:22:59 +01:00
Michal Privoznik
4c22fd717b wireshark: Provide registration code for newer wireshark
As advertised in previous commits, wireshark has changed the way
that plugins register. In fact, it has done so two times since
the last time we've touched our code (wireshark v2.5.0 and
v2.9.0). Use the wireshark script from respective releases to
generate newer registration callbacks and put them into our code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 09:22:59 +01:00
Michal Privoznik
a87a75e579 tools: Keep wireshark plugin registration code in git
In order to be able to dissect libvirt protocol the wireshark
plugin needs to be registered. So far this plugin registration
code was generated on every build using a script that was copied
over from wireshark's tools/ directory.

This is suboptimal, because the way that plugins register changes
across wireshark releases. Therefore, let's keep the generated
file in the git, put the command line used to generate the file
into a comment and remove the script.

This solution allows us to put different registration mechanism
into one file (under #ifdef-s) and thus compile with wider range
of wireshark releases.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 09:22:59 +01:00
Michal Privoznik
b76f4dec37 tools: Cleanup packet-libvirt.h
Move the majority of the packet-libvirt.h content into
packet-libvirt.c and expose only register functions which are the
only ones that are not static.

The rationale behind is that packet-libvirt.h will be included
from packet.c and therefore the header file needs to be as clean
as possible.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-02-12 09:22:59 +01:00
Cole Robinson
b79885b7f7 tests: domcaps: Remove dependency on libxl PVUSB support
Mock out libxlCapsHasPVUSB to always return true, so test results
aren't dependent on host libxl version

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-02-11 17:40:44 -05:00
Cole Robinson
b99fdb60b2 libxl: Break out libxlCapsHasPVUSB
No functional change, but this will allow us to mock out the function
in the test suite

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-02-11 17:40:44 -05:00
Cole Robinson
deb3b59b74 tests: Create a shared library with libxl driver
This allows us to mock functions in the libxl driver, like
is already possible for the qemu driver

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-02-11 17:40:44 -05:00
Cole Robinson
49a0bcfdf2 tests: Rename virmocklibxl.c -> libxlmock.c
Every other mock library is named ending in mock.c, move
virmocklibxl.c to follow that pattern

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-02-11 17:40:44 -05:00
Cole Robinson
e00771dfc3 docs: storage: owner/group default to libvirtd UID/GID
Commit fafcc818f changed the docs to say that when creating a
pool directory or file volume with no owner/group specified, they
will be inherited from the parent directory. This isn't correct
now and doesn't seem to have ever been correct

In reality default owner/group is whatever UID/GID libvirtd is
running as

Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-02-08 12:37:06 -05:00
Nikolay Shirokovskiy
d63c82df8b rpc: client: stream: fix multi thread abort/finish
If 2 threads call abort for example then one of them
will hang because client will send 2 abort messages and
server will reply only on first of them, the second will be
ignored. And on server reply client changes the state only
one of abort message to complete, the second will hang forever.
There are other similar issues.

We should complete all messages waiting reply if we got
error or expected abort/finish reply from server. Also if one
thread send finish and another abort one of them will win
the race and server will either abort or finish stream. If
stream is aborted then thread requested finishing should report
error. In order to archive this let's keep stream closing reason
in @closed field. If we receive VIR_NET_OK message for stream
then stream is finished if oldest (closest to queue end) message
in stream queue is finish message and stream is aborted if oldest
message is abort message. Otherwise it is protocol error.

By the way we need to fix case of receiving VIR_NET_CONTINUE
message. Now we take oldest message in queue and check if
this is dummy message. If one thread first sends abort and
second thread then receives data then oldest message is abort
message and second thread won't be notified when data arrives.
Let's find oldest dummy message instead.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-02-08 17:16:00 +01:00
Nikolay Shirokovskiy
fbcb73866b rpc: client stream: dispose private data on stream dispose
If we call virStreamFinish and virStreamAbort from 2 distinct
threads for example we can have access to freed memory.
Because when virStreamFinish finishes for example virStreamAbort
yet to be finished and it access virNetClientStreamPtr object
in stream->privateData.

Also it does not make sense to clear @driver field. After
stream is finished/aborted it is better to have appropriate
error message instead of "unsupported error".

This commit reverts [1] or virNetClientStreamPtr and
virStreamPtr will never be unrefed due to cyclic dependency.
Before this patch we don't have leaks because all execution
paths we call virStreamFinish or virStreamAbort.

[1] 8b6ffe40 : virNetClientStreamNew: Track origin stream

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-02-08 16:51:45 +01:00
Nikolay Shirokovskiy
d962f56fb6 rpc: client: don't set incomingEOF on errors
This mixing errors and EOF condition in one flag is odd.
Instead let's check st->err.code where appropriate.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-02-08 16:51:45 +01:00
Nikolay Shirokovskiy
ad063f6192 rpc: client: incapsulate error checks
Checking virNetClientStreamRaiseError without client lock
is racy which is fixed in [1] for example. Thus let's remove such checks
when we are sending message to server. And in other cases
(like virNetClientStreamRecvHole for example) let's move the check
into client stream code.

virNetClientStreamRecvPacket already have stream lock so we could
introduce another error checking function like virNetClientStreamRaiseErrorLocked
but as error is set when both client and stream lock are hold we
can remove locking from virNetClientStreamRaiseError because all
callers hold either client or stream lock.

Also let's split virNetClientStreamRaiseErrorLocked into checking
state function and checking message send status function. They are
same yet.

[1] 1b6a29c21: rpc: fix race on stream abort/finish and server side abort

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-02-08 16:51:45 +01:00
Nikolay Shirokovskiy
8457fd5034 rpc: add mising locking in virNetClientStreamRecvHole
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-02-08 16:51:45 +01:00
Nikolay Shirokovskiy
4deed5f3c7 rpc: fix propagation of errors from server
Stream server error is not propagated if thread does not have the buck.
In case we have the buck we are ok due to the code added in [1].

Let's check for stream error on all paths. Now we don't need
to raise error in virNetClientCallDispatchStream.

Old code reported error only if the first message in wait
queue awaits reply. It is odd as depends on wait queue
situation. For example if we have only TX
message in queue and in one iteration loop both send the
message and receive error then thread sending TX message did
not receive the error. Next if we have RX message (first)
and TX message (second) in queue and in one iteration
loop both send the TX message and receive error then
thread sending TX message received error. In short
it was inconsistent. Let's report error whenever
we received it and for every type of message as it makes
sense to report errors as early as possible.

[1] 16c6e2b41: Fix propagation of RPC errors from streams

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-02-08 16:51:45 +01:00
Nikolay Shirokovskiy
6709479a2f rpc: remove unused virNetClientSendNoReply
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-02-08 16:51:45 +01:00
Nikolay Shirokovskiy
2fd435b785 rpc: use single function to send stream messages
In next patches we'll add stream state checks to this
function that applicable to all call paths. This is handy
place because we hold client lock here.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-02-08 16:51:45 +01:00
Nikolay Shirokovskiy
a5445a3706 rpc: fix race on stream abort/finish and server side abort
Stream abort/finish can hang because we can receive abort message
from server and yet sent abort/finish message to server. The latter
will not be answered ever because after server sends abort message
it forgets the stream and messages for unknown stream are simply ignored.

We check for stream error at the very beginning of remoteStreamFinish/remoteStreamAbort
but stream error can be set after the check in another thread operating
on stream. Let's check for stream error under client lock similar
to what's done in [1].

[1] 833b901cb: stream: Check for stream EOF

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-02-08 16:51:45 +01:00
Andrea Bolognani
df2b31ffea qemu: Unify style for qemuDomain*()
These functions do mostly the same things, and it would be
preferrable if they did them in mostly the same ways. This
also fixes a few violations to our code style guidelines.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:25 +01:00
Andrea Bolognani
0440d5e90f qemu: Move qemuDomainSupportsPCI() to qemu_domain
The function operates on a virDomainDef and is not tied to
device address assignment in any way, so it makes more sense
for it to live along with qemuDomainIs*() and the like.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:22 +01:00
Andrea Bolognani
68e26a9ea2 qemu: Make most qemuDomainMachine*() functions static
Ideally we'd make all of them static, but there are a few
cases where we don't have a virDomainDef instance handy and
so they are the only option.

For the few ones we're forced to keep exporting, document
through comments that the alternative is preferred.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:20 +01:00
Andrea Bolognani
8a8b14718e qemu: Remove useless ARCH_IS_X86() call
Now that we have added architecture checks to all
qemuDomainIs*() functions, we no longer need to perform the
same checks separately.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:18 +01:00
Andrea Bolognani
aa2da99ee8 qemu: Add arch checks to qemuDomainMachine*()
There is very little overlap in the machine types available
on different architectures, so broadly speaking checking the
machine type is usually enough; regardless, it's better to
check the architecture as well.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:16 +01:00
Andrea Bolognani
9a5ce5cb29 qemu: Add arch parameter to qemuDomainMachine*()
We want the signatures to be consistent, and also we're
going to start using the additional parameter next.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:14 +01:00
Andrea Bolognani
4028d7a46a qemu: Move functions around
Make sure related functions, eg. all qemuDomainIs*(), are
close together instead of being sprinkled throughout both
the header and implementation file, and also that all
qemuDomainMachine*() functions are declared first since
we're going to make a bunch of them static later on.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:12 +01:00
Andrea Bolognani
616beb17d4 qemu: Use more specific prefixes
While the chances of the current checks resulting in false
positives are basically zero, it's still nicer to check for
the full prefix instead of the prefix's prefix.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:10 +01:00
Andrea Bolognani
629fdca0d0 qemu: Remove redundant condition
No need to check whether we're dealing with a pSeries guest
twice within just a few lines.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 16:16:05 +01:00
John Ferlan
73b9b8fe36 util: Use a semicolon for all VIR_DEFINE_AUTOPTR_FUNC
For consistency, let's use the semicolon for all definitions.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2019-02-08 06:57:23 -05:00
Erik Skultety
5846179a2e docs: Update the AMD SEV's spec URL
Luckily, the new URL still points to the same location, the only change
is in the document name where an escaped space (%20) was replaced by an
underscore.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2019-02-08 12:50:30 +01:00