[libvirt] [PATCH 0/4] Support for sending signals to guest processes
One of the things people want to be able todo with LXC is to send arbitrary signals to processes inside the container. This series introduces a virDomainSendProcessSignal API for that purpose. The LXC driver impl is limited to sending signals to the "init" pid (ie pid == 1), but in the future we'll extend this to arbitrary container PIDs (via /proc/$PID/ns/pid + setns()). If the QEMU guest agent had a suitable command we could support this for QEMU too.
From: "Daniel P. Berrange" <berrange@redhat.com> 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 | 60 ++++++++++++++++++++++++++++++++++++ src/driver.h | 7 +++++ src/libvirt.c | 72 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 4 files changed, 144 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 84dcde1..38ffebb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2989,6 +2989,66 @@ int virDomainSendKey(virDomainPtr domain, unsigned int flags); /* + * These just happen to match Linux signal numbers. The numbers + * will be mapped to whatever the SIGNUM is in the guest OS in + * question by the agent delivering the signal. The names are + * based on the POSIX / XSI signal standard though. + * + * Values over VIR_DOMAIN_PROCESS_SIGNAL_RTMIN are allowed, + * and will be mapped to the corresponding offset of the + * OS specific SIGRTMIN value + */ +typedef enum { + VIR_DOMAIN_PROCESS_SIGNAL_NOP = 0, /* No constant in POSIX/Linux */ + VIR_DOMAIN_PROCESS_SIGNAL_HUP = 1, /* SIGHUP */ + VIR_DOMAIN_PROCESS_SIGNAL_INT = 2, /* SIGINT */ + VIR_DOMAIN_PROCESS_SIGNAL_QUIT = 3, /* SIGQUIT */ + VIR_DOMAIN_PROCESS_SIGNAL_ILL = 4, /* SIGILL */ + VIR_DOMAIN_PROCESS_SIGNAL_TRAP = 5, /* SIGTRAP */ + VIR_DOMAIN_PROCESS_SIGNAL_ABRT = 6, /* SIGABRT */ + VIR_DOMAIN_PROCESS_SIGNAL_BUS = 7, /* SIGBUS */ + VIR_DOMAIN_PROCESS_SIGNAL_FPE = 8, /* SIGFPE */ + VIR_DOMAIN_PROCESS_SIGNAL_KILL = 9, /* SIGKILL */ + VIR_DOMAIN_PROCESS_SIGNAL_USR1 = 10, /* SIGUSR1 */ + VIR_DOMAIN_PROCESS_SIGNAL_SEGV = 11, /* SIGSEGV */ + VIR_DOMAIN_PROCESS_SIGNAL_USR2 = 12, /* SIGUSR2 */ + VIR_DOMAIN_PROCESS_SIGNAL_PIPE = 13, /* SIGPIPE */ + VIR_DOMAIN_PROCESS_SIGNAL_ALRM = 14, /* SIGALRM */ + VIR_DOMAIN_PROCESS_SIGNAL_TERM = 15, /* SIGTERM */ + VIR_DOMAIN_PROCESS_SIGNAL_STKFLT = 16, /* Not in POSIX (SIGSTKFLT on Linux )*/ + VIR_DOMAIN_PROCESS_SIGNAL_CHLD = 17, /* SIGCHLD */ + VIR_DOMAIN_PROCESS_SIGNAL_CONT = 18, /* SIGCONT */ + VIR_DOMAIN_PROCESS_SIGNAL_STOP = 19, /* SIGSTOP */ + VIR_DOMAIN_PROCESS_SIGNAL_TSTP = 20, /* SIGTSTP */ + VIR_DOMAIN_PROCESS_SIGNAL_TTIN = 21, /* SIGTTIN */ + VIR_DOMAIN_PROCESS_SIGNAL_TTOU = 22, /* SIGTTOU */ + VIR_DOMAIN_PROCESS_SIGNAL_URG = 23, /* SIGURG */ + VIR_DOMAIN_PROCESS_SIGNAL_XCPU = 24, /* SIGXCPU */ + VIR_DOMAIN_PROCESS_SIGNAL_XFSZ = 25, /* SIGXFSZ */ + VIR_DOMAIN_PROCESS_SIGNAL_VTALRM = 26, /* SIGVTALRM */ + VIR_DOMAIN_PROCESS_SIGNAL_PROF = 27, /* SIGPROF */ + VIR_DOMAIN_PROCESS_SIGNAL_WINCH = 28, /* Not in POSIX (SIGWINCH on Linux) */ + VIR_DOMAIN_PROCESS_SIGNAL_POLL = 29, /* SIGPOLL (also known as SIGIO on Linux) */ + VIR_DOMAIN_PROCESS_SIGNAL_PWR = 30, /* Not in POSIX (SIGPWR on Linux */ + VIR_DOMAIN_PROCESS_SIGNAL_SYS = 31, /* SIGSYS (also known as SIGUNUSED on Linux ) */ + + VIR_DOMAIN_PROCESS_SIGNAL_RTMIN = 32, /* SIGRTMIN */ + +#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 + */ + VIR_DOMAIN_PROCESS_SIGNAL_LAST +#endif +} virDomainProcessSignal; + +int virDomainSendProcessSignal(virDomainPtr domain, + unsigned int pid_value, + unsigned int signum, + unsigned int flags); + +/* * Deprecated calls */ virDomainPtr virDomainCreateLinux (virConnectPtr conn, diff --git a/src/driver.h b/src/driver.h index 7ba66ad..de58628 100644 --- a/src/driver.h +++ b/src/driver.h @@ -732,6 +732,12 @@ typedef int int nkeycodes, unsigned int flags); +typedef int + (*virDrvDomainSendProcessSignal)(virDomainPtr dom, + unsigned int pid_value, + unsigned int signum, + unsigned int flags); + typedef char * (*virDrvDomainMigrateBegin3) (virDomainPtr domain, @@ -1094,6 +1100,7 @@ struct _virDriver { virDrvNodeGetMemoryParameters nodeGetMemoryParameters; virDrvNodeSetMemoryParameters nodeSetMemoryParameters; virDrvNodeGetCPUMap nodeGetCPUMap; + virDrvDomainSendProcessSignal domainSendProcessSignal; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 4af6089..83550cd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8589,6 +8589,78 @@ error: return -1; } + +/** + * virDomainSendProcessSignal: + * @domain: pointer to domain object + * @pid_value: a positive integer process ID greater than zero + * @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. + * + * The @pid_value must specify a process ID greater than zero. The + * @pid_value numbers are from the container/guest namespace. + * + * If the @signum is VIR_DOMAIN_PROCESS_SIGNAL_NOP then this + * API will simply report whether the process is running in + * the container/guest + * + * Returns 0 in case of success, -1 in case of failure. + */ +int virDomainSendProcessSignal(virDomainPtr domain, + unsigned int pid_value, + unsigned int signum, + unsigned int flags) +{ + virConnectPtr conn; + VIR_DOMAIN_DEBUG(domain, "pid=%u, signum=%u flags=%x", + pid_value, signum, flags); + + virResetLastError(); + + if (pid_value == 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainSendProcessSignal) { + int ret; + ret = conn->driver->domainSendProcessSignal(domain, + pid_value, + signum, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + + /** * virDomainSetVcpus: * @domain: pointer to domain object, or NULL for Domain0 diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f494821..302fce5 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -574,4 +574,9 @@ LIBVIRT_1.0.0 { virNodeGetCPUMap; } LIBVIRT_0.10.2; +LIBVIRT_1.0.1 { + global: + virDomainSendProcessSignal; +} LIBVIRT_1.0.0; + # .... define new API here using predicted next version number .... -- 1.7.11.7
On 11/28/12 12:31, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
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 | 60 ++++++++++++++++++++++++++++++++++++ src/driver.h | 7 +++++ src/libvirt.c | 72 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 4 files changed, 144 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c index 4af6089..83550cd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8589,6 +8589,78 @@ error: return -1; }
+ +/** + * virDomainSendProcessSignal: + * @domain: pointer to domain object + * @pid_value: a positive integer process ID greater than zero + * @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. + * + * The @pid_value must specify a process ID greater than zero. The + * @pid_value numbers are from the container/guest namespace. + * + * If the @signum is VIR_DOMAIN_PROCESS_SIGNAL_NOP then this + * API will simply report whether the process is running in + * the container/guest
s/guest/guest./
+ * + * Returns 0 in case of success, -1 in case of failure. + */ +int virDomainSendProcessSignal(virDomainPtr domain, + unsigned int pid_value, + unsigned int signum, + unsigned int flags) +{ + virConnectPtr conn; + VIR_DOMAIN_DEBUG(domain, "pid=%u, signum=%u flags=%x", + pid_value, signum, flags); + + virResetLastError(); + + if (pid_value == 0) {
Use virCheckNonZeroArgGoto here.
+ virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + }
ACK to the code. As this is a API change, please wait a day or two for other possible comments to the semantic of this API. Peter
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 ---
/* + * These just happen to match Linux signal numbers. The numbers + * will be mapped to whatever the SIGNUM is in the guest OS in + * question by the agent delivering the signal. The names are + * based on the POSIX / XSI signal standard though. + * + * Values over VIR_DOMAIN_PROCESS_SIGNAL_RTMIN are allowed, + * and will be mapped to the corresponding offset of the + * OS specific SIGRTMIN value
+ VIR_DOMAIN_PROCESS_SIGNAL_SYS = 31, /* SIGSYS (also known as SIGUNUSED on Linux ) */ + + VIR_DOMAIN_PROCESS_SIGNAL_RTMIN = 32, /* SIGRTMIN */ + +#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 + */ + VIR_DOMAIN_PROCESS_SIGNAL_LAST +#endif +} virDomainProcessSignal;
I'm not comfortable with this; there are only a bounded number of signals allowed over SIGRTMIN (namely, up to SIGRTMAX); POSIX requires at least 8 real-time signals, but at the moment, Linux has 32 and 32-bit cygwin has only 1 (work is underway on building 64-bit cygwin which will copy Linux in having 32 real-time signals). The problem with having an unbounded number of realtime signals as the end of our list is that we cannot ever add extensions for any other signals on non-Linux hosts. We are unlikely to hit any implementation with more than 64 named signal values. Maybe we just bite the bullet and call out 32 VIR_DOMAIN_PROCESS_SIGNAL_REALTIME_NNN values, at which point, the VIR_DOMAIN_PROCESS_SIGNAL_LAST sentinel really is a sentinel and we are free to add more enums later if porting to a different platform. The other thing I'm worried about is that you didn't document what happens if a user passes a VIR_DOMAIN_PROCESS_SIGNAL_ value that we can't map to the underlying implementation (of course, it won't happen for Linux to Linux; but again, if we extend this to other platforms, then it is very reasonable to expect that VIR_DOMAIN_PROCESS_SIGNAL_STKFLT must be rejected if the guest OS doesn't have a STKFLT signal, or that realtime signals must fit within the bounds of the guest OS, even if those bounds differ from the Linux default of 32 levels).
+int virDomainSendProcessSignal(virDomainPtr domain, + unsigned int pid_value,
Do we really want 'unsigned int' pid_value, or do we want to allow the user to send signals to process groups (negative pid_t)? Also, should we make the parameter 'long long int' in order to accomodate 64-bit Windows having 64-bit pid_t?
+typedef int + (*virDrvDomainSendProcessSignal)(virDomainPtr dom, + unsigned int pid_value, + unsigned int signum, + unsigned int flags);
Obviously, this has to reflect any type changes made in the public API.
+/** + * virDomainSendProcessSignal: + * @domain: pointer to domain object + * @pid_value: a positive integer process ID greater than zero
Why greater than zero? Is there any reason to forbid killing a process group?
+ * @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. + * + * The @pid_value must specify a process ID greater than zero. The + * @pid_value numbers are from the container/guest namespace.
Maybe document that some hypervisors restrict which @pid_value can be used, to cover the fact that your initial LXC implementation requires @pid_value of 1.
+++ b/src/libvirt_public.syms @@ -574,4 +574,9 @@ LIBVIRT_1.0.0 { virNodeGetCPUMap; } LIBVIRT_0.10.2;
+LIBVIRT_1.0.1 { + global: + virDomainSendProcessSignal;
Potential merge conflicts here, but that's nothing too hard. I'm generally in favor of a new API along these lines, but am worried that your initial implementation may be painting us into portability corners.
From: "Daniel P. Berrange" <berrange@redhat.com> * src/remote/remote_protocol.x: message definition * src/remote/remote_driver.c: Register driver function * src/remote_protocol-structs: Test case --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 10 +++++++++- src/remote_protocol-structs | 7 +++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ec33698..9d04cc3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6124,6 +6124,7 @@ static virDriver remote_driver = { .domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */ .domainSendKey = remoteDomainSendKey, /* 0.9.3 */ + .domainSendProcessSignal = remoteDomainSendProcessSignal, /* 0.9.8 */ .domainBlockJobAbort = remoteDomainBlockJobAbort, /* 0.9.4 */ .domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d6ac3c1..56ef20e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1019,6 +1019,13 @@ struct remote_domain_send_key_args { unsigned int flags; }; +struct remote_domain_send_process_signal_args { + remote_nonnull_domain dom; + unsigned int pid_value; + unsigned int signum; + unsigned int flags; +}; + struct remote_domain_set_vcpus_args { remote_nonnull_domain dom; unsigned int nvcpus; @@ -3026,7 +3033,8 @@ enum remote_procedure { REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */ - REMOTE_PROC_NODE_GET_CPU_MAP = 293 /* skipgen skipgen */ + REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 294 /* autogen autogen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 6fe7213..1256d89 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -672,6 +672,12 @@ struct remote_domain_send_key_args { } keycodes; u_int flags; }; +struct remote_domain_send_process_signal_args { + remote_nonnull_domain dom; + u_int pid_value; + u_int signum; + u_int flags; +}; struct remote_domain_set_vcpus_args { remote_nonnull_domain dom; u_int nvcpus; @@ -2433,4 +2439,5 @@ enum remote_procedure { REMOTE_PROC_NETWORK_UPDATE = 291, REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, REMOTE_PROC_NODE_GET_CPU_MAP = 293, + REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 294, }; -- 1.7.11.7
On 11/28/12 12:31, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
* src/remote/remote_protocol.x: message definition * src/remote/remote_driver.c: Register driver function * src/remote_protocol-structs: Test case --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 10 +++++++++- src/remote_protocol-structs | 7 +++++++ 3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ec33698..9d04cc3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6124,6 +6124,7 @@ static virDriver remote_driver = { .domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */ .domainSendKey = remoteDomainSendKey, /* 0.9.3 */ + .domainSendProcessSignal = remoteDomainSendProcessSignal, /* 0.9.8 */
s/0.9.8/1.0.1/
.domainBlockJobAbort = remoteDomainBlockJobAbort, /* 0.9.4 */ .domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d6ac3c1..56ef20e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1019,6 +1019,13 @@ struct remote_domain_send_key_args { unsigned int flags; };
+struct remote_domain_send_process_signal_args { + remote_nonnull_domain dom; + unsigned int pid_value; + unsigned int signum; + unsigned int flags; +}; + struct remote_domain_set_vcpus_args { remote_nonnull_domain dom; unsigned int nvcpus; @@ -3026,7 +3033,8 @@ enum remote_procedure {
REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */ - REMOTE_PROC_NODE_GET_CPU_MAP = 293 /* skipgen skipgen */ + REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 294 /* autogen autogen priority:high */
When we will be implementing this for qemu with the guest agent we will need to set this to low priority. Is there a need to have this high priority due to LXC?
/* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 6fe7213..1256d89 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -672,6 +672,12 @@ struct remote_domain_send_key_args { } keycodes; u_int flags; }; +struct remote_domain_send_process_signal_args { + remote_nonnull_domain dom; + u_int pid_value; + u_int signum; + u_int flags; +}; struct remote_domain_set_vcpus_args { remote_nonnull_domain dom; u_int nvcpus; @@ -2433,4 +2439,5 @@ enum remote_procedure { REMOTE_PROC_NETWORK_UPDATE = 291, REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, REMOTE_PROC_NODE_GET_CPU_MAP = 293, + REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 294, };
From: "Daniel P. Berrange" <berrange@redhat.com>
* src/remote/remote_protocol.x: message definition * src/remote/remote_driver.c: Register driver function * src/remote_protocol-structs: Test case
Of course, this all depends on any changes made to 1/4 based on my review there; but it is fairly mechanical and matches (for now). However...
+++ b/src/remote/remote_driver.c @@ -6124,6 +6124,7 @@ static virDriver remote_driver = { .domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */ .domainSendKey = remoteDomainSendKey, /* 0.9.3 */ + .domainSendProcessSignal = remoteDomainSendProcessSignal, /* 0.9.8 */
s/0.9.8/1.0.1/
@@ -3026,7 +3033,8 @@ enum remote_procedure {
REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */ - REMOTE_PROC_NODE_GET_CPU_MAP = 293 /* skipgen skipgen */ + REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 294 /* autogen autogen priority:high */
Is this right? If we implement this via a guest agent command for qemu, then we must block waiting for a response from the agent, at which point I think we can no longer guarantee high priority.
From: "Daniel P. Berrange" <berrange@redhat.com> * tools/virsh.c: Add send-process-signal * tools/virsh.pod: Document new command --- tools/virsh-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 27 +++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cc47383..454cccf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5901,6 +5901,98 @@ cleanup: } /* + * "send-process-signal" command + */ +static const vshCmdInfo info_send_process_signal[] = { + {"help", N_("Send signals to processes") }, + {"desc", N_("Send signals to processes in the guest") }, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_send_process_signal[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"pid", VSH_OT_DATA, VSH_OFLAG_REQ, N_("the process ID") }, + {"signame", VSH_OT_DATA, VSH_OFLAG_REQ, N_("the signal number or name") }, + {NULL, 0, 0, NULL} +}; + +VIR_ENUM_DECL(virDomainProcessSignal) +VIR_ENUM_IMPL(virDomainProcessSignal, + VIR_DOMAIN_PROCESS_SIGNAL_LAST, + "nop", "hup", "int", "quit", "ill", /* 0-4 */ + "trap", "abrt", "bus", "fpe", "kill", /* 5-9 */ + "usr1", "segv", "usr2", "pipe", "alrm", /* 10-14 */ + "term", "stkflt", "chld", "cont", "stop", /* 15-19 */ + "tstp", "ttin", "ttou", "urg", "xcpu", /* 20-24 */ + "xfsz", "vtalrm", "prof", "winch", "poll", /* 25-29 */ + "pwr", "sys", "rtmin") /* 30-32 */ + +static int getSignalNumber(const char *signame) +{ + int signum; + + if (virStrToLong_i(signame, NULL, 10, &signum) >= 0) + return signum; + + if (STRPREFIX(signame, "sig")) + signame += 3; + + if ((signum = virDomainProcessSignalTypeFromString(signame)) >= 0) + return signum; + + if (STRPREFIX(signame, "rtmin+")) { + signame += 6; + if (virStrToLong_i(signame, NULL, 10, &signum) >= 0) + return signum + VIR_DOMAIN_PROCESS_SIGNAL_RTMIN; + } + + return -1; +} + +static bool +cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int ret = false; + const char *pidstr; + const char *signame; + unsigned int pid_value; + int signum; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "pid", &pidstr) <= 0) { + vshError(ctl, "%s", _("missing argument")); + return false; + } + + if (vshCommandOptString(cmd, "signame", &signame) <= 0) { + vshError(ctl, "%s", _("missing argument")); + return false; + } + + if (virStrToLong_ui(pidstr, NULL, 10, &pid_value) < 0) { + vshError(ctl, _("malformed PID value: %s"), pidstr); + goto cleanup; + } + + if ((signum = getSignalNumber(signame)) < 0) { + vshError(ctl, _("malformed signal name: %s"), signame); + goto cleanup; + } + + if (virDomainSendProcessSignal(dom, pid_value, signum, 0) < 0) + goto cleanup; + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + +/* * "setmem" command */ static const vshCmdInfo info_setmem[] = { @@ -8353,6 +8445,7 @@ const vshCmdDef domManagementCmds[] = { {"edit", cmdEdit, opts_edit, info_edit, 0}, {"inject-nmi", cmdInjectNMI, opts_inject_nmi, info_inject_nmi, 0}, {"send-key", cmdSendKey, opts_send_key, info_send_key, 0}, + {"send-process-signal", cmdSendProcessSignal, opts_send_process_signal, info_send_process_signal, 0}, {"managedsave", cmdManagedSave, opts_managedsave, info_managedsave, 0}, {"managedsave-remove", cmdManagedSaveRemove, opts_managedsaveremove, info_managedsaveremove, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index c4d505f..84c0f53 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1379,6 +1379,33 @@ B<Examples> # send a tab, held for 1 second virsh send-key --holdtime 1000 0xf +=item B<send-process-signal> I<domain-id> [I<--host-pid>] I<pid> I<signame> + +Send a signal I<signame> to the process identified by I<pid> running in +the virtual domain I<domain-id>. The I<pid> is a process ID in the virtual +domain namespace. + +The I<signame> argument may be either an integer signal constant number, +or one of the symbolic names: + + "nop", "hup", "int", "quit", "ill", + "trap", "abrt", "bus", "fpe", "kill", + "usr1", "segv", "usr2", "pipe", "alrm", + "term", "stkflt", "chld", "cont", "stop", + "tstp", "ttin", "ttou", "urg", "xcpu", + "xfsz", "vtalrm", "prof", "winch", "poll", + "pwr", "sys", "rtmin" + +The symbol name may optionally be prefixed with 'sig'. The 'rtmin' signal +also allows an optional suffix "+<number>" to refer to other real time +signals. + +B<Examples> + virsh send-process-signal myguest 1 15 + virsh send-process-signal myguest 1 term + virsh send-process-signal myguest 1 sigterm + virsh send-process-signal myguest 1 rtmin+12 + =item B<setmem> I<domain> B<size> [[I<--config>] [I<--live>] | [I<--current>]] -- 1.7.11.7
On 11/28/12 12:31, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
* tools/virsh.c: Add send-process-signal * tools/virsh.pod: Document new command --- tools/virsh-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 27 +++++++++++++++ 2 files changed, 120 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cc47383..454cccf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5901,6 +5901,98 @@ cleanup: }
/* + * "send-process-signal" command + */ +static const vshCmdInfo info_send_process_signal[] = { + {"help", N_("Send signals to processes") }, + {"desc", N_("Send signals to processes in the guest") }, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_send_process_signal[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"pid", VSH_OT_DATA, VSH_OFLAG_REQ, N_("the process ID") }, + {"signame", VSH_OT_DATA, VSH_OFLAG_REQ, N_("the signal number or name") }, + {NULL, 0, 0, NULL} +}; + +VIR_ENUM_DECL(virDomainProcessSignal) +VIR_ENUM_IMPL(virDomainProcessSignal, + VIR_DOMAIN_PROCESS_SIGNAL_LAST, + "nop", "hup", "int", "quit", "ill", /* 0-4 */ + "trap", "abrt", "bus", "fpe", "kill", /* 5-9 */ + "usr1", "segv", "usr2", "pipe", "alrm", /* 10-14 */ + "term", "stkflt", "chld", "cont", "stop", /* 15-19 */ + "tstp", "ttin", "ttou", "urg", "xcpu", /* 20-24 */ + "xfsz", "vtalrm", "prof", "winch", "poll", /* 25-29 */ + "pwr", "sys", "rtmin") /* 30-32 */ + +static int getSignalNumber(const char *signame)
I'd prefix the function name with vsh as with other helpers.
+{ + int signum; + + if (virStrToLong_i(signame, NULL, 10, &signum) >= 0) + return signum; + + if (STRPREFIX(signame, "sig")) + signame += 3; + + if ((signum = virDomainProcessSignalTypeFromString(signame)) >= 0) + return signum; + + if (STRPREFIX(signame, "rtmin+")) { + signame += 6; + if (virStrToLong_i(signame, NULL, 10, &signum) >= 0) + return signum + VIR_DOMAIN_PROCESS_SIGNAL_RTMIN; + } + + return -1; +} + +static bool +cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int ret = false; + const char *pidstr; + const char *signame; + unsigned int pid_value; + int signum; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "pid", &pidstr) <= 0) { + vshError(ctl, "%s", _("missing argument"));
You should print the name of the flag that has missing argument here as you happen to know it.
+ return false; + } + + if (vshCommandOptString(cmd, "signame", &signame) <= 0) { + vshError(ctl, "%s", _("missing argument"));
same here.
+ return false; + } + + if (virStrToLong_ui(pidstr, NULL, 10, &pid_value) < 0) { + vshError(ctl, _("malformed PID value: %s"), pidstr); + goto cleanup; + } + + if ((signum = getSignalNumber(signame)) < 0) { + vshError(ctl, _("malformed signal name: %s"), signame); + goto cleanup; + } + + if (virDomainSendProcessSignal(dom, pid_value, signum, 0) < 0) + goto cleanup; + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + +/* * "setmem" command */ static const vshCmdInfo info_setmem[] = { @@ -8353,6 +8445,7 @@ const vshCmdDef domManagementCmds[] = { {"edit", cmdEdit, opts_edit, info_edit, 0}, {"inject-nmi", cmdInjectNMI, opts_inject_nmi, info_inject_nmi, 0}, {"send-key", cmdSendKey, opts_send_key, info_send_key, 0}, + {"send-process-signal", cmdSendProcessSignal, opts_send_process_signal, info_send_process_signal, 0}, {"managedsave", cmdManagedSave, opts_managedsave, info_managedsave, 0}, {"managedsave-remove", cmdManagedSaveRemove, opts_managedsaveremove, info_managedsaveremove, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index c4d505f..84c0f53 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1379,6 +1379,33 @@ B<Examples> # send a tab, held for 1 second virsh send-key --holdtime 1000 0xf
+=item B<send-process-signal> I<domain-id> [I<--host-pid>] I<pid> I<signame> + +Send a signal I<signame> to the process identified by I<pid> running in +the virtual domain I<domain-id>. The I<pid> is a process ID in the virtual +domain namespace. + +The I<signame> argument may be either an integer signal constant number, +or one of the symbolic names: + + "nop", "hup", "int", "quit", "ill", + "trap", "abrt", "bus", "fpe", "kill", + "usr1", "segv", "usr2", "pipe", "alrm", + "term", "stkflt", "chld", "cont", "stop", + "tstp", "ttin", "ttou", "urg", "xcpu", + "xfsz", "vtalrm", "prof", "winch", "poll", + "pwr", "sys", "rtmin" + +The symbol name may optionally be prefixed with 'sig'. The 'rtmin' signal +also allows an optional suffix "+<number>" to refer to other real time +signals. + +B<Examples> + virsh send-process-signal myguest 1 15 + virsh send-process-signal myguest 1 term + virsh send-process-signal myguest 1 sigterm + virsh send-process-signal myguest 1 rtmin+12 + =item B<setmem> I<domain> B<size> [[I<--config>] [I<--live>] | [I<--current>]]
Looks good. ACK with the nits fixed. Peter
From: "Daniel P. Berrange" <berrange@redhat.com>
* tools/virsh.c: Add send-process-signal * tools/virsh.pod: Document new command --- tools/virsh-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 27 +++++++++++++++ 2 files changed, 120 insertions(+)
+static int getSignalNumber(const char *signame) +{ + int signum; + + if (virStrToLong_i(signame, NULL, 10, &signum) >= 0) + return signum; + + if (STRPREFIX(signame, "sig")) + signame += 3;
Please make this case-insensitive. You just know some users will want to type 'SIGKILL' instead of 'kill', and still have it map to the right libvirt enum value.
+ + if (STRPREFIX(signame, "rtmin+")) { + signame += 6; + if (virStrToLong_i(signame, NULL, 10, &signum) >= 0) + return signum + VIR_DOMAIN_PROCESS_SIGNAL_RTMIN;
Okay, you did take care of realtime signals. However, should you do any validation that they aren't doing stupid stuff like 'rtmin+ -2'?
+++ b/tools/virsh.pod @@ -1379,6 +1379,33 @@ B<Examples> # send a tab, held for 1 second virsh send-key --holdtime 1000 0xf
+=item B<send-process-signal> I<domain-id> [I<--host-pid>] I<pid> I<signame> + +Send a signal I<signame> to the process identified by I<pid> running in +the virtual domain I<domain-id>. The I<pid> is a process ID in the virtual +domain namespace. + +The I<signame> argument may be either an integer signal constant number,
Should we mention that these constants are hard-coded by libvirt, and might not mention the signal numbers on either the client running virsh, or the guest where the signal will be executed?
From: "Daniel P. Berrange" <berrange@redhat.com> Implement the new API for sending signals to processes in a guest for the LXC driver. Only support sending signals to the init process for now, because - The kernel does not appear to expose the mapping between container PID numbers and host PID numbers anywhere in the host OS namespace - There is no race-free validate whether a host PID corresponds to a process in a container. * src/lxc/lxc_driver.c: Allow sending processes signals --- src/lxc/lxc_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 991b593..c9b8d86 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2666,6 +2666,78 @@ cleanup: return ret; } + +static int +lxcDomainSendProcessSignal(virDomainPtr dom, + unsigned int pid_value, + unsigned int signum, + unsigned int flags) +{ + virLXCDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virLXCDomainObjPrivatePtr priv; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + pid_t victim; + int ret = -1; + + virCheckFlags(0, -1); + + lxcDriverLock(driver); + virUUIDFormat(dom->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + lxcDriverUnlock(driver); + if (!vm) { + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + priv = vm->privateData; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + /* + * XXX if the kernel has /proc/$PID/ns/pid we can + * switch into container namespace & that way be + * able to kill any PID. Alternatively if there + * is a way to find a mapping of guest<->host PIDs + * we can kill that way. + */ + if (pid_value != 1) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Only the init process may be killed")); + goto cleanup; + } + + if (!priv->initpid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Init pid is not yet available")); + goto cleanup; + } + victim = priv->initpid; + + /* We're relying on fact libvirt header signal numbers + * are taken from Linux, to avoid mapping + */ + if (kill(victim, signum) < 0) { + virReportSystemError(errno, + _("Unable to send %d signal to process %d"), + signum, victim); + goto cleanup; + } + + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + + static int lxcListAllDomains(virConnectPtr conn, virDomainPtr **domains, @@ -2751,6 +2823,7 @@ static virDriver lxcDriver = { .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */ .nodeGetMemoryParameters = nodeGetMemoryParameters, /* 0.10.2 */ .nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */ + .domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */ }; static virStateDriver lxcStateDriver = { -- 1.7.11.7
On 11/28/12 12:31, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Implement the new API for sending signals to processes in a guest for the LXC driver. Only support sending signals to the init process for now, because
- The kernel does not appear to expose the mapping between container PID numbers and host PID numbers anywhere in the host OS namespace - There is no race-free validate whether a host PID corresponds to a process in a container.
* src/lxc/lxc_driver.c: Allow sending processes signals --- src/lxc/lxc_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+)
ACK. Peter
Implement the new API for sending signals to processes in a guest for the LXC driver. Only support sending signals to the init process for now, because
- The kernel does not appear to expose the mapping between container PID numbers and host PID numbers anywhere in the host OS namespace - There is no race-free validate whether a host PID corresponds
s/race-free/race-free way to/
+ /* We're relying on fact libvirt header signal numbers + * are taken from Linux, to avoid mapping
Cute, but at least you documented your assumption. Should you validate that signum <= SIGRTMAX, though, for a nicer error than what kill() would give for out-of-range signum?
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Peter Krempa