When adding a new object to the domain object list, there should
have been 2 virObjectRef calls made one for each list into which
the object was placed to match the 2 virObjectUnref calls that
would occur during Remove as part of virHashRemoveEntry when
virObjectFreeHashData is called when the element is removed from
the hash table as set up in virDomainObjListNew.
Some drivers (libxl, lxc, qemu, and vz) handled this inconsistency
by calling virObjectRef upon successful return from virDomainObjListAdd
in order to use virDomainObjEndAPI when done with the returned @vm.
While others (bhyve, openvz, test, and vmware) handled this via only
calling virObjectUnlock upon successful return from virDomainObjListAdd.
This patch will "unify" the approach to use virDomainObjEndAPI
for any @vm successfully returned from virDomainObjListAdd.
Because list removal is so tightly coupled with list addition,
this patch fixes the list removal algorithm to return the object
as entered - "locked and reffed". This way, the callers can then
decide how to uniformly handle add/remove success and failure.
This removes the onus on the caller to "specially handle" the
@vm during removal processing.
The Add/Remove logic allows for some logic simplification such
as in libxl where we can Remove the @vm directly rather than
needing to set a @remove_dom boolean and removing after the
libxlDomainObjEndJob completes as the @vm is locked/reffed.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Rework the code such that virDomainObjListFindByID will always
return a locked/ref counted object so that the callers can
always do the same cleanup logic to call virDomainObjEndAPI.
Makes accessing the objects much more consistent.
NB:
There were 2 callers (lxcDomainLookupByID and qemuDomainLookupByID)
that were already using the ByID name, but not virDomainObjEndAPI -
these were changed as well in this update/patch.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Now that every caller is using virDomainObjListFindByUUIDRef,
let's just remove it and keep the name as virDomainObjListFindByUUID.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
For vzDomainLookupByID and vzDomainLookupByUUID let's
return a locked and referenced @vm object so that callers
can then use the common and more consistent virDomainObjEndAPI
in order to handle cleanup rather than needing to know that the
returned object is locked and calling virObjectUnlock.
The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.
Also adjust the prlsdkHandle{VmState|VmRemoved|Perf}Event APIs
in the same manner.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Rather than have two API's doing different things for different
callers, let's make one API that will always return a locked and
ref counted object. That way, the callers will always know that
they must call virDomainObjEndAPI and not have to decide whether
they should call virObjectUnlock instead.
This will make things consistent with LookupByName which returns
the locked and ref counted object.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Broken by [1] commit - trailing comma instead of semicolon. Fortunately
the issue did not get sneak in released 4.2 version. Note that uriSchemes
for parallelsConnectDriver should not be allocated on stack.
[1] 8e4f9a27: "driver: declare supported URI schemes in virConnectDriver struct"
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
So far we are repeating the following lines over and over:
if (!(virSomeObjectClass = virClassNew(virClassForObject(),
"virSomeObject",
sizeof(virSomeObject),
virSomeObjectDispose)))
return -1;
While this works, it is impossible to do some checking. Firstly,
the class name (the 2nd argument) doesn't match the name in the
code in all cases (the 3rd argument). Secondly, the current style
is needlessly verbose. This commit turns example into following:
if (!(VIR_CLASS_NEW(virSomeObject,
virClassForObject)))
return -1;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Avoid the need for the drivers to explicitly check for a NULL path by
making sure it is at least the empty string.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ensuring that we don't call the virDrvConnectOpen method with a NULL URI
means that the drivers can drop various checks for NULL URIs. These were
not needed anymore since the probe functionality was split
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Declare what URI schemes a driver supports in its virConnectDriver
struct. This allows us to skip trying to open the driver entirely
if the URI scheme doesn't match.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add a localOnly flag to the virConnectDriver struct which allows a
driver to indicate whether it is local-only, or permits remote
connections. Stateful drivers running inside libvirtd are generally
local only. This allows us to remote the check for uri->server != NULL
from most drivers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add typedef for the anonymous enum used for the driver features. This
allows the usage of the type in a switch statement and taking
advantage of the compilers feature to detect uncovered cases.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Build is broken after 67966ad51 [1].
[1] m4: enforce that all enum cases are listed in switch statements
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Build is broken by 5529b057 [1].
[1] cfg: forbid includes of headers in network and storage drivers again
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Loadable drivers must never depend on each other. Over time some usage
mistakenly crept in for the storage and network drivers, but now this is
eliminated the syntax-check rules can enforce this separation once more.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This attribute was used to decide whether to format the type
attribute of the <target> element, but the logic didn't take into
account all possible cases and as such could lead to unexpected
results. Moreover, it's one more thing to keep track of, and can
easily fall out of sync with other attributes.
Now that we have VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE, we can
use that value to signal that no specific target type has been
configured for the serial device and as such the attribute should
not be formatted at all. All other values are now formatted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Right-aligning backslashes when defining macros or using complex
commands in Makefiles looks cute, but as soon as any changes is
required to the code you end up with either distractingly broken
alignment or unnecessarily big diffs where most of the changes
are just pushing all backslashes a few characters to one side.
Generated using
$ git grep -El '[[:blank:]][[:blank:]]\\$' | \
grep -E '*\.([chx]|am|mk)$$' | \
while read f; do \
sed -Ei 's/[[:blank:]]*[[:blank:]]\\$/ \\/g' "$f"; \
done
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
There is no need to have two different enums where one has the same
values as the other one with some additions.
Currently for on_poweroff and on_reboot we allow only subset of actions
that are allowed for on_crash. This was covered in parse time using
two different enums. Now to make sure that we don't allow setting
actions that are not supported we need to check it while validating
domain config.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1497396
The other APIs accept both, ifname and MAC address. There's no
reason virDomainInterfaceStats can't do the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In the past we updated host-model CPUs with host CPU data by adding a
model and features, but keeping the host-model mode. And since the CPU
model is not normally formatted for host-model CPU defs, we had to pass
the updateCPU flag to the formatting code to be able to properly output
updated host-model CPUs. Libvirt doesn't do this anymore, host-model
CPUs are turned into custom mode CPUs once updated with host CPU data
and thus there's no reason for keeping the hacks inside CPU XML
formatters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The implementation of virConnectBaselineCPU may be different for each
hypervisor. Thus it shouldn't really be implmented in the cpu code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
To handle setting a default heads value. Convert callers that were
doing it by hand
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
At the time the check was written virtuozzo did not use disabled items in boot
order configuration. Boot items were always enabled. Now they can be disabled
as well. Supporting such items is easy - they just should be ignored.
The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
constants are only used by a handful of files, so are better
kept in virsocketaddr.h or the source file that uses them.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If a remote call fails during event registration (more than likely from
a network failure or remote libvirtd restart timed just right), then when
calling the virObjectEventStateDeregisterID we don't want to call the
registered @freecb function because that breaks our contract that we
would only call it after succesfully returning. If the @freecb routine
were called, it could result in a double free from properly coded
applications that free their opaque data on failure to register, as seen
in the following details:
Program terminated with signal 6, Aborted.
#0 0x00007fc45cba15d7 in raise
#1 0x00007fc45cba2cc8 in abort
#2 0x00007fc45cbe12f7 in __libc_message
#3 0x00007fc45cbe86d3 in _int_free
#4 0x00007fc45d8d292c in PyDict_Fini
#5 0x00007fc45d94f46a in Py_Finalize
#6 0x00007fc45d960735 in Py_Main
#7 0x00007fc45cb8daf5 in __libc_start_main
#8 0x0000000000400721 in _start
The double dereference of 'pyobj_cbData' is triggered in the following way:
(1) libvirt_virConnectDomainEventRegisterAny is invoked.
(2) the event is successfully added to the event callback list
(virDomainEventStateRegisterClient in
remoteConnectDomainEventRegisterAny returns 1 which means ok).
(3) when function remoteConnectDomainEventRegisterAny is hit,
network connection disconnected coincidently (or libvirtd is
restarted) in the context of function 'call' then the connection
is lost and the function 'call' failed, the branch
virObjectEventStateDeregisterID is therefore taken.
(4) 'pyobj_conn' is dereferenced the 1st time in
libvirt_virConnectDomainEventFreeFunc.
(5) 'pyobj_cbData' (refered to pyobj_conn) is dereferenced the
2nd time in libvirt_virConnectDomainEventRegisterAny.
(6) the double free error is triggered.
Resolve this by adding a @doFreeCb boolean in order to avoid calling the
freeCb in virObjectEventStateDeregisterID for any remote call failure in
a remoteConnect*EventRegister* API. For remoteConnect*EventDeregister* calls,
the passed value would be true indicating they should run the freecb if it
exists; whereas, it's false for the remote call failure path.
Patch based on the investigation and initial patch posted by
fangying <fangying1@huawei.com>.
virDomainXMLOption gains driver specific callbacks for parsing and
formatting save cookies.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This will be used later when a save cookie will become part of the
snapshot XML using new driver specific parser/formatter functions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
While checking for ABI stability, drivers might pose additional
checks that are not valid for general case. For instance, qemu
driver might check some memory backing attributes because of how
qemu works. But those attributes may work well in other drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Put domain access after acquiring job condition, otherwise
another job can change it meanwhile.
Signed-off-by: Konstantin Neumoin <kneumoin@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We have to use waitDomainJob instead of waitJob, because of it
unlock the domain until job has finished, so domain will be available
for other clients.
Signed-off-by: Konstantin Neumoin <kneumoin@virtuozzo.com>
Added only in drivers that were already calling
virCapabilitiesInitNUMA(). Instead of refactoring all the callers to
behave the same way in case of error, just follow what the callers are
doing for all the functions.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
So far our code is full of the following pattern:
dom = virGetDomain(conn, name, uuid)
if (dom)
dom->id = 42;
There is no reasong why it couldn't be just:
dom = virGetDomain(conn, name, uuid, id);
After all, client domain representation consists of tuple (name,
uuid, id).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There is no "node driver" as there was before, drivers have to do
their own ACL checking anyway, so they all specify their functions and
nodeinfo is basically just extending conf/capablities. Hence moving
the code to src/conf/ is the right way to go.
Also that way we can de-duplicate some code that is in virsysfs and/or
virhostcpu that got duplicated during the virhostcpu.c split. And
Some cleanup is done throughout the changes, like adding the vir*
prefix etc.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
When creating host CPU definition usable with a given emulator, the CPU
should not be defined using an unsupported CPU model. The new @models
and @nmodels parameters can be used to limit CPU models which can be
used in the result.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The parameter can be used to request either VIR_CPU_TYPE_HOST (which has
been assumed so far) or VIR_CPU_TYPE_GUEST definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
cpuNodeData has always been followed by cpuDecode as no hypervisor
driver is really interested in raw CPUID data for a host CPU. Let's
create a new CPU driver API which returns virCPUDefPtr directly.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The new API is called virCPUDataFree. Individual CPU drivers are no
longer required to implement their own freeing function unless they need
to free architecture specific data from virCPUData.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Current code for example can call unsubscribe if connection
succeeds but subscribing fails. This will probabaly lead
only to spurious error messages without any actual inconsistencies
but nevertheless.
When we happen to lose a domain but still get a performance event
for it, we should also free the event handle.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
This is necessary to be able to get statistics for venet0 or
"host-routed" adapter, which has -1 index and thus, its statistics
is shown as "net.nic4294967295".
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Because this is invalid xml for containers. This patch almost
reverts 7eda8369, but still skips converting vz sdk bootorder
for containers to libvirt bootorder because we use boot order
in containers for quite different purpurse.
Before, boot devices information for CTs was always empty and we
didn't indicate that containers can boot from disk.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Virtuozzo SDK interface doesn't differ filesystems from disks and sees them as disks.
Before, we always mistakenly presented disks based on files as filesystems, which is
not completely correct. Now we are going to show either disks or filesystems depending
on a hint, which uses boot device section of VZ config. Though this information
doesn't change booting order of a CT, it is used by vz libvirt interface as a hint
for libvirt representation of disks. Since now, if we have filesystems in input xml,
then we add them to VZ booting devices list and rely on this information to show
corresponding libvirt xml.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Implicit devices like controllers are confusing for CTs and
function virDomainDefAddImplicitDevices never intended to be called
for CTs.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
This flag is used in Virtuozzo backend implicitly, thus
we need to support it and don't fail if it's set.
Signed-off-by: Pavel Glushchak <pglushchak@virtuozzo.com>
This flag tells backend not to create instance
disks making behavior the same as in qemu driver.
Disk files have to be created beforehand on target
host manually or by upper management layer i.e.
OpenStack Nova.
Signed-off-by: Pavel Glushchak <pglushchak@virtuozzo.com>
This reverts commit 3a6cf6fc16.
Mistakenly this commit was pushed because I thought I missed the
corret one b880ff42dd while in fact I didn't.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
We have couple of functions that operate over NULL terminated
lits of strings. However, our naming sucks:
virStringJoin
virStringFreeList
virStringFreeListCount
virStringArrayHasString
virStringGetFirstWithPrefix
We can do better:
virStringListJoin
virStringListFree
virStringListFreeCount
virStringListHasString
virStringListGetFirstWithPrefix
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
New line character in name of domain is now forbidden because it
mess virsh output and can be confusing for users.
Validation of name is done in drivers, after parsing XML to avoid
problems with dissappeared domains which was already created with
new-line char in name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
dom xml generated on begin step should be passed
to perform step in VIR_MIGRATE_PARAM_DEST_XML parameter.
Otherwise 'XML error: failed to parse xml document' is
raised on destination host as dom xml is NULL.
Signed-off-by: Pavel Glushchak <pglushchak@virtuozzo.com>
Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to support it.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Change the virDomainChrDef to use a pointer to 'source' and allocate
that pointer during virDomainChrDefNew.
This has tremendous "fallout" in the rest of the code which mainly
has to change source.$field to source->$field.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Absent driver name attribute is invalid xml. Which in turn makes
unusable 'virsh edit' for example. The value does not make
much sense and ignored on input so nobody will hurt.
vz sdk supports setting serial number only for disk devices.
Getting serial upon cdrom(for example) is error however
setting is just ignored. Let's check for disk device
explicitly for clarity in both cases.
Setting serial number for other devices is ignored
with an info note just as before.
We need usual conversion from "" to NULL in direction
vz sdk -> libvirt, because "" is not valid for libvirt
and "" means unspecifiend in vz sdk which is NULL for libvirt.
Modeled after the qemuDomainHostdevPrivatePtr (commit id '27726d8c'),
create a privateData pointer in the _virDomainChardevDef to allow storage
of private data for a hypervisor in order to at least temporarily store
secret data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the secret data, there's no need to add code
code to handle this new structure there.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Just like virDomainDefPostParseCallback has gained new
parseOpaque argument, we need to follow the logic with
virDomainDeviceDefPostParse.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We want to pass the proper opaque pointer instead of NULL to
virDomainDefParse and subsequently virDomainDefParseNode too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Some callers might want to pass yet another pointer to opaque
data to post parse callbacks. The driver generic one is not
enough because two threads executing post parse callback might
want to see different data (e.g. domain object pointer that
domain def belongs to).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While dettaching/attaching device in OpenStack, nova
calls vzDomainDettachDevice twice, because the update of the internal
configuration of the ct comes a bit latter than the update event.
As the result, we suffer from the second call to dettach the same device.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
libvirt-python passes parameter bandwidth = 0
by default. This means that bandwidth is unlimited.
VZ driver doesn't support bandwidth rate limiting,
but we still need to handle it and fail if bandwidth > 0.
Signed-off-by: Pavel Glushchak <pglushchak@virtuozzo.com>
* Added VIR_MIGRATE_LIVE, VIR_MIGRATE_UNDEFINE_SOURCE and
VIR_MIGRATE_PERSIST_DEST to supported migration flags
Signed-off-by: Pavel Glushchak <pglushchak@virtuozzo.com>
vzDomainMigrateConfirm3Params is whitelisted. Otherwise we need to
move removing domain from domain list from perform to confirm
step. This would further imply adding a flag and check that migration
is in progress to prohibit mistakenly (maliciously) removing domains
on confirm step. vz version of p2p also need to be fixed to include confirm step.
One would also need to add means to cleanup pending migration
on client disconnect as now is has state across several API
calls.
On the other hand current version of confirm step is totaly
harmless thus it is easier to whitelist it at the moment.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
ACL check on perform step should be in API call itself to make ACL
checking script pass. Thus we need to reorganize code to obtain
domain object in perform API itself. Most of this is straight
forward, the only nuance is dropping locks on lengthy remote
operations.
The other motivation is to have only perform step ACL checks for
p2p migration instead of both begin in perform if we can leave
ACL check in vzDomainMigratePerformStep.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
We need it to prepare the calls for ACL checks otherwise ACL checking
script will fail.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
This action deserves its own function and makes main API call
structure much cleaner.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
