Use automatic memory freeing and don't check return value of
virJSONValueNewString as it can't fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The parent object takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr for the JSON value objects and remove the cleanup label
and inline freeing of objects.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree for the pointer of the added object and remove the NULL
checks for values returned by virJSONValueNew* (except
virJSONValueNewNumberDouble) since they can't fail nowadays.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We know the exact number of keys or array members for the copied objects
so we can pre-allocate the arrays rather than inserting into them in a
loop incurring realloc copy penalty.
Also virJSONValueCopy now can't fail since all of the functions
allocating the different cases use just g_new/g_strdup internally so we
can remove the NULL checks from the recursive calls.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function takes ownership of @value on success so the proper
semantics will be to clear out the @value pointer. Convert @value to a
double pointer to do this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The functions report errors already and the error can nowadays only
happen on programmer errors (if the passed virJSONValue isn't an
object), which won't happen. Remove the reporting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For incremental backup we need QEMU_CAPS_BLOCKDEV,
QEMU_CAPS_BLOCKDEV_REOPEN, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Preserve block dirty bitmaps after migration with
QEMU_MONITOR_MIGRATE_NON_SHARED_(DISK|INC).
This patch implements functions which offer the bitmaps to the
destination, check for eligibility on destination and then configure
source for the migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In case when the block migration job required temporary bitmaps for
merging the appropriate checkpoints we need to clean them up when
cancelling the job. On success we don't need to do that though as the
bitmaps are just temporary thus are not written to disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add status XML infrastructure for storing a list of block dirty bitmaps
which are temporarily used when migrating a VM with
VIR_MIGRATE_NON_SHARED_DISK for cleanup after a libvirtd restart during
migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
'qemuMigrationCookieBlockDirtyBitmapsMatchDisks' maps the bitmaps from
the migration cookie to actual disk objects definition pointers.
'qemuMigrationCookieBlockDirtyBitmapsToParams' converts the bitmap
definitions from the migration cookie into parameters for the
'block-bitmap-mapping' migration parameter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In cases where we are copying the storage we need to ensure that also
bitmaps are copied properly. This patch adds migration cookie XML
infrastructure which will allow the migration sides reach consensus on
which bitmaps to migrate.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add the migration capability flag and the propagation of the
corresponding mapping configuration. The mapping will be produced from
the bitmaps on disk depending on both sides of the migration and the
necessity to perform merges.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
There's no need in the cleanup steps to invoke a transaction to delete a
single bitmap.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The non-transaction wrapper is useful for code paths which want to
delete individual bitmaps or for cleanup after a failed job where we
want to attempt to delete every bitmap individually to prevent a failure
from cleaning up the rest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use the new format when pre-creating the image for the user. Users
wishing to use the legacy format can always provide their own images or
use shared storage.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Such images don't support stuff like dirty bitmaps. Note that the
synthetic test for detecting bitmaps is used as an example to prevent
adding additional test cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The capability represents qemu's ability to setup mappings for migrating
block dirty bitmaps and is based on presence of the 'transform' property
of the 'block-bitmap-mapping' property of 'migrate-set-parameters' QMP
command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This patch partially reverts commit 5cde9dee where the qemuExtDevicesStop()
was moved to a location before the QEMU process is stopped. It may be
alright to tear down some devices before QEMU is stopped, but it doesn't work
for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop
it before libvirt may try to clean it up. So this patch moves the
virFileDeleteTree() calls after the call to qemuExtDevicesStop() so that the
pid file of virtiofsd is not deleted before that call.
Afftected libvirt versions are 6.10 and 7.0.
Fixes: 5cde9dee8c
Cc: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A VM defined similar to:
...
<features><kvm><hint-dedicated state='on'/></kvm></features>
<cpu mode="host-model"/>
...
is currently invalid, as hint-dedicated is only allowed if cpu mode
is host-passthrough or maximum. This restriction is unnecessary, see
https://bugzilla.redhat.com/show_bug.cgi?id=1857671
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Stay true to the name of the function and clear the pointer
after freeing it.
This also silences a bogus Coverity report about a double
free in qemuMonitorGetCPUInfo where qemuMonitorCPUInfoClear
is called right after allocating a new qemuMonitorCPUInfo
to fill out the non-zero defaults.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The comment and the caller assume virQEMUSaveDataNew only steals
domXML on success, but it is copied even on failure.
Also remove the misleading g_steal_pointer call on a local variable.
Reported by coverity.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The switch to g_auto left this one call behind.
Reported by Coverity.
Fixes: 4ab0d1844a
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Commit 76f4788932 made qemuNodeDeviceDetachFlags() unusable due to an
'if then else if' chain that will always results in a 'return -1',
regardless of 'driverName' input.
Found by Coverity.
Fixes: 76f4788932
Reported-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Just return when alias is null and Remove the 'ret' variable.
Signed-off-by: Yi Li <yili@winhong.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This script works under two specific conditions. For each opened file,
search for all functions that has ACL calls and store them, and see
if there is a vir*DriverPtr struct declared in it. For each implementation
found, check if there is an ACL verification inside it, and error out if
none was found. The script also supports the concept of stub, where another
function takes the responsibility for the ACL call instead of the
original API.
Unfortunately this is not enough to cover the new scenario we have now,
with domain_driver.c containing helper functions that execute the ACL
calls. The script does not store state between files because, until now,
it wasn't needed to - APIs and stubs and vir*DriverPtr declarations were
always in the same file. Also, the script will not check for ACL in functions
that does not belong to a vir*DriverPtr interface. What we have now in
domain_driver.c breaks both assumptions: the functions are in a different
file, and there is no vir*DriverPtr being implemented in the file that
uses these functions.
This patch changes check-aclrules.py to accomodate this scenario. The helpers
that have ACL checks are stored beforehand in aclFuncHelpers, allowing other
files to use them to recognize a stub situation. In case the current file
being analyzed is domain_driver.c itself, we'll do a manual check using
aclFuncHelpers to verify that these functions indeed have ACL checks.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceDetachFlags() and qemuNodeDeviceDetachFlags() are mostly
equal, aside from how the virHostdevmanager pointer is retrieved and
the PCI stub driver used.
Now that the PCI stub driver verification is done early in both functions,
we can use the virDomainDriverNodeDeviceDetachFlags() helper to reduce
code duplication between them. 'driverName' is checked inside the helper
to set the appropriate stub driver.
The helper is named with the 'Flags' suffix, even when the helper itself
isn't receiving the flags from the callers, to be compliant with the
ACL function virNodeDeviceDetachFlagsEnsureACL() that is being called
inside it and was called from the original functions. Renaming the helper
would implicate in renaming REMOTE_PROC_NODE_DEVICE_DETACH_FLAGS, and all the
related structs inside remote_protocol.x, to be compliant with the ACL
rules.
This is not being checked at this moment, but we'll fix check-aclrules.py to
verify all the helpers that calls ACL functions in domain_driver.c shortly.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceReAttach() and qemuNodeDeviceReAttach() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.
Put the common code into virDomainDriverNodeDeviceReAttach() to reduce
code duplication.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will use g_autoptr() with virNodeDevicePtr for cleanups.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxlNodeDeviceReset() and qemuNodeDeviceReset() are mostly equal,
differing only how the virHostdevManager pointer is retrieved.
Put the common code into virDomainDriverNodeDeviceReset() to reduce
code duplication.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Setting the system time backward would lead to a
multiplication overflow in function virKeepAliveStart.
The function virKeepAliveTimerInternal got the same bug too.
Backtrace below:
#0 0x0000ffffae898470 in raise () from /usr/lib64/libc.so.6
#1 0x0000ffffae89981c in abort () from /usr/lib64/libc.so.6
#2 0x0000ffffaf9a36a8 in __mulvsi3 () from /usr/lib64/libvirt.so.0
#3 0x0000ffffaf8fd9e8 in virKeepAliveStart (ka=0xaaaaf954ce10, interval=interval entry=0,
count=count entry=0) at ../../src/rpc/virkeepalive.c:283
#4 0x0000ffffaf908560 in virNetServerClientStartKeepAlive (client=0xaaaaf954cbe0)
at ../../src/rpc/virnetserverclient.c:1628
#5 0x0000aaaac57eb6dc in remoteDispatchConnectSupportsFeature (server=0xaaaaf95309d0,
msg=0xaaaaf9549d90, ret=0xffff8c007fc0, args=0xffff8c002e70, rerr=0xffff9ea054a0,
client=0xaaaaf954cbe0) at ../../src/remote/remote_daemon_dispatch.c:5063
#6 remoteDispatchConnectSupportsFeatureHelper (server=0xaaaaf95309d0, client=0xaaaaf954cbe0,
msg=0xaaaaf9549d90, rerr=0xffff9ea054a0, args=0xffff8c002e70, ret=0xffff8c007fc0)
at ./remote/remote_daemon_dispatch_stubs.h:3503
#7 0x0000ffffaf9053a4 in virNetServerProgramDispatchCall(msg=0xaaaaf9549d90, client=0xaaaaf954cbe0,
server=0x0, prog=0xaaaaf953a170) at ../../src/rpc/virnetserverprogram.c:451
#8 virNetServerProgramDispatch (prog=0xaaaaf953a170, server=0x0, server entry=0xaaaaf95309d0,
client=0xaaaaf954cbe0, msg=0xaaaaf9549d90) at ../../src/rpc/virnetserverprogram.c:306
#9 0x0000ffffaf90a6bc in virNetServerProcessMsg (msg=<optimized out>, prog=<optimized out>,
client=<optimized out>, srv=0xaaaaf95309d0) at ../../src/rpc/virnetserver.c:137
#10 virNetServerHandleJob (jobOpaque=0xaaaaf950df80, opaque=0xaaaaf95309d0)
at ../../src/rpc/virnetserver.c:154
#11 0x0000ffffaf812e14 in virThreadPoolWorker (opaque=<optimized out>)
at ../../src/util/virthreadpool.c:163
#12 0x0000ffffaf81237c in virThreadHelper (data=<optimized out>) at ../../src/util/virthread.c:246
#13 0x0000ffffaea327ac in ?? () from /usr/lib64/libpthread.so.0
#14 0x0000ffffae93747c in ?? () from /usr/lib64/libc.so.6
(gdb) frame 3
#3 0x0000ffffaf8fd9e8 in virKeepAliveStart (ka=0xaaaaf954ce10, interval=interval entry=0,
count=count entry=0) at ../../src/rpc/virkeepalive.c:283
283 timeout = ka->interval - delay;
(gdb) list
278 now = time(NULL);
279 delay = now - ka->lastPacketReceived; <='delay' got a negative value
280 if (delay > ka->interval)
281 timeout = 0;
282 else
283 timeout = ka->interval - delay;
284 ka->intervalStart = now - (ka->interval - timeout);
285 ka->timer = virEventAddTimeout(timeout * 1000, virKeepAliveTimer, <= multiplication overflow
286 ka, virObjectFreeCallback);
287 if (ka->timer < 0)
(gdb) p now
$2 = 18288001
(gdb) p ka->lastPacketReceived
$3 = 1609430405
Signed-off-by: BiaoXiang Ye <yebiaoxiang@huawei.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
All of these strings are allocated once, freed once, and are never
returned out of the function where they are declared.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
* src/util/virsocket.c (virSocketRecvFD): Set msg.msg_controllen as documented
in the man pages.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
According to meson.build the minimal version of curl needed is
7.18.0 which was released in January 2008. If the minimal version
is bumped to 7.19.1 (released in November 2008) we can drop some
workarounds because this newer version provides APIs we need.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
These are all cases when 1) the pointer is passed by reference from
the caller (ie.e. **) and expects it to be NULL on return if there is
an error, or 2) the variable holding the pointer is being checked or
re-used in the same function, but not right away.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
switching to g_autofree left many cleanup: sections empty.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Or when it will be immediately have a new value assigned to it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
mimeType is initialized to NULL, and then only set in one place, just
before a check (not involving mimeType) that then VIR_FREEs mimeType
if it fails. If we just reorder the code to do the check prior to
setting mimeType, then there won't be any need to VIR_FREE(mimeType)
on failure (because it will already be empty/NULL).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If we put the potential return string into the g_autofreed tmpResult,
and the move it to the returned "result" only as a final step ater, we
can avoid the need to explicitly VIR_FREE (or g_free) on failure.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Although the three functions esxFreePrivate(), esxFreeStreamPrivate(),
and esxUtil_FreeParsedUri() are calling VIR_FREE on *object, and so in
theory the caller of the function might rely on "object" (the free
function's arg) being set to NULL, in practice these functions are
only called from a couple places each, and in all cases the pointer
that is passed is a local variable, and goes out of scope almost
immediately after calling the Free function, so it is safe to change
VIR_FREE() into g_free().
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
volumeName was defined at the top of the function, then a new string
was assigned to it each time through a loop, but after the first
iteration of the loop, the previous string wasn't freed before
allocating a new string the next time. By reducing the scope of
volumeName to be just the loop, and making it g_autofree, we eliminate
the leak.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These strings were being VIR_FREEd multiple times because they were
defined at the top of a function, but then set each time through a
loop. But they are only used inside that loop, so they can be
converted to use g_autofree if their definition is also placed inside
that loop.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
All of these strings are allocated once, freed once, and are never
returned out of the function where they are created, used, and are
freed.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
All callers are now using the on|off syntax, so yes|no is a unreachable
code path.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU has long accepted many different values for boolean properties, but
set accepted has been different depending on which QEMU parser you hit.
The on|off values were supported by all QEMU parsers. The yes|no, y|n,
true|false values were only partially supported:
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html
Thus we should standardize on on|off everywhere since that is most
widely supported in QEMU.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU has long accepted many different values for boolean properties, but
set accepted has been different depending on which QEMU parser you hit.
The on|off values were supported by all QEMU parsers. The yes|no, y|n,
true|false values were only partially supported:
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html
Thus we should standardize on on|off everywhere since that is most
widely supported in QEMU.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.
The long format has been supported with -vnc since the change to use
QemuOpts in 2.2.0, so we check based on the new capability flag.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This was introduced in QEMU 2.2.0, and is visible by -vnc appearing in
the "query-command-line-options" data.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When virDomainGetFSInfo() is called over a QEMU/KVM domain it
results into calling of 'guest-get-fsinfo' guest agent command to
which it replies with info on guest (mounted) filesystems. When
filling return structure we also try to do basic lookup and
translate guest agent provided disk address into disk target (as
seen in domain XML). This can of course fail - guest can have
variety of disks not recorded in domain XML (iSCSI, scsi_debug,
NFS to name a few). If that's the case, a debug message is logged
and no disk target is added into the return structure.
However, due to the way our code is written the caller is led to
believe that the target was added into the structure. This may
lead to a situation where the array of disk targets (strings)
contains NULL. But our RPC structure says the array contains only
non-NULL strings. This results in somewhat 'cryptic' (at least to
users) error message:
error: Unable to get filesystem information
error: Unable to encode message payload
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919783
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous commit, the freeing of @info_ret inside of
virDomainFSInfoFormat() looks like this:
for () {
if (info_ret)
virDomainFSInfoFree(info_ret[i]);
}
It is needless to compare @info_ret against NULL in each
iteration. We can switch the order and do the comparison first
followed by the loop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When qemuDomainGetFSInfo() is called it calls
qemuDomainGetFSInfoAgent() which executes 'guest-get-fsinfo'
guest agent command, parses returned JSON and returns an array of
qemuAgentFSInfo structures (well, pointers to those structs).
Then it grabs a domain job and tries to do some matching of guest
returned info against domain definition. This matching is done in
virDomainFSInfoFormat() which also frees the array of
qemuAgentFSInfo structures allocated earlier.
But this is not just. If acquiring the domain job fails (or
domain activeness check executed right after that fails) then
virDomainFSInfoFormat() is not called, leaking the array of
structs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As the very first thing, this function checks whether the number
of items inside @agentinfo array is not negative. This is
redundant as the only caller - qemuDomainGetFSInfo() already
checked for that and would not even call this function if that
was the case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.
The long format has been supported with -spice since at least 1.5.3,
so we don't need to check for it.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.
The long format has been supported with -chardev since at least 1.5.3,
so we don't need to check for it.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The -2 value is misleading because if 'qemuAgentFSFreeze' fails it
doesn't necessarily mean that the command was sent to the agent.
Since callers don't care about the -2 value specifically, remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
If we didn't freeze any filesystems we should not even attempt thawing
them. Additionally 'guest-fsfreeze-freeze' fails if the filesystems are
already frozen, where thawing them may break users data integrity if
they used VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE accidentally after an
explicit virDomainFSFreeze and the next snapshot without that flag would
be taken with already thawed filesystems.
This effectively reverts 7c736bab06 .
Libvirt nowadays checks whether the guest agent is connected and pings
it before issuing an command so it's very unlikely that we'd end up in a
situation where qemuSnapshotCreateActiveExternal froze filesystems and
didn't thaw them.
Additionally we now discourage the use of
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE since users have better control if
they freeze the FS themselves.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The flag creates additional points of failure which are hard to recover
from, such as when thawing of the filesystems fails after an otherwise
successful snapshot.
Encourage use of explicit virDomainFSFreeze/virDomainFSThaw.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use g_autofree for 'dom_xml' to free it on some of the (unlikely) code
paths jumping to cleanup prior to the deallocation which is done right
after it's not needed any more since it's a big string.
Noticed when running under valgrind:
==2204780== 8,192 bytes in 1 blocks are definitely lost in loss record 2,539 of 2,551
==2204780== at 0x483BCE8: realloc (vg_replace_malloc.c:834)
==2204780== by 0x4D890DF: g_realloc (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780== by 0x4DA3AF0: g_string_append_vprintf (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780== by 0x4917293: virBufferAsprintf (virbuffer.c:307)
==2204780== by 0x49B0B75: virDomainChrDefFormat (domain_conf.c:26109)
==2204780== by 0x49E25EF: virDomainDefFormatInternalSetRootName (domain_conf.c:28956)
==2204780== by 0x15F81D24: qemuDomainDefFormatBufInternal (qemu_domain.c:6204)
==2204780== by 0x15F8270D: qemuDomainDefFormatXMLInternal (qemu_domain.c:6229)
==2204780== by 0x15F8270D: qemuDomainDefFormatLive (qemu_domain.c:6279)
==2204780== by 0x15FD8100: qemuMigrationSrcBeginPhase (qemu_migration.c:2395)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer3 (qemu_migration.c:4640)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer (qemu_migration.c:5093)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformJob (qemu_migration.c:5168)
==2204780== by 0x15FE280E: qemuMigrationSrcPerform (qemu_migration.c:5372)
==2204780== by 0x15F9BA3D: qemuDomainMigratePerform3Params (qemu_driver.c:11841)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In one of my previous commits I've made an attempt to restore the
noqueue qdisc on a TAP corresponding to domain's <interface/> if
QoS is cleared out. The commit consisted of two almost identical
hunks. In both the pointer is dereferenced. But in one of them,
the pointer to new bandwidth can't be NULL while in the other it
can leading to a crash.
Fixes: d53b092353
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919619
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Previous patches have converted VIR_FREE to g_free in functions with
names ending in Free() and Dispose(), but there are a few similar
functions with names that don't fit that pattern, but server the same
purpose (and thus can survive the same conversion). in particular
*Free*(), and *Unref().
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Invocations of the macro ESX_VI__TEMPLATE__FREE() will free the main
object (referenced as "item") that's pointing to all the things being
VIR_FREEd in the body, so it is safe for all the pointers in item to
just be g_freed rather that VIR_FREEd.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The root directory can be provided by user (or a temporary one is
generated) and is always formatted into connection URI for both
secret driver and QEMU driver, like this:
qemu:///embed?root=$root
But if it so happens that there is an URI unfriendly character in
root directory or path to it (say a space) then invalid URI is
formatted which results in unexpected results. We can trust
g_dir_make_tmp() to generate valid URI but we can't trust user.
Escape user provided root directory. Always.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920400
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This API allows fetching a list of informational messages recorded
against the domain. This provides a way to give information about
tainting of the guest due to undesirable actions/configs, as well
as provide details of deprecated features.
The output of this API is explicitly targetted at humans, not
machines, so it is inappropriate to attempt to pattern match on
the strings and take action off them, not least because the messages
are marked for translation.
Should there be a demand for machine targetted information, this
would have to be addressed via a new API, and is not planned at
this point in time.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
These messages are only valid while the domain is running.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
These messages will be stored in the live status XML.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The <teaming> element in <interface> allows pairing two interfaces
together as a simple "failover bond" network device in a guest. One of
the devices is the "transient" interface - it will be preferred for
all network traffic when it is present, but may be removed when
necessary, in particular during migration, when traffic will instead
go through the other interface of the pair - the "persistent"
interface. As it happens, in the QEMU implementation of this teaming
pair (called "virtio failover" in QEMU) the transient interface is
always a host network device assigned to the guest using VFIO (aka
"hostdev"); the persistent interface is always an emulated virtio NIC.
When support was initially added for <teaming>, it was written to
require that the transient/hostdev device be defined using <interface
type='hostdev'>; this was done because the virtio failover
implementation in QEMU and the virtio guest driver demands that the
two interfaces in the pair have matching MAC addresses, and the only
way libvirt can guarantee the MAC address of a hostdev network device
is to use <interface type='hostdev'>, whose main purpose is to
configure the device's MAC address before handing the device to
QEMU. (note that <interface type='hostdev'> in turn requires that the
network device be an SRIOV VF (Virtual Function), as that is the only
type of network device whose MAC address we can set in a way that will
survive the device's driver init in the guest).
It has recently come up that some users are unable to use <teaming>
because they are running in a container environment where libvirt
doesn't have the necessary privileges or resources to set the VF's MAC
address (because setting the VF MAC is done via the same device's PF
(Physical Function), and the PF is not exposed to libvirt's container).
At the same time, these users *are* able to set the VF's MAC address
themselves in advance of staring up libvirt in the container. So they
could theoretically use the <teaming> feature if libvirt just skipped
the "setting the MAC address" part.
Fortunately, that is *exactly* the difference between <interface
type='hostdev'> (which must be a "hostdev VF") and <hostdev> (a "plain
hostdev" - it could be *any* PCI device; libvirt doesn't know what type
of PCI device it is, and doesn't care).
But what is still needed is for libvirt to provide a small bit of
information on the QEMU commandline argument for the hostdev, telling
QEMU that this device will be part of a team ("failover pair"), and
the id of the other device in the pair.
To make both of those goals simultaneously possible, this patch adds
support for the <teaming> element to plain <hostdev> - libvirt doesn't
try to set any MAC addresses, and QEMU gets the extra commandline
argument it needs)
(actually, this patch adds only the parsing/formatting of the
<teaming> element in <hostdev>. The next patch will actually wire that
into the qemu driver.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In preparation for using the same element in two places, split the
parsing/formating for that subelement out of the virDomainNetDef
functions into their own functions.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
To make it easier to split out the parsing/formatting of the <teaming>
element into separate functions (so we can more easily add the
<teaming> element to <hostdev>, change its virDomainNetDef so that it
points to a virDomainNetTeamingInfo rather than containing one.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This struct was previously defined only within virDomainNetDef where
it was used, but I need to also use it in virDomainHostdevDef, so move
the internal struct out to its own "official" struct and give it the
standard typedef duo and *Free() function.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previously we only checked MAC address and PCI address (or CCW
address). This is not enough information in cases where PCI address
isn't provided and multiple interfaces have the same MAC address (for
example, a virtio + hostdev "teaming" pair - their MAC addresses are
always the same).
Resolves: https://bugzilla.redhat.com/1926190
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
