Re: [libvirt] [PATCH v2 1/4] Add virDomainSendProcessSignal API
Add an API for sending signals to arbitrary processes in the
guest OS. This is primarily useful for container based virt,
but can be used for machine virt too, if there is a suitable
guest agent,
* include/libvirt/libvirt.h.in: Add virDomainSendProcessSignal
and virDomainProcessSignal enum
* src/driver.h: Driver entry point
* src/libvirt.c, src/libvirt_public.syms: Impl for new API
---
include/libvirt/libvirt.h.in | 97
++++++++++++++++++++++++++++++++++++++++++++
src/driver.h | 7 ++++
src/libvirt.c | 84
++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 1 +
4 files changed, 189 insertions(+)
+ *
+ * Do not rely on all values matching Linux though. It is possible
Why two spaces mid-sentence?
+ VIR_DOMAIN_PROCESS_SIGNAL_RT32 = 64, /* SIGRTMIN + 32 /
SIGRTMAX */
Yes, this works better for me.
+
+#ifdef VIR_ENUM_SENTINELS
+ /* This is the "last" signal only in so much as none
+ * of the latter ones have named constants. They are
+ * just numeric offsets from RTMIN upwards
This comment is now out of date.
+/**
+ * virDomainSendProcessSignal:
+ * @domain: pointer to domain object
+ * @pid_value: a positive integer process ID, or negative integer
process group ID
+ * @signum: a signal from the virDomainProcessSignal enum
+ * @flags: one of the virDomainProcessSignalFlag values
+ *
+ * Send a signal to the designated process in the guest
+ *
+ * The signal numbers must be taken from the virDomainProcessSignal
+ * enum. These will be translated to the corresponding signal
+ * number for the guest OS, by the guest agent delivering the
+ * signal. If there is no mapping from virDomainProcessSignal to
+ * the native OS signals, this API will report an error
s/$/./
+ *
+ * If @pid_value is an integer greater than zero, it is
+ * treated as a process ID. If @pid_value is an integer
+ * less than zero, it is treated as a process group ID.
+ * All the @pid_value numbers are from the container/guest
+ * namespace. The value zero is not valid.
+ *
+ * Not all hypervisors will support sending signals to
+ * arbitrary processes or process groups. If this API is
+ * implemented the minimum requirement is to be able to
+ * use @pid_value==1 (ie kill init). No other value is
s/ie/i.e./
+
+ if (signum > VIR_DOMAIN_PROCESS_SIGNAL_LAST) {
+ virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ goto error;
+ }
Drop this hunk (rather, move it to patch 4/4). Such a check
should be done in the hypervisor driver, not in the entry
point. Otherwise, if we add a new signal later, this hunk
would prevent an older virsh from sending the new signal.
ACK with those changes.