Up until now we've implicitly relied on the fact that failures
reported from this function were simply ignored, but that's
about to change and so we need a proper mock.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
That's more consistent with our usual naming convention.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This behavior reflects the needs of the QEMU driver and has no
place in a generic module such as virProcess.
Thanks to the changes made with the previous commit, it is now
safe to remove these checks and make all virProcessSetMax*()
functions finally behave the same way.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The current code is written under the assumption that, for all
limits except the core size, asking for the limit to be set to
zero is a no-op, and so the operation is performed
unconditionally.
While this is the behavior we want for the QEMU driver, the
virCommand and virProcess facilities are generic, and should not
implement this kind of policy: asking for a limit to be set to
zero should result in that limit being set to zero every single
time.
Add some checks in the QEMU driver, effectively moving the
policy where it belongs.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently this only happens for the core size, but we want the
behavior to be consistent for other limits as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuProcessLaunch() is the correct place to set process limits,
and in fact is where we were dealing with almost all of them,
but the memory locking limit was handled in
qemuBuildCommandLine() instead for some reason.
The code is rewritten so that the desired limit is calculated
and applied in separated steps, which will help with further
changes, but this doesn't alter the behavior.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Doing this now will make the next changes nicer.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These functions abstract part of the existing logic, which is
the same in all virProcessSetMax*() functions, and changes it
so that which underlying syscall is used depends on their
availability rather than on the context in which they are
called: since prlimit() and {g,s}etrlimit() have slightly
different requirements, using the same one every single time
should make for a more consistent experience.
As part of the change, we also remove the special case for
passing zero to virProcessSetMax*() functions: we have removed
all callers that depended on that functionality in the previous
commit, so this is now safe to do and makes the semantics
simpler.
This commit is better viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, the functions accept either an explicit pid or zero,
in which case the current process should be modified: the latter
might sound like a convenient little feature, but in reality
obtaining the pid of the current process is a single additional
function call away, so it hardly makes a difference.
Removing the few cases in which we're passing zero will allow us
to simplify and improve the functions later.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This allows the VNC client user to perform a shutdown, reboot and reset
of the VM from the host side.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The <graphics type="vnc" .... powerControl="yes"/> option instructs the
VNC server to enable an extension that lets the client perform a
graceful shutdown, reboot and hard reset.
This is enabled by default since it cannot be assumed that the VNC
client user has administrator rights over the guest OS. In the case
where the VNC user is a guest administrator though, it is reasonable
to allow direct power control host side too.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Calling a stub should always result in ENOSYS being raised,
regardless of what arguments are passed to it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We're going to change their behavior, so it's good to have the
current one documented to serve as baseline.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For reasons unknown, when rewriting this code and dropping
libdevmapper I've mistakenly used incorrect length of dm.name. In
linux/dm-ioctl.h the dm_ioctl struct is defined as follows:
#define DM_NAME_LEN 128
struct dm_ioctl {
...
char name[DM_NAME_LEN]; /* device name */
...
};
However, when copying string into this member, DM_TABLE_DEPS was
used, which is defined as follows:
#define DM_TABLE_DEPS _IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl)
After decryption, this results in the following size: 3241737483.
Fixes: 2249455654
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tools depend on keycode generated sources, so declare that as an
explicit dependency, otherwise it might fail with:
../tools/virsh-completer-domain.c:35:10: fatal error: 'virkeynametable_linux.h' file not found
^~~~~~~~~~~~~~~~~~~~~~~~~
Fixes: b0f4cf25a6
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit bbc25f0d03 juggled around some
error reporting. Unfortunately virFirewallApply tries to report the
errno stored in the firewall object and we'd try to do that when the
firewall object is NULL too. Report EINVAL if 'firewall' is NULL.
Found by Coverity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
There's an optimization in virBufferAdd which returns early when the
length of the added string is 0 (given that auto-indent is disabled).
The optimization causes inconsistent behaviour between these two cases:
virBufferAdd(buf, "", 0); // this doesn't initialize the buffer
and
virBufferAdd(buf, "", -1); //this initializes the buffer
Since using an empty string is used to prime the buffer to an empty
string it can be confusing. Remove the optimization.
This fixes such a wrong initialization done in x86FeatureNames.
Note that our code in many places expects that if no virBuffer APIs are
used on a buffer object, then NULL should be retured, so we can't always
prime the buffer to an empty string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In cases we use -1 for failure internally we still must return
EXIT_FAILURE.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Simplify the logic picking which element form to format by using
virBuffers for the partial properties and virXMLFormatElement for
combining them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use an allocated buffer for 'cpu_header' so that g_strdup(_printf) can
be used to fill it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use a dynamic string helper so that we don't have to calculate the
string lengths and then iterate from the rear.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We can remove the check that 'idx' is negative by forcing callers to
pass unsigned numbers, which they do already or have a check that 'idx'
is positive.
This in turn allows us to remove most return value NULL checks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function is used to automatically feed a buffer into a pipe which
can be used by the command to read contents of the buffer.
Rather than passing in a pipe, let's create the pipe inside
virCommandSetSendBuffer and directly associate the reader end with the
command. This way the ownership of both ends of the pipe will end up
with the virCommand right away reducing the need of cleanup in callers.
The returned value then can be used just to format the appropriate
arguments without worrying about cleanup or failure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't fail nowadays. Remove the return value and adjust the
only caller which ensures that @cmd is non-NULL and @fd is positive.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the check and reporting of error from the individual virCommand
APIs into a separate helper. This will aid future refactors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If WITH_PIPE2 is not defined we attempt to set the pipe to nonblocking
operation after they are created. We errorneously rewrote the existing
error message on failure to do so or even reported an error if quiet
mode was requested.
Fixes: ab36f72947
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is constructing an error message from a prefix and the
contents of the qemu log file. Marking just two string modifiers as
translatable is pointless and will certainly confuse translators.
Remove the marking and add a comment which bypasses the
sc_libvirt_unmarked_diagnostics check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that error message formatting doesn't use fixed size buffers we can
drop the math for calculating the maximum chunk of log to report in the
error message and use a round number. This also makes it obvious that
the chosen number is arbitrary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use of VIR_ERROR_MAX_LENGTH is actually misleading to the readers
because it implies that the strings in virError are 1024 bytes at most.
That isn't true at least for the 'message' field as it's constructed
from concatenating the detail string which (was) max 1024 bytes with
the string variant of the error code without limiting to 1024.
Use a local copy for declaring the struct for error transport with a
comment so that's obvious that it's a local decision to use 1k buffers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some error message reporting functions already have allocated buffers
which were used to format the error message, so copying the strings is
redundant.
Extract the internals from 'virRaiseErrorFull' to
'virRaiseErrorInternal' which takes allocated strings as arguments and
steals them, so that callers can reuse the buffers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This was (probably) a relict from times when we cared about OOM
conditions and the possibility to report the error. Nowadays it doesn't
make sense as virRaiseErrorFull will do an allocated copy of the strings
and also concatenate the error message prefix with the detail which
doesn't guarantee that the result will be less than 1024 chars.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use glib functions to do the relative name lookup instead of manual
assembly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Passing 'strlen(src)' for length makes it equivalent to virStrcpy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virStrncpy was called with -1 for length of the copied source which is
equivalent to virStrcpy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We want a (possibly truncated) copy of the full source string so
virStrcpy is a better fit.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
15 out of 72 invocations of virStrcpy(Static) ignore the return value as
it's either impossible to fail or in certain cases a truncated copy is
still good enough. Unfortunately virStrcpy doesn't copy anything in
such case as the checks are done first.
Fix this by using g_strlcpy for the implementation and removing
G_GNUC_WARN_UNUSED_RESULT from the function so that callers can decide
when it's okay.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Surprisingly, we never documented the relationship between
connection URI and the location of qemu.conf. Users might wonder
what qemu.conf is loaded when they are connecting to the session
daemon or embed URI. And what to do if the file doesn't exist for
the URI they're using.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Similarly to the crash workaround:
commit 0db4743645
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Tue Jul 28 16:52:47 2020 +0100
util: avoid crash due to race in glib event loop code
we need to do this in the other event loop as crash in that one was also
reported:
https://bugzilla.redhat.com/show_bug.cgi?id=1931331
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This way it can be used from other places as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
