This reverts commit 150540394ddaa515f6857616a2bcf792748f162c.
Turns out, this feature is not needed and QEMU will fix TSC
without any intervention from outside.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>P
Some versions of Windows hang on reboot if their TSC value is greater
than 2^54. The workaround is to reset the TSC to a small value. Add
to the domain configuration an attribute for this. It can be used
by QEMU and in principle also by ESXi, which has a property called
monitor_control.enable_softResetClearTSC as well.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since its v5.0.0 release QEMU is capable of specifying number of
threads used to allocate memory. It defaults to 1, which may be
too low for humongous guests with gigantic pages.
In general, on QEMU cmd line level it is possible to use
different number of threads per each memory-backend-* object, in
practical terms it's not useful. Therefore, use <memoryBacking/>
to set guest wide value and let all memory devices 'inherit' it,
silently. IOW, don't introduce per device knob because that would
only complicate things for a little or no benefit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The updateLock is a R/W lock held by anything which needs to read or
modify the rules associated with an NWFilter.
APIs for defining/undefining NW filters rules hold a write lock on
updateLock.
APIs for creating/deleting NW filter bindings hold a read lock on
updateLock, which prevents define/undefine taking place concurrently.
The problems arise when we attempt to creating two NW filter bindings in
parallel.
Thread 1 can acquire the mutex for filter A
Thread 2 can acquire the mutex for filter B
Consider if filters A and B both reference filters C and D, but in
different orders:
Filter A
-> filter C
-> filter D
Filter B
-> filter D
-> filter C
Thread 1 will try to acquire locks in order A, C, D while thread 1 will
try to acquire in order A, D, C. Deadlock can still occur.
Think we can sort the list of filters before acquiring locks on all of
them ? Nope, we allow arbitrary recursion:
Filter A
-> filter C
-> filter E
-> filter F
-> filter H
-> filter K
-> filter D
-> filter G
-> filter I
So we can't tell from looking at 'A' which filters we're going to
need to lock. We can only see the first level of filters references
and we need to lock those before we can see the second level of
filters, etc.
We could probably come up with some cleverness to address this but
it isn't worth the time investment. It is simpler to just keep the
process of creating NW filter bindings totally serialized.
Using two separate locks for this serialization though is pointless.
Every code path which gets a read(updateLock) will go on to hold
updateMutex. It is simpler to just hold write(updateLock) and
get rid of updateMutex. At that point we don't need updateLock
to be a R/W lock, it can be a plain mutex.
Thus this patch gets rid of the current updateLock and updateMutex
and introduces a new top level updateMutex.
This has a secondary benefit of introducing fairness into the
locking. With a POSIX R/W lock, you get writer starvation if
you have lots of readers. IOW, if we call virNWFilterBIndingCreate
and virNWFilterBindingDelete in a tight loop from a couple of
threads, we can prevent virNWFilterDefine from ever acquiring
a write lock.
Getting rid of the R/W lock gives us FIFO lock acquisition
preventing starvation of any API call servicing.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
schemas are used for more than just documentation,
virsh edit fails if schemas are not available.
Therefore, fix the no-docs build by moving schemas/
to the parsing code inside src/conf/.
Signed-off-by: Claudio Fontana <cfontana@suse.de>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The idea of the manual mode is to allow a synchronized snapshot in cases
when the storage is outsourced to an unmanaged storage provider which
requires cooperation with snapshotting.
The mode will instruct the hypervisor to pause along when the other
components are snapshotted and the 'manual' disk can be snapshotted
along. This increases latency of the snapshot but allows them in
otherwise impossible situations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While its name would suggest that virNWFilterObj is an actual
virObject it is not. It's a plain structure (with virMutex as its
first member). Therefore, when locking the struct
virObjectLockGuard() can' be used and virLockGuardLock() must be
used instead.
Spotted-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This enables a later patch to simplify locking during initialization
and cleanup of virNWFilterDriverState.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virHashTableForEach unhelpfully has payload/key args in
its callback reversed compared to g_hash_table_foreach.
When converting from one to the other the semantics
change but you don't get a compile error
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The current use of an array for nwfilter objects requires
the caller to iterate over all elements to find a filter,
and also requires locking each filter.
Switching to a pair of hash tables enables O(1) lookups
both by name and uuid, with no locking required.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The function will be reused in the nodedev drivers udev handling.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
In cases when the hostname of the NBD server doesn't match the hostname
in the TLS certificate the new attribute 'tlsHostname' can be used to
override it.
Add the XML infrastructure and tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The value will be used to override the hostname used for validation of
TLS certificates.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the code to use proper types for the memory and disk snapshot
location and fix the parsing code to be compatible with an unsigned
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Separate the steps of parsing the memory snapshot config from the
post-processing and validation code. The upcoming patch refactoring the
parsing will be simpler.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Assign directly into the definition. The cleanup code can deal with
that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory cleanup, decrease scope of variables and remove the
'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All callers except the one in the 'esx' driver pass the flag. The 'esx'
driver has a check that 'def->ndisks' is zero after parsing the
definition. This means that we can simply always parse the disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The string value associated to the enum is "no". Rename the enum
accordingly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The snapshot location enum is also needed for the disk definition so if
we house it inside domain_conf we can use the proper type for it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'virStorageType' as type for the 'type' member and convert the code
to work properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the function to avoid the cleanup section used to just free
memory associated with the parsed object.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For the various structs storing lists of objects, the access
to the hash tables is not lockless. The mutex on the object
owning the hash table must be held.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is to make it explicit that the template only applies to the NVRAM
store, not the main loader binary, even if the loader is writable.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All callers currently guarantee flags passed to virDomainObjGetMessages
are either zero or contain at least one of the supported flags. But it
doesn't mean we should not check for the possibility an unknown flag was
the only one passed to virDomainObjGetMessages.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are few places where a virPCIDeviceAddress typed variable
is allocated on the stack but it's not initialized. This can lead
to random values of its members which in turn can lead to a
random behaviour.
Generated with help of the following spatch:
@@
identifier I;
@@
- virPCIDeviceAddress I;
+ virPCIDeviceAddress I = { 0 };
And then fixing bhyveAssignDevicePCISlots() which does declare
the variable and then explicitly zero it by calling memset() only
to set a specific member afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In one of my previous commits, I've changed an XPath in
virCPUDefParseXML() from "boolean(./counter...)" to
"./counter...)". Notice the dangling closing bracket? Well, I
didn't back then.
Fixes: 0fe2d8dd335054fae38b46bbbac58a4662e1a1d0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This change was generated using the following spatch:
@ rule1 @
expression a;
identifier f;
@@
<...
- f(*a);
... when != a;
- *a = NULL;
+ g_clear_pointer(a, f);
...>
@ rule2 @
expression a;
identifier f;
@@
<...
- f(a);
... when != a;
- a = NULL;
+ g_clear_pointer(&a, f);
...>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As the parent address is part of the mdev nodedev name lets expose the
internally available parent address in the XML.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The @unique argument didn't exist even when the function was
introduced in a042275a396e, and the @vm argument was not renamed
when the function was changed to take a virDomainDef* instead of
a virDomainObj* in 7ed6934f3b92.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Introduce support for
<serial type='pty'>
<target type='isa-debug'>
<model type='isa-debugcon'/>
</target>
<address type='isa' iobase='0x402'/>
</console>
which is used as a way to receive debug messages from the
firmware on x86 platforms.
Note that the default port is hypervisor specific, with QEMU
currently using 0xe9 since that's the original Bochs debug port.
For use with SeaBIOS/OVMF, the iobase port needs to be explicitly
set to 0x402.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The serial port model cannot be allowed to change across migration
as it affects ABI.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When virNodeDeviceObjListRemove() is called, the passed
virNodeDeviceObj is removed from internal list of node devices
and then unrefed and unlocked. While the former is warranted (the
object was refed at the beginning of the function) the unlock is
not. In fact, it's wrong from conceptual POV. We still want
threads working on the object tu mutually exclude each other.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There are a few places where a variable is VIR_FREE()-d and then
explicitly set to NULL. This is not necessary since VIR_FREE()
does that for us.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The idea behind virNWFilterBindingObjNew() is to create and
return an object of virNWFilterBindingObjClass class. The class
is virObjectLockable (and the corresponding
_virNWFilterBindingObj structure has virObjectLockable parent).
But for some reason plain virObjectNew() is called. This is wrong
because the mutex in the parent is left uninitialized.
Next, the returned object is not locked. This is wrong because in
some cases the returned object is added onto a list of bindings
and then passed to virNWFilterBindingObjEndAPI() which unlocks it
right away. This is potentially dangerous because we might just
have unlocked the object for another thread.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We document that <address type='unassigned'/> can be used only
for <hostdev/>-s. However, corresponding validation rule is
missing. Let's put the rule into hypervisor agnostic part of
validation process so that all drivers can benefit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently, virDomainClockDef is formatted inside
virDomainDefFormatInternalSetRootName() which is already long
enough. Move the code into a new function
(virDomainClockDefFormat()) and make the code use
virXMLFormatElement() while at it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function never returns an error, make it void then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement() to simplify virDomainTimerDefFormat().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The @mode member of the _virDomainTimerDef struct stores
values of the virDomainTimerModeType enum, or -1 for the
default value (when user provided no value in XML).
This is needlessly complicated. Introduce new value to the enum
which reflects the default state.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The @track member of the _virDomainTimerDef struct stores
values of the virDomainTimerTrackType enum, or -1 for the
default value (when user provided no value in XML).
This is needlessly complicated. Introduce new value to the enum
which reflects the default state.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The @tickpolicy member of the _virDomainTimerDef struct stores
values of the virDomainTimerTickpolicyType enum, or -1 for the
default value (when user provided no value in XML).
This is needlessly complicated. Introduce new value to the enum
which reflects the default state.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the _virDomainTimerDef structure we have @present member which
is like virTristateBool, except it's an integer and has values
shifted by one. This is harder to read. Retype the member to
virTristateBool which we are familiar with.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function never returns an error, make it void then. And
while at it, make the @src argument const to make it obvious it's
never changed inside the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The only caller of this function
(qemuProcessFindCharDevicePTYsMonitor()) doesn't pass NULL.
Remove corresponding check from virDomainChrSourceDefCopy().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
