From: "eblake@redhat.com" <eblake@redhat.com>
To: Raphael Norwitz <raphael.norwitz@nutanix.com>
Cc: "peter.maydell@linaro.org" <peter.maydell@linaro.org>,
"mst@redhat.com" <mst@redhat.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
"mreitz@redhat.com" <mreitz@redhat.com>,
"stefanha@redhat.com" <stefanha@redhat.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"sgarzare@redhat.com" <sgarzare@redhat.com>
Subject: Re: [PATCH 1/2] storage-daemon: add opt to print when initialized
Date: Mon, 30 Aug 2021 11:05:35 -0500 [thread overview]
Message-ID: <20210830160535.e65icwcf4bbvq333@redhat.com> (raw)
In-Reply-To: <20210830155611.GA23709@raphael-debian-dev>
On Mon, Aug 30, 2021 at 03:56:16PM +0000, Raphael Norwitz wrote:
> On Fri, Aug 27, 2021 at 01:51:48PM -0500, eblake@redhat.com wrote:
> > On Fri, Aug 27, 2021 at 04:50:35PM +0000, Raphael Norwitz wrote:
> > > This change adds a command line option to print a line to standard out
> > > when the storage daemon has completed initialization and is ready to
> > > serve client connections.
> > >
> > > This option will be used to resolve a hang in the vhost-user-blk-test.
> >
> > Doesn't the existing --pidfile already serve the same job? That is,
> > why not fix vhost-user-blk-test to take advantage of the pid-file
> > creation rather than output to stdout as evidence of when the storage
> > daemon is up and running?
> >
> > Therefore, I don't think we need this patch.
> >
>
> Sure - that make sense. I didn't use the pid-file because I didn't want to
> risk leaving junk on the filesystem if the storage-daemon crashed.
Ideally, storage-daemon doesn't crash during the test. But even if it
does, we should still be able to register which files will be cleaned
up while exiting the test (if they exist), regardless of whether the
test succeeded or failed, because we have control over the pidfile
name before starting storage-daemon. Put another way, the task of
cleaning up a pidfile during a test should not be a show-stopper.
[Side note: A long time ago, there were patches submitted to make the
iotests ./check engine run EVERY test in its own subdirectory, so that
cleaning up all files created by the test was trivial: nuke the
directory. It also has the benefit that for debugging a failing test,
you merely pass an option to ./check that says to not nuke the
directory. But it did not get applied at the time, and we have had
enough changes in the meantime that reinstating such a useful patch
would basically be work from scratch at this point]
>
> I'll send a V2 using pid-file without this change.
Thanks, looking forward to it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
next prev parent reply other threads:[~2021-08-30 16:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-27 16:50 [PATCH 1/2] storage-daemon: add opt to print when initialized Raphael Norwitz
2021-08-27 16:50 ` [PATCH 2/2] Prevent vhost-user-blk-test hang Raphael Norwitz
2021-08-27 18:55 ` eblake
2021-08-27 18:51 ` [PATCH 1/2] storage-daemon: add opt to print when initialized eblake
2021-08-30 15:56 ` Raphael Norwitz
2021-08-30 16:05 ` eblake [this message]
2021-08-30 21:51 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210830160535.e65icwcf4bbvq333@redhat.com \
--to=eblake@redhat.com \
--cc=mreitz@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=raphael.norwitz@nutanix.com \
--cc=sgarzare@redhat.com \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).