From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ECD82C433DB for ; Mon, 1 Mar 2021 16:17:53 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7331264ED5 for ; Mon, 1 Mar 2021 16:17:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7331264ED5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:55044 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lGlF2-0008JB-CZ for qemu-devel@archiver.kernel.org; Mon, 01 Mar 2021 11:17:52 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:44928) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lGlDT-0006U3-Eh for qemu-devel@nongnu.org; Mon, 01 Mar 2021 11:16:15 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:29991) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lGlDR-0004iF-CQ for qemu-devel@nongnu.org; Mon, 01 Mar 2021 11:16:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1614615372; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=t5WBE1vDL3JPlITbTuw9lR2ZY2cmoxPwaf1EnE9FtZo=; b=bHsLI37ivS0+V224pZDd10rzXH5b4s9so1Of6Tyjto6om26Yqhhn/ZtXt9GGCpVCsrHW+y dYAnM3UeT/94MuNZjtKE5YlkOallRidgxzQQdjLsrggxHdq8YR28xXuk2JppituSOZFj+I 8w5FanGF810ZM2ktkAg0lZIZFf5hFI8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-270-2RbtwPrkOa69R_jJUFd7ag-1; Mon, 01 Mar 2021 11:15:58 -0500 X-MC-Unique: 2RbtwPrkOa69R_jJUFd7ag-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8EA0B1E567; Mon, 1 Mar 2021 16:15:57 +0000 (UTC) Received: from redhat.com (ovpn-113-132.ams2.redhat.com [10.36.113.132]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9CE661F478; Mon, 1 Mar 2021 16:15:49 +0000 (UTC) Date: Mon, 1 Mar 2021 16:15:47 +0000 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= To: Stefan Hajnoczi Subject: Re: [PATCH] qemu-storage-daemon: add --pidfile option Message-ID: References: <20210301160857.130478-1-stefanha@redhat.com> MIME-Version: 1.0 In-Reply-To: <20210301160857.130478-1-stefanha@redhat.com> User-Agent: Mutt/2.0.5 (2021-01-21) X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=berrange@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Received-SPF: pass client-ip=63.128.21.124; envelope-from=berrange@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Cc: afrosi@redhat.com, Kevin Wolf , qemu-devel@nongnu.org, qemu-block@nongnu.org, "Richard W . M . Jones" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Mon, Mar 01, 2021 at 04:08:57PM +0000, Stefan Hajnoczi wrote: > Daemons often have a --pidfile option where the pid is written to a file > so that scripts can stop the daemon by sending a signal. > > The pid file also acts as a lock to prevent multiple instances of the > daemon from launching for a given pid file. > > QEMU, qemu-nbd, qemu-ga, virtiofsd, and qemu-pr-helper all support the > --pidfile option. Add it to qemu-storage-daemon too. > > Reported-by: Richard W.M. Jones > Signed-off-by: Stefan Hajnoczi > --- > docs/tools/qemu-storage-daemon.rst | 10 ++++++++++ > storage-daemon/qemu-storage-daemon.c | 29 ++++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+) > > diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst > index f63627eaf6..8f4ab16ffc 100644 > --- a/docs/tools/qemu-storage-daemon.rst > +++ b/docs/tools/qemu-storage-daemon.rst > @@ -118,6 +118,16 @@ Standard options: > List object properties with ``,help``. See the :manpage:`qemu(1)` > manual page for a description of the object properties. > > +.. option:: --pidfile PATH > + > + is the path to a file where the daemon writes its pid. This allows scripts to > + stop the daemon by sending a signal:: > + > + $ kill -SIGTERM $( + > + A file lock is applied to the file so only one instance of the daemon can run > + with a given pid file path. The daemon unlinks its pid file when terminating. Usually a pidfile wants to have some explicit synchronization rules defined. AFAICS, qsd doesn't have a --daemonize option to sync against. If we're using the FD passing trick for the monitor, however, we want a guarantee that the pidfile is written before the monitor accepts the first client. IOW, the parent process needs to know that once it has done the QMP handshake, there is guaranteed tobe a pidfile present, or if the QMP handshake fails, then the app is guaranteed to have quit. IIUC, this impl should be ok in this respect, because we won't process the QMP handdshake until the main loop runs, at which point the pidfile is present. So we just need to document that the pidfile is guaranteed to be written by the time QMP is active. > + > Examples > -------- > Launch the daemon with QMP monitor socket ``qmp.sock`` so clients can execute > diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c > index 9021a46b3a..011ae49ac3 100644 > --- a/storage-daemon/qemu-storage-daemon.c > +++ b/storage-daemon/qemu-storage-daemon.c > @@ -59,6 +59,7 @@ > #include "sysemu/runstate.h" > #include "trace/control.h" > > +static const char *pid_file; > static volatile bool exit_requested = false; > > void qemu_system_killed(int signal, pid_t pid) > @@ -126,6 +127,7 @@ enum { > OPTION_MONITOR, > OPTION_NBD_SERVER, > OPTION_OBJECT, > + OPTION_PIDFILE, > }; > > extern QemuOptsList qemu_chardev_opts; > @@ -164,6 +166,7 @@ static void process_options(int argc, char *argv[]) > {"monitor", required_argument, NULL, OPTION_MONITOR}, > {"nbd-server", required_argument, NULL, OPTION_NBD_SERVER}, > {"object", required_argument, NULL, OPTION_OBJECT}, > + {"pidfile", required_argument, NULL, OPTION_PIDFILE}, > {"trace", required_argument, NULL, 'T'}, > {"version", no_argument, NULL, 'V'}, > {0, 0, 0, 0} > @@ -275,6 +278,9 @@ static void process_options(int argc, char *argv[]) > qobject_unref(args); > break; > } > + case OPTION_PIDFILE: > + pid_file = optarg; > + break; > default: > g_assert_not_reached(); > } > @@ -285,6 +291,27 @@ static void process_options(int argc, char *argv[]) > } > } > > +static void pid_file_cleanup(void) > +{ > + unlink(pid_file); > +} > + > +static void pid_file_init(void) > +{ > + Error *err = NULL; > + > + if (!pid_file) { > + return; > + } > + > + if (!qemu_write_pidfile(pid_file, &err)) { > + error_reportf_err(err, "cannot create PID file: "); > + exit(EXIT_FAILURE); > + } > + > + atexit(pid_file_cleanup); > +} > + > int main(int argc, char *argv[]) > { > #ifdef CONFIG_POSIX > @@ -312,6 +339,8 @@ int main(int argc, char *argv[]) > qemu_init_main_loop(&error_fatal); > process_options(argc, argv); > > + pid_file_init(); > + > while (!exit_requested) { > main_loop_wait(false); > } > -- > 2.29.2 > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|