In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.
In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.
With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to find the
author of a particular bit of code.
This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.
The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Seeing a log message saying 'flags=93' is ambiguous & confusing unless
you happen to know that libvirt always prints flags as hex. Change our
debug messages so that they always add a '0x' prefix when printing flags,
and '0' prefix when printing mode. A few other misc places gain a '0x'
prefix in error messages too.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When listening for a subset of monitor events, it can be tedious
to register for each event name in series; nicer is to register
for multiple events in one go. Implement a flag to use regex
interpretation of the event filter.
While at it, prove how much I hate the shift key, by adding a
way to filter for 'shutdown' instead of 'SHUTDOWN'. :)
* include/libvirt/libvirt-qemu.h
(virConnectDomainQemuMonitorEventRegisterFlags): New enum.
* src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister):
Document flags.
* tools/virsh-domain.c (cmdQemuMonitorEvent): Expose them.
* tools/virsh.pod (qemu-monitor-event): Document this.
* src/conf/domain_event.c
(virDomainQemuMonitorEventStateRegisterID): Add flags.
(virDomainQemuMonitorEventFilter): Handle regex, and optimize
client side.
(virDomainQemuMonitorEventCleanup): Clean up regex.
Signed-off-by: Eric Blake <eblake@redhat.com>
Several times in the past, qemu has implemented a new event,
but libvirt has not yet caught up to reporting that event to
the user applications. While it is possible to track libvirt
logs to see that an unknown event was received and ignored,
it would be nicer to copy what 'virsh qemu-monitor-command'
does, and expose this information to the end developer as
one of our unsupported qemu-specific commands.
If you find yourself needing to use this API for more than
just development purposes, please ask on the libvirt list
for a supported counterpart event to be added in libvirt.so.
While the supported virConnectDomainEventRegisterAny() API
takes an id which determines the signature of the callback,
this version takes a string filter and always uses the same
signature. Furthermore, I chose to expose this as a new API
instead of trying to add a new eventID at the top level, in
part because the generic option lacks event name filtering,
and in part because the normal domain event namespace should
not be polluted by a qemu-only event. I also added a flags
argument; unused for now, but we might decide to use it to
allow a user to request event names by glob or regex instead
of literal match.
This API intentionally requires full write access (while
normal event registration is allowed on read-only clients);
this is in part due to the fact that it should only be used
by debugging situations, and in part because the design of
per-event filtering in later patches ended up allowing for
duplicate registrations that could potentially be abused to
exhaust server memory - requiring write privileges means
that such abuse will not serve as a denial of service attack
against users with higher privileges.
* include/libvirt/libvirt-qemu.h
(virConnectDomainQemuMonitorEventCallback)
(virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New prototypes.
* src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New functions.
* src/libvirt_qemu.syms (LIBVIRT_QEMU_1.2.1): Export them.
* src/driver.h (virDrvConnectDomainQemuMonitorEventRegister)
(virDrvConnectDomainQemuMonitorEventDeregister): New callbacks.
Signed-off-by: Eric Blake <eblake@redhat.com>
Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In datatype.c, virGetDomainSnapshot could result in the message:
error: invalid domain pointer in bad domain
Furthermore, while there are a few functions in libvirt.c that
only care about a virDomainPtr without regards to the connection
(such as virDomainGetName), most functions also require a valid
connection. Yet several functions were blindly dereferencing
the conn member without checking it for validity first (such as
virDomainOpenConsole). Rather than try and correct all usage
of VIR_IS_DOMAIN vs. VIR_IS_CONNECTED_DOMAIN, it is easier to
just blindly require that a valid domain object always has a
valid connection object (which should be true anyways, since
every domain object holds a reference to its connection, so the
connection will not be closed until all domain objects have
also been closed to release their reference).
After this patch, all places that validate a domain consistently
report:
error: invalid domain pointer in someFunc
* src/datatypes.h (virCheckDomainReturn, virCheckDomainGoto): New
macros.
* src/datatypes.c (virGetDomainSnapshot): Use new macro.
(virLibConnError): Delete unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
The datatype.c object checks could result in a message like:
error: invalid connection pointer in no connection
This consolidates all clients of this message to have uniform contents:
error: invalid connection pointer in someFunc
Note that virCheckConnectReturn raises an error immediately; in
datatypes.c, where we don't need to raise the error (but instead
just leave it in the thread-local setting), we use
virCheckConnectGoto and the cleanup label instead. Then, for
consistency in that file, all subsequent error messages are
touched to also use the cleanup error label.
* src/datatypes.h (virCheckConnectReturn)
(virCheckConnectGoto): New macros.
* src/datatypes.c: Use new macro.
* src/libvirt-qemu.c (virDomainQemuAttach): Likewise.
(virLibConnError): Delete unused macro.
* src/libvirt-lxc.c (virLibConnError): Likewise.
* src/libvirt.c: Use new macro throughout.
* docs/api_extension.html.in: Modernize documentation.
Signed-off-by: Eric Blake <eblake@redhat.com>
Some of our operation denied messages are outright stupid; for
example, if virIdentitySetAttr fails:
error: operation Identity attribute is already set forbidden for read only access
This patch fixes things to a saner:
error: operation forbidden: Identity attribute is already set
It also consolidates the most common usage pattern for operation
denied errors: read-only connections preventing a public API. In
this case, 'virsh -r -c test:///default destroy test' changes from:
error: operation virDomainDestroy forbidden for read only access
to:
error: operation forbidden: read only access prevents virDomainDestroy
Note that we were previously inconsistent on which APIs used
VIR_FROM_DOM (such as virDomainDestroy) vs. VIR_FROM_NONE (such as
virDomainPMSuspendForDuration). After this patch, all uses
consistently use VIR_FROM_NONE, on the grounds that it is unlikely
that a caller learning that a call is denied can do anything in
particular with extra knowledge which error domain the call belongs
to (similar to what we did in commit baa7244).
* src/util/virerror.c (virErrorMsg): Rework OPERATION_DENIED error
message.
* src/internal.h (virCheckReadOnlyGoto): New macro.
* src/util/virerror.h (virReportRestrictedError): New macro.
* src/libvirt-lxc.c: Use new macros.
* src/libvirt-qemu.c: Likewise.
* src/libvirt.c: Likewise.
* src/locking/lock_daemon.c (virLockDaemonClientNew): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
We weren't very consistent in our use of VIR_ERR_NO_SUPPORT; many
users just passed __FUNCTION__ on, while others passed "%s" to
silence over-eager compilers that warn about __FUNCTION__ not
containing any %. It's nicer to route all these uses through
a single macro, so that if we ever need to change the reporting,
we can do it in one place.
I verified that 'virsh -c test:///default qemu-monitor-command test foo'
gives the same error message before and after this patch:
error: this function is not supported by the connection driver: virDomainQemuMonitorCommand
Note that in libvirt.c, we were inconsistent on whether virDomain*
API used virLibConnError() (with VIR_FROM_NONE) or virLibDomainError()
(with VIR_FROM_DOMAIN); this patch unifies these errors to all use
VIR_FROM_NONE, on the grounds that it is unlikely that a caller
learning that a call is unimplemented can do anything in particular
with extra knowledge of which error domain it belongs to.
One particular change to note is virDomainOpenGraphics which was
trying to fail with VIR_ERR_NO_SUPPORT after a failed
VIR_DRV_SUPPORTS_FEATURE check; all other places that fail a
feature check report VIR_ERR_ARGUMENT_UNSUPPORTED.
* src/util/virerror.h (virReportUnsupportedError): New macro.
* src/libvirt-qemu.c: Use new macro.
* src/libvirt-lxc.c: Likewise.
* src/lxc/lxc_driver.c: Likewise.
* src/security/security_manager.c: Likewise.
* src/util/virinitctl.c: Likewise.
* src/libvirt.c: Likewise.
(virDomainOpenGraphics): Use correct error for unsupported feature.
Signed-off-by: Eric Blake <eblake@redhat.com>
We document that calling any public API wipes out all prior
libvirt errors in the same thread; but weren't obeying this
style in a few functions.
There are a couple of nested uses of virConnectRef (in lxc
and qemu reboot paths), but they should not be affected by
this change in semantics since there should not be any
previous error getting nuked (a later patch will clean up
the nested calls, along with abuse of virConnectClose on
cleanup paths which DOES nuke errors).
* src/libvirt.c (virGetVersion, virConnectRef, virDomainRef)
(virDomainGetSecurityLabel, virDomainGetSecurityLabelList)
(virDomainSetMetadata, virDomainGetMetadata)
(virNodeGetSecurityModel, virNetworkRef, virInterfaceRef)
(virStoragePoolRef, virStorageVolRef, virNodeDeviceGetName)
(virNodeDeviceRef, virSecretRef, virStreamRef, virNWFilterRef)
(virDomainSnapshotRef): Reset error on entrance.
(do_open): Drop redundant error reset.
* src/libvirt-qemu.c (virDomainQemuAgentCommand): Likewise.
* src/libvirt-lxc.c (virDomainLxcEnterNamespace)
(virDomainLxcEnterSecurityLabel): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
I noticed that the virDomainQemuMonitorCommand debug output wasn't
telling me the name of the domain it was working on. While it was
easy enough to determine which pointer matches the domain based on
other log messages, it is nicer to be consistent.
* src/util/viruuid.h (VIR_UUID_DEBUG): Moved here from...
* src/libvirt.c (VIR_UUID_DEBUG): ...here.
(VIR_ARG15, VIR_HAS_COMMA, VIR_DOMAIN_DEBUG_EXPAND)
(VIR_DOMAIN_DEBUG_PASTE, VIR_DOMAIN_DEBUG_0, VIR_DOMAIN_DEBUG_1)
(VIR_DOMAIN_DEBUG_2, VIR_DOMAIN_DEBUG): Move...
* src/datatypes.h: ...here.
* src/libvirt-qemu.c (virDomainQemuMonitorCommand)
(virDomainQemuAgentCommand): Better debug messages.
* src/libvirt-lxc.c (virDomainLxcOpenNamespace): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The virLibConnError macros in libvirt-lxc.c and
libvirt-qemu.c were passing NULL for the filename.
This causes a crash if the logging code is configured
to use journald.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Ensure that the driver struct field names match the public
API names. For an API virXXXX we must have a driver struct
field xXXXX. ie strip the leading 'vir' and lowercase any
leading uppercase letters.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Directories python/tools/examples should include them in <> form,
though this patch allows "" form in these directories by excluding
them, a later patch will do the cleanup.
https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.
* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/; If/. If/
Per the FSF address could be changed from time to time, and GNU
recommends the following now: (http://www.gnu.org/licenses/gpl-howto.html)
You should have received a copy of the GNU General Public License
along with Foobar. If not, see <http://www.gnu.org/licenses/>.
This patch removes the explicit FSF address, and uses above instead
(of course, with inserting 'Lesser' before 'General').
Except a bunch of files for security driver, all others are changed
automatically, the copyright for securify files are not complete,
that's why to do it manually:
src/security/security_selinux.h
src/security/security_driver.h
src/security/security_selinux.c
src/security/security_apparmor.h
src/security/security_apparmor.c
src/security/security_driver.c
To ensure consistent error reporting of invalid arguments,
provide a number of predefined helper methods & macros.
- An arg which must not be NULL:
virCheckNonNullArgReturn(argname, retvalue)
virCheckNonNullArgGoto(argname, label)
- An arg which must be NULL
virCheckNullArgGoto(argname, label)
- An arg which must be positive (ie 1 or greater)
virCheckPositiveArgGoto(argname, label)
- An arg which must not be 0
virCheckNonZeroArgGoto(argname, label)
- An arg which must be zero
virCheckZeroArgGoto(argname, label)
- An arg which must not be negative (ie 0 or greater)
virCheckNonNegativeArgGoto(argname, label)
* src/libvirt.c, src/libvirt-qemu.c,
src/nodeinfo.c, src/datatypes.c: Update to use
virCheckXXXX macros
* po/POTFILES.in: Add libvirt-qemu.c and virterror_internal.h
* src/internal.h: Define macros for checking invalid args
* src/util/virterror_internal.h: Define macros for reporting
invalid args
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
No thanks to 64-bit windows, with 64-bit pid_t, we have to avoid
constructs like 'int pid'. Our API in libvirt-qemu cannot be
changed without breaking ABI; but then again, libvirt-qemu can
only be used on systems that support UNIX sockets, which rules
out Windows (even if qemu could be compiled there) - so for all
points on the call chain that interact with this API decision,
we require a different variable name to make it clear that we
audited the use for safety.
Adding a syntax-check rule only solves half the battle; anywhere
that uses printf on a pid_t still needs to be converted, but that
will be a separate patch.
* cfg.mk (sc_correct_id_types): New syntax check.
* src/libvirt-qemu.c (virDomainQemuAttach): Document why we didn't
use pid_t for pid, and validate for overflow.
* include/libvirt/libvirt-qemu.h (virDomainQemuAttach): Tweak name
for syntax check.
* src/vmware/vmware_conf.c (vmwareExtractPid): Likewise.
* src/driver.h (virDrvDomainQemuAttach): Likewise.
* tools/virsh.c (cmdQemuAttach): Likewise.
* src/remote/qemu_protocol.x (qemu_domain_attach_args): Likewise.
* src/qemu_protocol-structs (qemu_domain_attach_args): Likewise.
* src/util/cgroup.c (virCgroupPidCode, virCgroupKillInternal):
Likewise.
* src/qemu/qemu_command.c(qemuParseProcFileStrings): Likewise.
(qemuParseCommandLinePid): Use pid_t for pid.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* src/conf/domain_conf.h (_virDomainObj): Likewise.
* src/probes.d (rpc_socket_new): Likewise.
* src/qemu/qemu_command.h (qemuParseCommandLinePid): Likewise.
* src/qemu/qemu_driver.c (qemudGetProcessInfo, qemuDomainAttach):
Likewise.
* src/qemu/qemu_process.c (qemuProcessAttach): Likewise.
* src/qemu/qemu_process.h (qemuProcessAttach): Likewise.
* src/uml/uml_driver.c (umlGetProcessInfo): Likewise.
* src/util/virnetdev.h (virNetDevSetNamespace): Likewise.
* src/util/virnetdev.c (virNetDevSetNamespace): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput): Likewise.
* src/conf/storage_conf.h (_virStoragePerms): Use mode_t, uid_t,
and gid_t rather than int.
* src/security/security_dac.c (virSecurityDACSetOwnership): Likewise.
* src/conf/storage_conf.c (virStorageDefParsePerms): Avoid
compiler warning.
The new doc text had a few readability issues. Also, the
monitor command text copied a bit too much from the attach case.
* src/libvirt-qemu.c (virDomainQemuMonitorCommand)
(virDomainQemuAttach): Fix typos and grammar.
Continuation of commit 313ac7fd, and enforce things with a syntax
check.
Technically, virNetServerClientCalculateHandleMode is not printing
a mode_t, but rather a collection of VIR_EVENT_HANDLE_* bits;
however, these bits are < 8, so there is no different in the
output, and that was the easiest way to silence the new syntax check.
* cfg.mk (sc_flags_debug): New syntax check.
(exclude_file_name_regexp--sc_flags_debug): Add exemptions.
* src/fdstream.c (virFDStreamOpenFileInternal): Print flags in
hex, mode_t in octal.
* src/libvirt-qemu.c (virDomainQemuMonitorCommand)
(virDomainQemuAttach): Likewise.
* src/locking/lock_driver_nop.c (virLockManagerNopInit): Likewise.
* src/locking/lock_driver_sanlock.c (virLockManagerSanlockInit):
Likewise.
* src/locking/lock_manager.c: Likewise.
* src/qemu/qemu_migration.c: Likewise.
* src/qemu/qemu_monitor.c: Likewise.
* src/rpc/virnetserverclient.c
(virNetServerClientCalculateHandleMode): Print mode with %o.
Introduce a new API in libvirt-qemu.so
virDomainPtr virDomainQemuAttach(virConnectPtr domain,
unsigned long long pid,
unsigned int flags);
This allows libvirtd to attach to an existing, externally
launched QEMU process. This is useful for QEMU developers who
prefer to launch QEMU themselves for debugging/devel reasons,
but still want the benefit of libvirt based tools like
virt-top, virt-viewer, etc
* include/libvirt/libvirt-qemu.h: Define virDomainQemuAttach
* src/driver.h, src/libvirt-qemu.c, src/libvirt_qemu.syms:
Driver glue for virDomainQemuAttach
Done mechanically with:
$ git grep -l '\bDEBUG0\? *(' | xargs -L1 sed -i 's/\bDEBUG0\? *(/VIR_&/'
followed by manual deletion of qemudDebug in daemon/libvirtd.c, along
with a single 'make syntax-check' fallout in the same file, and the
actual deletion in src/util/logging.h.
* src/util/logging.h (DEBUG, DEBUG0): Delete.
* daemon/libvirtd.h (qemudDebug): Likewise.
* global: Change remaining clients over to VIR_DEBUG counterpart.
Add the library entry point for the new virDomainQemuMonitorCommand()
entry point. Because this is not part of the "normal" libvirt API,
it gets its own header file, library file, and will eventually
get its own over-the-wire protocol later in the series.
Changes since v1:
- Go back to using the virDriver table for qemuDomainMonitorCommand, due to
linking issues
- Added versioning information to the libvirt-qemu.so
Changes since v2:
- None
Changes since v3:
- Add LGPL header to libvirt-qemu.c
- Make virLibConnError and virLibDomainError macros instead of function calls
Changes since v4:
- Move exported symbols to libvirt_qemu.syms
Signed-off-by: Chris Lalancette <clalance@redhat.com>
