John Ferlan [Sun, 3 Nov 2019 12:12:58 +0000 (07:12 -0500)]
conf: Remove ATTRIBUTE_NONNULL for virDomainQemuMonitorEventNew
Commit 17561eb36 modified the logic to check "if (!event)" for an
attribute that was not supposed to be passed as NULL. This causes
the static checker/Coverity build to fail. Since the check is made,
alter the header.
Also add an error message since returning -1 without some sort of
error message as previously would have happened with the failed
VIR_STRDUP so that the eventual error doesn't get the default
for some reason message.
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The "Security" section has been used in the past, so we're only
documenting existing behavior; the "Packaging changes" will be
used in the next commit, as well as in future releases when we
make more changes that are relevant to packagers, such as the
switch to Meson.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Peter Krempa [Thu, 24 Oct 2019 11:02:41 +0000 (13:02 +0200)]
util: buffer: Reimplement virBuffer internals using glib's GString
GString is surprisingly similar to what libvirt was doing painstakingly
manually. Yet it doesn't support the automatic indentation features we
use for XML so we rather keep those in form of virBuffer using GString
internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 24 Oct 2019 11:23:32 +0000 (13:23 +0200)]
util: buffer: Properly URLencode strings
According to rfc3986:
2.3. Unreserved Characters
Characters that are allowed in a URI but do not have a reserved
purpose are called unreserved. These include uppercase and lowercase
letters, decimal digits, hyphen, period, underscore, and tilde.
URIs that differ in the replacement of an unreserved character with
its corresponding percent-encoded US-ASCII octet are equivalent: they
identify the same resource. However, URI comparison implementations
do not always perform normalization prior to comparison (see Section
6). For consistency, percent-encoded octets in the ranges of ALPHA
(%41-%5A and %61-%7A), DIGIT (%30-%39), hyphen (%2D), period (%2E),
underscore (%5F), or tilde (%7E) should not be created by URI
producers and, when found in a URI, should be decoded to their
corresponding unreserved characters by URI normalizers.
Thus we must not include few other characters which don't match
c_isalpha to conform to the rules.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 24 Oct 2019 10:51:24 +0000 (12:51 +0200)]
util: virbuffer: Remove @dynamic from virBufferGetIndent
After the conversion of all callers that would pass true as @dynamic to
a different function we can remove the unused argument now.
Additionally modify the return type to 'size_t' as indentation can't be
negative and remove checks whether @buf is passed as it's caller's duty
to do so.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 24 Oct 2019 10:29:12 +0000 (12:29 +0200)]
util: buffer: Split getting of effective indent out of virBufferGetIndent
The function basically does two very distinct things depending on a
bool. As a first step of conversion split out the case when @dynamic is
true and implement it as a new function and convert all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 24 Oct 2019 07:25:20 +0000 (09:25 +0200)]
util: buffer: Simplify handling of indent overflows
Rather than setting usage error truncate the indentation level. Having
the output string misformated is way more useful to figure out where the
error lies rather than reporting an error after a giant formatter
function.
In testBufAutoIndent we now validate that the indentation is truncated
and testBufAddBuffer2 is removed since it became bogus.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 24 Oct 2019 06:49:57 +0000 (08:49 +0200)]
util: buffer: Simplify escape buffer allocations
Replace combinations of xalloc_oversized and VIR_ALLOC_N_QUIET by using
g_malloc0_n which does the checking internally.
This conversion is done with a semantic difference and slightly higher
memory requirements as I've opted to allocate one chunk more than
necessary rather than trying to accomodate the NUL byte separately.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 22 Oct 2019 14:10:53 +0000 (16:10 +0200)]
remote: Use g_new0 to allocate 'remote_string' in event RPC handlers
Few events emit optional strings. We need to allocate the container for
it first. Note that remote_nonnull_string is used as the type as the
internal part of the string is nonnull if the container is present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ACKed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Tue, 22 Oct 2019 11:46:37 +0000 (13:46 +0200)]
remote: dispatch: Remove return value from make_nonnull_* helpers
After conversion to g_strdup, the helpers now always return success.
Remove the return value to simplify the callers.
Note that many occurrences of these is in the code generated by
gendispatch.pl. Since gendispatch aggregates many cases together an
incremental conversion would require more invasive changes to
gendispatch for the time of conversion which doesn't make sense.
Also in many cases the helper was the last place where the 'error:'
label was used and thus also those conversions must be included in this
patch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ACKed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Mon, 7 Oct 2019 14:19:34 +0000 (16:19 +0200)]
qemu: monitor: Add helper for generating data for block bitmap merging
Introduce qemuMonitorTransactionBitmapMergeSourceAddBitmap which adds
the appropriate entry into a virJSONValue array to be used with
qemuMonitorTransactionBitmapMerge. Bitmap merging supports two possible
formats and this new helper implements the more universal one specifying
also the source node name.
In addition use the new helper in the testQemuMonitorJSONTransaction
test.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 3 Oct 2019 12:35:58 +0000 (14:35 +0200)]
qemu: checkpoint: Extract finalizing steps of checkpoint creation
Extract the linking and saving bits of checkpoint creation into
qemuCheckpointCreateFinalize so that qemuCheckpointCreateXML is a bit
simpler and also makes it reusable in the backup code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 1 Oct 2019 13:04:33 +0000 (15:04 +0200)]
qemu: checkpoint: Split out checkpoint creation code
Separate out individual steps of creating a checkpoint from
qemuCheckpointCreateXML into separate functions. This makes the function
more readable and understandable and also some of the new functions will
be reusable when we will be creating a checkpoint along with a backup
in the upcoming patches.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 1 Oct 2019 06:10:18 +0000 (08:10 +0200)]
conf: checkpoint: Don't clear current checkpoint when redefining
If we are updating the current checkpoint when redefining by mentioning
the current checkpoint as a parent of the newly redefined one we don't
have to clear it first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 1 Oct 2019 05:22:14 +0000 (07:22 +0200)]
conf: checkpoint: Don't clear current checkpoint when redefining an existing one
There's no point in clearing the current checkpoint when we are just
changing the definition of the current checkpoint as by the virtue of the
'update_current' flag the same checkpoint would become current in
qemuCheckpointCreateXML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 1 Oct 2019 06:00:26 +0000 (08:00 +0200)]
conf: Don't reuse variable for different object in virDomainCheckpointRedefinePrep
The 'other' variable was used to store the parent of the redefined
checkpoint and then the existing version of the currently redefined
checkpoint. Make it less confusing by adding a 'parent' variable for the
first case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 1 Oct 2019 05:22:14 +0000 (07:22 +0200)]
conf: snapshot: Don't clear current snapshot when redefining an existing one
There's no point in clearing the current snapshot when we are just
changing the definition of the current snapshot as by the virtue of the
'update_current' flag the same snapshot would become current in
qemuDomainSnapshotCreateXML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Tue, 22 Oct 2019 15:31:54 +0000 (17:31 +0200)]
tests/domaincaps: Use testQemuCapsIterate()
Now that the only data we need for fully testing a QEMU binary is
the (version, arch) combo, we can stop providing that information
ourselves and instead rely on testQemuCapsIterate() automatically
picking up new input files as they are added to the repository,
the same way the qemucapabilities and qemucaps2xml tests already
behave.
Unsurprisingly, this change results in a bunch of extra output
files being created, significantly expanding our test coverage.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Andrea Bolognani [Tue, 22 Oct 2019 15:16:26 +0000 (17:16 +0200)]
tests/domaincaps: Make test matrix programmatic
For each QEMU version there are usually several different,
architecture-dependedn scenarios that we're interested in testing;
however, since the test matrix has to be explicitly created by
calling DO_TEST_QEMU() multiple times with different arguments, we
end up with spotty coverage.
Fix this by implementing the arch-specific rules in code, which
result in the full coverage for a (version, arch) combo being
automatically achieved with a single call to DO_TEST_QEMU().
Unsurprisingly, this change results in a bunch of extra output
files being created.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Andrea Bolognani [Tue, 22 Oct 2019 14:56:40 +0000 (16:56 +0200)]
tests/domaincaps: Don't require redundant information
The full name of the test case, as well as the name of the QEMU
binary and corresponding capabilities file, can all be derived
from other information passed to the test, so there's no point in
asking the user to provide them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Andrea Bolognani [Tue, 22 Oct 2019 14:27:21 +0000 (16:27 +0200)]
tests/domaincaps: Move most of DO_TEST_QEMU() into a function
Macros become less and less appealing the more work you perform
inside them: DO_TEST_QEMU() has arguably already crossed that
threshold, and we're going to add even more code later on.
While factoring the code out of the macro, convert it to use the
GLib string manipulation functions and take advantage of autofree.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The usual convention is to use ${foo}test.c for the test program
itself and either ${foo}data/ or ${foo}outdata/, depending on
whether it contains both input and output files or only the latter,
for the corresponding data directory.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Andrea Bolognani [Tue, 22 Oct 2019 14:08:10 +0000 (16:08 +0200)]
tests: testQemuCapsIterate: Pass prefix and version to callback
Right now we're passing a "base" string that contains both,
separated by an underscore. Some changes that we're going to
introduce later will require us to have the version number on its
own, and instead of delegating the task of splitting the two apart
to the callback it make more sense to perform it upfront.
This change results in quite a bit of churn because we're now
using the version number only, without the prefix, to calculate
the dummy microcodeVersion.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Andrea Bolognani [Tue, 22 Oct 2019 13:44:37 +0000 (15:44 +0200)]
tests: testQemuCapsIterate: Pass suffix to callback
Right now users need to hardcode the suffix, which is not a big
deal since they're the ones who passed it to testQemuCapsIterate()
in the first place; however, since we're already passing most of
the information to the callback and we're going to add more later
on, it makes sense to be consistent and pass the suffix too.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
If files whose name doesn't follow the expected format are added
to the repository, it's better to make the test suite fail than to
silently ignore them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Pavel Hrdina [Thu, 24 Oct 2019 10:11:18 +0000 (12:11 +0200)]
build: src: fix libtool dependency issue
Libtool gets a wrong order of arguments of libraries to install and it
fails when installing libvirt-admin.so that libvirt.so is not yet
installed. Caused by commit <3097282d8668693eb4b7c3fb1b4fe5b474996b9c>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Thu, 17 Oct 2019 08:10:10 +0000 (10:10 +0200)]
Drop needless ret variable
In few places we have the following code pattern:
int ret;
... /* @ret is not accessed here */
ret = f(...);
return ret;
This pattern can be written less verbose:
...
return f(...);
This patch was generated with following coccinelle spatch:
@@
type T;
constant C;
expression f;
identifier ret;
@@
-T ret = C;
... when != ret
-ret = f;
-return ret;
+return f;
Afterwards I needed to fix a few places, e.g. comment in
virDomainNetIPParseXML() was removed too because coccinelle
thinks it refers to @ret while in fact it doesn't. Also in few
places it replaced @ret declaration with a few spaces instead of
removing the line. But nothing terribly wrong.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Sat, 19 Oct 2019 10:09:32 +0000 (12:09 +0200)]
src: Don't rely on virAsprintf() returning string length
In a few places our code relies on the fact that virAsprintf()
not only prints to allocated string but also that it returns the
length of that string. Fortunately, only few such places were
identified:
In case of virNWFilterSnoopLeaseFileWrite() and virFilePrintf()
we can use strlen() right after virAsprintf() to calculate the
length. In case of virDoubleToStr() it's only caller checks for
error case only, so we can limit the set of returned values to
just [-1, 0].
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pavel Hrdina [Fri, 18 Oct 2019 15:21:06 +0000 (17:21 +0200)]
docs: generate files into build dir and stop distributing them
Historically we did not support VPATH builds and everything was
generated into source directory. The introduction of VPATH builds did
not changed the way how our documentation is handled.
This patch changes the rules to generate everything into build
directory and stops distributing generated files in order to have
properly separated VPATH builds.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pavel Hrdina [Tue, 15 Oct 2019 10:41:29 +0000 (12:41 +0200)]
build: move admin code into admin directory
There is no need to have the libvirt-admin.so library definition in the
src directory. In addition the library uses directly code from admin
sub-directory so move the remaining bits there as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Wed, 28 Aug 2019 09:41:49 +0000 (11:41 +0200)]
domain_conf: Relax SCSI addr used check
In domain_conf.c we have virDomainSCSIDriveAddressIsUsed()
function which returns true or false if given drive address is
already in use for given domain config or not. However, it also
takes a shortcut and returns true (meaning address in use) if the
unit number equals 7. This is because for some controllers this
is reserved address. The limitation comes mostly from vmware and
applies to lsilogic, buslogic, spapr-vscsi and vmpvscsi models.
On the other hand, we were not checking for the maximum unit
number (aka LUN number) which is also relevant and differs from
model to model.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Tue, 10 Sep 2019 14:45:47 +0000 (16:45 +0200)]
domain_conf: Make virDomainDeviceFindSCSIController accept virDomainDeviceDriveAddress struct
So far, the virDomainDeviceFindSCSIController() takes
virDomainDeviceInfo structure which is an overkill. It assumes
that the passed structure is type of
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE which is not obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Ján Tomko [Fri, 18 Oct 2019 21:12:19 +0000 (23:12 +0200)]
docs: hacking: mention GLib alternatives of libvirt string allocation macros
Document the preferred alternatives to existing libvirt macros for
allocating strings. These cannot be deleted just yet because
converting them will require a lot of work.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Ján Tomko [Fri, 18 Oct 2019 21:12:19 +0000 (23:12 +0200)]
docs: hacking: mention GLib alternatives of libvirt allocation macros
Document the preferred alternatives to existing libvirt macros for
memory allocation. These cannot be deleted just yet because
converting them will require a lot of work.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Ján Tomko [Fri, 18 Oct 2019 20:57:23 +0000 (22:57 +0200)]
docs: hacking: emphasize some sections
Namely:
* holding up the first-time patch submissions for moderation,
which might cause first-time submitters to question the process
* not CC-ing individual developers
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Thu, 10 Oct 2019 13:02:00 +0000 (15:02 +0200)]
qemu: block: Don't query monitor in qemuBlockStorageSourceCreateDetectSize
Calling the monitor was convenient for the implementation in
qemuDomainBlockCopyCommon, but causes the snapshot code to call
query-named-block-nodes for every disk.
Fix this by removing the monitor call from
qemuBlockStorageSourceCreateDetectSize so that the data can be reused in
loops.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ACKed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Wed, 9 Oct 2019 12:13:10 +0000 (14:13 +0200)]
qemu: monitor: Introduce new interface to query-named-block-nodes
Retrieve data for individual block nodes in a hash table. Currently only
capacity and allocation data is extracted but this will be extended in
the future.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ACKed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Wed, 9 Oct 2019 13:34:54 +0000 (15:34 +0200)]
util: hash: Add new constructor 'virHashNew'
Add a simpler constructor for hash tables which specifically does not
require specifying the initial hash size and uses simpler freeing
function.
The initial hash table size usually is not important as the hash table
is growing when it reaches certain number of entries in one bucket.
Additionally many callers pass in a random small number for ad-hoc table
use so using a central one will simplify things.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ACKed-by: Eric Blake <eblake@redhat.com>
projects / libvirt.git / log