Remove error handling from the call to 'virDomainMomentObjNew' as it
can't return NULL and replace 'virHashAddEntry' by 'g_hash_table_insert'
as we've already checked that snapshot with such name doesn't exist in
the hash table. This removes handling for two impossible errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
VM XML accepts target.port but this does not get passed while
building the QEMU command line for this VM.
Signed-off-by: Divya Garg <divya.garg@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit takes care of following cases:
-> Check availability of requested ports.
->The total number of requested ports should not be more than
VIR_MAX_ISA_SERIAL_PORTS.
->The ports requested should be less than VIR_MAX_ISA_SERIAL_PORTS.
->VIR_MAX_ISA_SERIAL_PORTS should correspond to MAX_ISA_SERIAL_PORTS
specified in QEMU code commit def337ffda34d331404bd7f1a42726b71500df22.
-> Prevent duplicate device assignments to the same port.
-> In case no ports are provided in the XML, this patch scans the list of unused
isa-serial indices to automatically assign available ports for this VM.
Signed-off-by: Divya Garg <divya.garg@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These changes make the g_auto-ification in the next commit clearer.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit f4aae9726df factored out the snapshot redefinition code into a
separate function, but didn't account for the fact that the code is
consuming the reference to the snapshot definition and by moving the
code away the caller (qemuSnapshotCreateXML) now frees the definition
which didn't happen before as we cleared the pointer.
Fix it by increasing the reference locally. Later patches will refactor
the code so that it's more obvious what's happening.
Fixes: f4aae9726df
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2039651
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'def' is commonly used to refer to domain definition. Most of the
snapshot code uses 'snapdef' for the snapshot definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our approach to snapshots without metadata was to insert them to the
snapshot list and then later remove them from the list when the flag is
present.
This quirky logic was broken in a recent refactor of the snapshot code
causing that the snapshot stayed inserted in the snapshot list.
Recent refactor of the snapshot code didn't faithfully relocate this
logic to the new function.
Rather than attempting to restore the quirky logic of adding and then
removing the object, don't add the snapshot into the list at all when
the user doesn't want metadata.
We achieve this by creating a temporary 'virDomainMomentObj' wrapper
which is not inserted into the list and using that instead of calling
virDomainSnapshotAssignDef.
Fixes: 9bad0fb809b
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2039131
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Export 'virDomainMomentObjNew' and 'virDomainMomentObjFree' and define
the latter as autoptr cleanup function for 'virDomainMomentObj'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Format the function prototypes the same way as in the .c file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently virProcessGetStatInfo() always returns success and only logs error
when it is unable to parse the data. Make this function actually report the
error and return a negative value in this error scenario.
Fix the callers so that they do not override the error generated.
Also fix non-linux implementation of this function so as to report error.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When -device is configured via JSON a bug [1] is triggered in qemu were
the DEVICE_DELETED event for the removal of the device frontend is no
longer delivered to libvirt. Without the DEVICE_DELETED event we don't
remove the corresponding entries in the VM XML.
Until qemu will be fixed we must stop using the JSON syntax for -device.
This patch removes the detection of the capability. The capability is
used only during startup of a fresh VM so we don't need to consider any
compaitibility steps for existing VMs.
For users who wish to use 'libvirt-7.9' and 'libvirt-7.10' with
'qemu-6.2' there are two possible workarounds:
- filter out the 'device.json' qemu capability '/etc/libvirt/qemu.conf':
capability_filters = [ "device.json" ]
- filter out the 'device.json' qemu capability via qemu namespace XML:
<domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
[...]
<qemu:capabilities>
<qemu:del capability='device.json'/>
</qemu:capabilities>
</domain>
We must never again use the same capability name as we are now
instructing users to filter it as a workaround so once qemu is fixed
we'll need to pick a new capability value for it.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=2036669
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2035237
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virConnectOpenAuth() calls virConnectOpenInternal(). This later function
generates fine grained errors arising from various failure conditions that are
more accurate than a "catch all" broader VIR_ERR_OPERATION_FAILED error that
the callers of this function generates. Remove the broader error so that more
specific errors can be caught and processed.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are a few glib functions that abort on OOM and thus there's
no point in checking their retval against NULL. Nevertheless, we
do have those checks in a few places. Remove them.
Generated using the following spatch:
@@
expression x;
identifier n;
expression r;
@@
(
x = g_strdup_printf(...);
| x = g_strdup_vprintf(...);
| x = g_strdup(...);
| x = g_strndup(...);
| x = g_new0(...);
| x = g_realloc(...);
)
... when != x
- if(!x)
(
- return r;
|
- goto n;
)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Both virProcessGetStatInfo() and virProcessGetSchedInfo() are
Linux centric. Provide stubs for non-Linux platforms.
Fixes: d73852c49962fbfe652fc7bec595a3f242faf847
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit c7f3a1f7870 turned qemuDomainMachineNeedsFDC() effectively into
qemuDomainIsQ35. Use it instead as it also matches the non-canonicalized
'q35'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All the fd-passing setup of chardevs which this hack meant to disable
was moved to the host-preparation phase which is skipped for formatting
of non-real commandlines.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virURIParamAppend() unconditionally returns 0. Simplify and make the return type
as void type.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Add domainGetVcpuPinInfo and nodeGetCPUMap callbacks to ch driver
Signed-off-by: Vineeth Pillai <viremana@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move qemuGetProcessInfo and qemuGetSchedInfo methods to util and share them
with ch driver.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This change adds the domain name in the error and debug logs during
monitor IO processing so that we may infer which VM experienced
errors such as IO or socket hangup. This may help in debugging
monitor IO errors.
Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
DEBUG_IO and DEBUG_RAW_IO are disabled and hence the code #defined under them
are useless. Remove them.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In a few places (e.g. device attach/detach/update) we are given a
device XML, parse it but then need a copy of parsed data so that
the original can be passed to function handling the request over
inactive XML and the copy is then passed to function handling the
operation over live XML. Note, both functions consume passed
device on success, hence the need for copy.
The problem is in combination of how the copy is obtained and
where is passed. The copy is done by calling
virDomainDeviceDefCopy() which does only inactive copy, i.e. no
live information is copied over (e.g. no aliases).
Then, this copy (inactive XML effectively) is passed to function
handling live part of the operation (e.g.
qemuDomainUpdateDeviceLive()) and the definition containing all
the juicy, live bits is passed to function handling inactive part
of the operation (e.g. qemuDomainUpdateDeviceConfig()).
This is rather incorrect, and XML copies should be passed to
their respective functions.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2036895
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The commit that added error checking to this function
forgot to adjust the WIN32 stub.
Fixes: a873924e36b28c5b125621e35b32beb6b077bcc8
Signed-off-by: Ján Tomko <jtomko@redhat.com>
There is no need to do that since both fallible functions do that already.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Callers that already do this anyway can be cleaned up thanks to this and the one
that does not (daemon startup) gains the benefit of the error being printed to
standard error output changing:
LIBVIRT_LOG_OUTPUTS=1:invalid libvirtd
/home/nert/dev/libvirt/upstream/build/src/libvirtd: initialisation failed
into:
LIBVIRT_LOG_OUTPUTS=1:invalid libvirtd
libvirt: error : invalid argument: Invalid destination 'invalid' for output '1:invalid'
/home/nert/dev/libvirt/upstream/build/src/libvirtd: initialisation failed
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
And make callers check the return value as well. This helps error out early for
invalid environment variables.
That is desirable because it could lead to deadlocks. This can happen when
resetting logging after fork() reports translated errors because gettext
functions are not reentrant. Well, it is not limited to resetting logging after
fork(), it can be any translation at that phase, but parsing environment
variables is easy to make fail on purpose to show the result, it can also happen
just due to a typo.
Before this commit it is possible to deadlock the daemon on startup
with something like:
LIBVIRT_LOG_FILTERS='1:*' LIBVIRT_LOG_OUTPUTS=1:stdout libvirtd
where filters are used to enable more logging and hence make the race less rare
and outputs are set to invalid
Combined with the previous patches this changes
the following from:
...
<deadlock>
to:
...
libvirtd: initialisation failed
The error message is improved in future commits and is also possible thanks to
this patch.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This prevents starting any daemons with improper logging settings. This is
desirable on its own, but will be even more beneficial when more functions start
reporting errors and failing on them, coming up in following patches
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The only difference is that we are not going to be guaranteed that the mutex is
normal (as opposed to recursive, although there is no system known to me that
would default to recursive mutexes), but that was done only to find occasional
errors (during runtime, back in 2010, commit 336fd879c00b). Functions using
this mutex are mostly stable and unchanging, and it makes the virLogOnceInit()
function only return 0 (or possibly abort in glib calls). On top of that we can
assume that the virLogMutex is always initialized which enables us to be more
consistent in some early error reporting.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
