Re: [libvirt] [PATCH] test_driver: implement virDomainSendProcessSignal

On Tue, Jun 04, 2019 at 03:17:43PM +0200, Ilias Stamatis wrote: > Only succeed when @pid_value is 1, since according to the docs this is Why do we need to restrict ourselves for @pid 1 in the test driver? This restriction exists in LXC for a reason, but why in test driver? a) it's only a check with @pid not being actually used b) each guest OS will have their own limit in /proc/sys/kernel/pid_max for the max number of PID so restricting it from the top doesn't make sense c) -@pid is used for process groups, so again, this will be handled within the guest OS in reality

With the check removed: Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; > the minimum requirement for any driver to implement this API. > > >From man 2 kill: > The only signals that can be sent to process ID 1, the init process, are > those for which init has explicitly installed signal handlers. > > Regarding sending SIGTERM and SIGKILL to init, the test driver domains > pretend to run Linux, and on Linux init/systemd explicitly ignores these > signals. > > Correspondingly, we can assume that no signal handlers are installed for > any other signal and succeed by doing nothing. > > Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com&gt; > --- > > Alternatively, we could succeed when @pid_value is any number or belongs > to a set of specific numbers. > > src/test/test_driver.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > mode change 100644 => 100755 src/test/test_driver.c > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > old mode 100644 > new mode 100755 > index cae2521b21..490a605a77 > --- a/src/test/test_driver.c > +++ b/src/test/test_driver.c > @@ -2825,6 +2825,40 @@ static int testDomainSetMetadata(virDomainPtr dom, > return ret; > } > > +static int > +testDomainSendProcessSignal(virDomainPtr dom, > + long long pid_value, > + unsigned int signum, > + unsigned int flags) > +{ > + int ret = -1; > + virDomainObjPtr vm = NULL; > + > + virCheckFlags(0, -1); > + > + if (pid_value != 1) { > + virReportError(VIR_ERR_INVALID_ARG, > + _("only sending a signal to pid 1 is supported")); > + return -1; > + } > + > + if (signum >= VIR_DOMAIN_PROCESS_SIGNAL_LAST) { > + virReportError(VIR_ERR_INVALID_ARG, > + _("signum value %d is out of range"), > + signum); > + return -1; > + } > + > + if (!(vm = testDomObjFromDomain(dom))) > + goto cleanup; > + > + /* do nothing */ > + ret = 0; > + > + cleanup: > + virDomainObjEndAPI(&vm); > + return ret; > +} > > static int testNodeGetCellsFreeMemory(virConnectPtr conn, > unsigned long long *freemems, > @@ -7027,6 +7061,7 @@ static virHypervisorDriver testHypervisorDriver = { > .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ > .domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */ > .domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */ > + .domainSendProcessSignal = testDomainSendProcessSignal, /* 5.5.0 */ > .connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */ > .domainManagedSave = testDomainManagedSave, /* 1.1.4 */ > .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.4 */ > -- > 2.21.0 > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list