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>.
Use ATTRIBUTE_FALLTHROUGH, introduced by commit
5d84f5961b, instead of comments to
indicate that the fall through is an intentional behavior.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
They do the same thing with only one difference. Let's put them
together (like we already do with virFDStreamCloseInt) so that future
changes don't miss one of the implementations. Also to clean up the
code.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The number of records that virConnectGetAllDomainStats can return per
domain is currently limited to 4096. This is quite low -- for
example, a single guest with ~320 disks will hit this limit. This
increases the limit to make it much larger. Note that
VIR_NET_MESSAGE_MAX still protects the total message size in the case
where there are many domains and many disks per domain.
I tested this using a guest with 500 disks with no issues.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1440683
These flags to APIs will tell if caller wants to use sparse
stream for storage transfer. At the same time, it's safe to
enable them in storage driver frontend and rely on our backends
checking the flags. This way we can enable specific flags only on
some specific backends, e.g. enable
VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM for filesystem backend but
not iSCSI backend.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Now that we have RPC wrappers over VIR_NET_STREAM_HOLE we can
start wiring them up. This commit wires up situation when a
client wants to send a hole to daemon.
To keep stream offsets synchronous, upon successful call on the
daemon skip the same hole in local part of the stream.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add a new argument to daemonCreateClientStream in order to allow for
future expansion to mark that a specific stream can be used to skip
data, such as the case with sparsely populated files. The new flag will
be the eventual decision point between client/server to decide whether
both ends can support and want to use sparse streams.
A new bool 'allowSkip' is added to both _virNetClientStream and
daemonClientStream in order to perform the tracking.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add a virStreamPtr pointer to the _virNetClientStream
in order to reverse track the parent stream.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There are three virStreamDriver's currently supported:
* virFDStream
* remote driver
* ESX driver
For now, backend virStreamRecvFlags support for only remote driver and
ESX driver is sufficient. Future patches will update virFDStream.
Signed-off-by: Michal Privoznik <mprivozn@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>
When using thin provisioning, management tools need to resize the disk
in certain cases. To avoid having them to poll disk usage introduce an
event which will be fired when a given offset of the storage is written
by the hypervisor. Together with the API which will be added later, it
will allow registering thresholds for given storage backing volumes and
this event will then notify management if the threshold is exceeded.
Similarly to domainSetGuestVcpus this commit adds API which allows to
modify state of individual vcpus rather than just setting the count.
This allows to enable CPUs in specific guest NUMA nodes to achieve any
necessary configuration.
On a system with 697 SCSI disks each configured with 8 paths the command
virsh nodedev-list fails with
error: Failed to list node devices
error: internal error: Too many node_devices '16816' for limit '16384'
Increasing the upper limit on lists of node devices from 16K to 64K.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
This commit removes the handcrafted code for
remoteDomainCreateWithFlags() and lets it auto generate.
A little bit of history repeating...
Commit 03d813bbcd removed the auto generation of
remoteDomainCreateWithFlags() because it was thought that the design
flaw in the remote protocol for virDomainCreate is also within the
remote protocol for virDomainCreateWithFlags. As the commit message of
ddaf15d7a3 mentions this is not the case therefore we
can auto generate the client part.
Even worse there was a typo in remoteDomainCreateWithFlags()
'remote_domain_create_with_flags_args ret;' but in fact it has to be
'remote_domain_create_with_flags_ret ret;'.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
The handler for the device removal failed event was using
the struct for the device added event. Fortunately the
layout was the same, so this was harmless.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When changing the metadata via virDomainSetMetadata, we now
emit an event to notify the app of changes. This is useful
when co-ordinating different applications read/write of
custom metadata.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1332019
This function will essentially be a wrapper to virStorageVolInfo in order
to provide a mechanism to have the "physical" size of the volume returned
instead of the "allocation" size. This will provide similar capabilities to
the virDomainBlockInfo which can return both allocation and physical of a
domain storage volume.
NB: Since we're reusing the _virStorageVolInfo and not creating a new
_virStorageVolInfoFlags structure, we'll need to generate the rpc APIs
remoteStorageVolGetInfoFlags and remoteDispatchStorageVolGetInfoFlags
(although both were originally created from gendispatch.pl and then
just copied into daemon/remote.c and src/remote/remote_driver.c).
The new API will allow the usage of a VIR_STORAGE_VOL_GET_PHYSICAL flag
and will make the decision to return the physical or allocation value
into the allocation field.
In order to get that physical value, virStorageBackendUpdateVolTargetInfoFD
adds logic to fill in physical value matching logic in qemuStorageLimitsRefresh
used by virDomainBlockInfo when the domain is inactive.
Signed-off-by: John Ferlan <jferlan@redhat.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>
Implement in virtNetClient and VirNetSocket the needed functions to
expose a new libssh transport, providing all the options that the
libssh2 transport supports.
We are about to add 6 new values to fetch. This will put us over the
current limit of 16 (we're at 13 now).
Once there are more than 16 parameters, this will affect existing clients
that attempt to fetch blockiotune config values for the domain from the
remote host since the server side has no mechanism to determine whether
the capability for the emulator exists and thus would attempt to return
all known values from the persistentDef. If attempting to fetch the
blockiotune values from a running domain, the code will check the emulator
capabilities and set maxparams (in qemuDomainGetBlockIoTune) appropriately.
On the client side of the remote connection, it uses this constant in
xdr_remote_domain_get_block_io_tune_ret and virTypedParamsDeserialize
calls, so if a remote server returns more than 16 parameters, then the
client will fail with "Unable to decode message payload".
Signed-off-by: John Ferlan <jferlan@redhat.com>
The REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX was erroneously used in the
remoteDomainBlockStatsFlags and remoteDomainGetBlockIoTune calls. Change
the constant to be the right one.
Fortunately, all 3 are defined as 16.
Signed-off-by: John Ferlan <jferlan@redhat.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>
This way we make naming consistent to API calls and make subsequent
ACL checks possible (otherwise ACL check would discover name
discrepancies).
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
This event is emitted when a nodedev XML definition is updated,
like when cdrom media is changed in a cdrom block device.
Also includes node device update event implementation for udev
backend, virsh nodedev-event support, and event-test support
The VIR_STORAGE_POOL_EVENT_REFRESHED constant does not
reflect any change in the lifecycle of the storage pool.
It should thus not be part of the storage pool lifecycle
event set, but rather be a top level event in its own
right. Thus we introduce VIR_STORAGE_POOL_EVENT_ID_REFRESH
to replace it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This partially reverts commit 9b45c9f049.
It changed the default format of socket address from the one SASL
requires, but did not adjust all the callers.
It also removed the test coverage for it.
Revert most of the changes except the virSocketAddrFormatFull support
for URI-formatted strings.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1345743 while
reverting the format used by virt-admin's client-info command from
the URI one to the SASL one.
https://bugzilla.redhat.com/show_bug.cgi?id=1345743
To allow finer-grained control of vcpu state using guest agent this API
can be used to individually set the state of the vCPU.
This will allow to better control NUMA enabled guests and/or test
various vCPU configurations.
Add a rather universal API implemented via typed params that will allow
to query the guest agent for the state and possibly other aspects of
guest vcpus.
Since it's rather tedious to write the dispatchers for functions that
return an array of typed parameters (which are rather common) let's add
some rpcgen code to generate them.
Support reading the TLS priority from the client configuration
file via the "tls_priority" config option, eg
$ cat $HOME/.config/libvirt/libvirt.conf
tls_priority="NORMAL:-VERS-SSL3.0"
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virConnectOpenInternal method opens the libvirt client
config file and uses it to resolve things like URI aliases.
There may be driver specific things that are useful to
store in the config file too, so rather than have them
re-parse the same file, pass the virConfPtr down to the
drivers.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add support for a "tls_priority" URI parameter in remote
driver URIs. eg
qemu+tls://localhost/session?tls_priority=NORMAL:-VERS-SSL3.0
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Extend the virNetTLSContextNew* constructors to allow
the TLS priority string to be passed in, overriding the
compile time default.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Our socket address format is in a rather non-standard format and that is
because sasl library requires the IP address and service to be delimited by a
semicolon. The string form is a completely internal matter, however once the
admin interfaces to retrieve client identity information are merged, we should
return the socket address string in a common format, e.g. format defined by
URI rfc-3986, i.e. the IP address and service are delimited by a colon and
in case of an IPv6 address, square brackets are added:
Examples:
127.0.0.1:1234
[::1]:1234
This patch changes our default format to the one described above, while adding
separate methods to request the non-standard SASL format using semicolon as a
delimiter.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
If you compile a client --without-polkit, and connect to a URI that needs
polkit auth, the connection will fail with:
$ ./tools/virsh --connect qemu+ssh://crobinso@machine/system
error: failed to connect to the hypervisor
error: authentication failed: unsupported authentication type 2
This is because the client side portion of the polkit handling is
compiled out. However, nothing polkit specific is actually required
of the client.
Fix that error by unconditionally compiling the basic polkit client
handling.
https://bugzilla.redhat.com/show_bug.cgi?id=635529
Since we didn't opt to use one single event for device lifecycle for a
VM we are missing one last event if the device removal failed. This
event will be emitted once we asked to eject the device but for some
reason it is not possible.
I've noticed that these APIs are missing @flags argument. Even
though we don't have a use for them, it's our policy that every
new API must have @flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
