Using the 'uuid' element for ppc64 NVDIMM memory added in the
previous patch, use it in qemuBuildMemoryDeviceStr() to pass
it over to QEMU.
Another ppc64 restriction is the necessity of a mem->labelsize,
given than ppc64 only support label-area backed NVDIMMs.
Finally, we don't want ppc64 NVDIMMs to align up due to the
high risk of going beyond the end of file with a 256MiB
increment that the user didn't predict. Align it down
instead. If target size is less than the minimum of
256MiB + labelsize, error out since QEMU will error out
if we attempt to round it up to the minimum.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
ppc64 NVDIMM support was implemented in QEMU by commit [1].
The support is similar to what x86 already does, aside from
an extra 'uuid' element.
This patch introduces a new optional 'uuid' element for the
NVDIMM memory model. This element behaves like the 'uuid'
element of the domain definition - if absent, we'll create
a new one, otherwise use the one provided by the XML.
The 'uuid' element is exclusive to pseries guests and are
unavailable for other architectures.
Next patch will use this new element to add NVDIMM support
for ppc64.
[1] ee3a71e366
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Update ppc64 capabilities to pick up the new NVDIMM capability
support for ppc64.
Since the ppc64 capabilities weren't updated for some time, the
bulk of the changes here are related to the blockdev support
(see commit c6a9e54ce3 for info) that we are picking up just
now.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In previous patches virKeyFile was replaced with its GLib
counterpart which created an incompatible change: comments can
now begin only with a number sign (#). While this won't probably
affect anyone, mention it in the release notes.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The functionality is now provided by glib's GKeyFile.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace libvirt's virKeyFile by glib's GKeyFile.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function changes the amount of time that libvirt waits for a
response from the guest agent for all guest agent commands. Since this
is a configuration change, it should not be allowed on read-only
connections.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The agent 'guest-sync' command historically had a 5s response timeout
which was different from other agent commands, which waited forever.
When we added the ability to customize the response timeout for guest
agent commands, we intended to continue to use 5s for 'guest-sync' when
the user specified a response timeout greater than 5s, and use the
user-specified timeout if it was below 5s. Unfortunately, when
attempting to determine whether the user-specified timeout was less than
5s, we were comparing against an enum value of
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT (which is -1) rather than against
the actual time value that it represented (5).
This change makes it so that 'guest-sync' now uses the user-specified
tiemout if it is less than 5s.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use existing function built for this exact purpose.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When a QEMU process dies in the middle of a hotplug, then we fail
to restore the seclabels on the device. The problem is that if
the thread doing hotplug locks the domain object first and thus
blocks the thread that wants to do qemuProcessStop(), the
seclabel cleanup code will see vm->pid still set and mount
namespace used and therefore try to enter the namespace
represented by the PID. But the PID is gone really and thus
entering will fail and no restore is done. What we can do is to
try enter the namespace (if requested to do so) but if entering
fails, fall back to no NS mode.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814481
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
When running a function in a forked child, so far the only thing
we could report is exit status of the child and the error
message. However, it may be beneficial to the caller to know the
actual error that happened in the child.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The @src is not always a file. It may also be a directory (for
instance qemuDomainCreateDeviceRecursive() assumes that) - even
though it doesn't happen usually. Anyway, mount() can mount only
a dir onto a dir and a file onto a file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The @devPath variable is not modifiable. It merely just points to
string containing path where private devtmpfs is being
constructed. Make it const so it doesn't look weird that it's not
freed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
If building namespace fails somewhere in the middle (that is some
files exists under devMountsSavePath[i]), then plain rmdir() is
not enough to remove dir. Umount the temp location and use
virFileDeleteTree() to remove the directory.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The virFileMakePathWithMode() which is our recursive version of
mkdir() fails, it simply just returns a negative value with errno
set. No error is reported (as compared to virFileTouch() for
instance).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Validate that we are able to parse back the dotted syntax arrays we were
generating in the pre-blockdev era.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For the few instances where we'd generate an array in dotted syntax we
should be able to parse it back. Add another step in deflattening of the
dotted syntax which reconstructs the arrays so that the backing store
parser can parse it.
https://bugzilla.redhat.com/show_bug.cgi?id=1466177
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Extract the code so that there's a clean separation once we'll want do
do other steps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory handling to remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virBitmapNewEmpty can't fail now so we can make it obvious and fix all
callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virStorageEncryptionSecretPtr may have a string inside it, thus we must
copy the string too. Use virSecretLookupDefCopy to do that.
Caused by non-obvious code introduced in 756b46ddd2 and later 47e88b33b
which added a string that needed to be copied.
https://bugzilla.redhat.com/show_bug.cgi?id=1814923
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function always returns succes so there's no need for a return
value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuBlockStorageSourceGetFormatRawProps aggregated both formats but
since we now have props specific for either of those formats it's
unwanted to aggregate the code such way. Split out the 'luks' props
formatter into qemuBlockStorageSourceGetFormatLUKSProps.
The wrong separation demonstrates istself on formatting of the 'size'
and 'offset' attributes for the 'luks' driver which does not conform
to the qapi schema.
https://bugzilla.redhat.com/show_bug.cgi?id=1814975
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'luks' driver in qemu is as any other non-raw format driver and thus
doesn't support the properties for 'slice'. Since libvirt considers
luks files to be raw+encryption we need to special case them when
dealing with the slice.
https://bugzilla.redhat.com/show_bug.cgi?id=1814975
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce qemuBlockStorageSourceNeedsStorageSliceLayer which will hold
the decision logic and fix all places that open-code it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since libvirt handles the luks encryption in a weird special way
(raw+encryption) we should really test that case with slices as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A fixup to patch [1]. We need to reset await_event in all
error paths.
[1] 52532073d : qemu: remove redundant needReply argument of qemuAgentCommand
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In one of my previous commits I've introduced code that creates
all devices for given (possible) multipath target. But I've made
a mistake there - the code accesses 'next->path' without checking
if the disk source is local. Note that the 'next->path' is
NULL/doesn't make sense for VIR_STORAGE_TYPE_NVME.
Fixes: a30078cb83
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814947
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since the removal of gnulib, HAVE_SOCKETPAIR is no more defined, making
these two tests effectively skipped.
Use the same strategy used in other generic library bits, i.e. exclude
the socketpair usage on Windows.
Semi-related change in virnetdaemontest.c to make it build: since
virutil.h does not include unistd.h anymore, we need to include it.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
It seems like CD-ROMs may have no 'fileName' property specified in case
there is nothing configured as attachment for the drive. Hence, make
sure that virVMXParseDisk() do not consider it mandatory anymore,
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
used also to parse disk and floppies, so make sure that a NULL fileName
is handled in cdrom- and floppy-related paths.
https://bugzilla.redhat.com/show_bug.cgi?id=1808610
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Move earlier the checks for skipping a hard disk when parsing a CD-DROM,
and for skipping a CD-ROM when parsing a hard disk. This should have no
behaviour changes, and avoids to add repeated checks in following
commits.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Many calls of qemuMonitorDelObject don't actually check the return value
or report the error from the object deletion itself since they are on
cleanup paths. In some cases this can lead to reporting of spurious
errors e.g. when qemuMonitorDelObject is used to clean up a possibly
pre-existing objects.
Add a new argument for qemuMonitorDelObject which controls whether
the internals report errors from qemu and fix all callers accordingly.
Note that some of the cases on device unplug which check the error code
don't in fact propagate the error to the user, but in this case it is
important to add the log entry anyways for tracing that the device
deletion failed.
https://bugzilla.redhat.com/show_bug.cgi?id=1784040
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In some cases we'll need to check whether there was an error but avoid
reporting an actual libvirt error. Rename qemuMonitorJSONCheckError to
qemuMonitorJSONCheckErrorFull with a new flag to suppress the error
reporting and add a wrapper with the original name so that callers don't
need to be fixed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'g_autoptr' and remove the cleanup label and ret variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When changing media we'd attempt to remove the managed pr daemon even if
neither of the images involved in the media change used it. This caused
libvirtd to log a spurious error:
2020-03-18 01:41:19.832+0000: 643207: error : qemuMonitorJSONCheckError:412 : internal error: unable to execute QEMU command 'object-del': object 'pr-helper0' not found
With this patch we completely avoid calling the deletion code.
https://bugzilla.redhat.com/show_bug.cgi?id=1814486
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The loop which checks whether the vcpus are in proper configuration for
the requested hot(un)plug skips the first modified vcpu. This means
that 'firstvcpu' which is used to print the error message in case the
configuration is not suitable would never point to the first modified
vcpu.
In cases such as:
<vcpu placement='auto' current='5'>8</vcpu>
<vcpus>
<vcpu id='0' enabled='yes' hotpluggable='no'/>
<vcpu id='1' enabled='yes' hotpluggable='no'/>
<vcpu id='2' enabled='yes' hotpluggable='no'/>
<vcpu id='3' enabled='yes' hotpluggable='no'/>
<vcpu id='4' enabled='yes' hotpluggable='no'/>
<vcpu id='5' enabled='no' hotpluggable='yes'/>
<vcpu id='6' enabled='no' hotpluggable='yes'/>
<vcpu id='7' enabled='no' hotpluggable='yes'/>
</vcpus>
# virsh setvcpu --config --disable upstream 1
error: invalid argument: vcpu '-1' can't be modified as it is followed by non-hotpluggable online vcpus
After this fix the proper vcpu is reported in the error message:
# virsh setvcpu --config --disable upstream 1
error: invalid argument: vcpu '1' can't be modified as it is followed by non-hotpluggable online vcpu
https://bugzilla.redhat.com/show_bug.cgi?id=1611061
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
So far, when using the qemu:///embed driver, management
applications can't chose whether they want to register their
domains in machined or not. While having that option is certainly
desired, it will require more work. What we can do meanwhile is
to generate names that include part of hash of the root
directory. This is to ensure that if two applications using
different roots but the same domain name (and ID) start the
domain no clashing name for machined is generated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When initializing virQEMUDriverConfig structure we are given the
root directory for possible embed connection. Save it for future
use. While we could get it later from @uri member, it's not as
easy as dereferencing a pointer (virURIParse() +
virURIGetParam() + error reporting).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The check attribute was completely ignored for host-passthrough CPUs
even if they explicitly requested some features to be enabled. For
example, a domain with the following CPU definition
<cpu mode='host-passthrough' check='full'>
<feature policy='require' name='svm'/>
</cpu>
would happily start even when 'svm' cannot be enabled.
Let's call virCPUArchUpdateLive for host-passthrough CPUs with
VIR_CPU_CHECK_FULL to make sure the architecture specific code can
validate the provided virtual CPU against the desired definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1515677
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Adding more checks into the existing if statements would turn them into
an unreadable mess.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The updateLive CPU sub-driver function is supposed to be called only for
a subset of CPU definitions. Let's make it more obvious by turning a
negative test and return into a positive check.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
