qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Cc: "kwolf@redhat.com" <kwolf@redhat.com>,
	Denis Lunev <den@virtuozzo.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"stefanha@redhat.com" <stefanha@redhat.com>,
	"mreitz@redhat.com" <mreitz@redhat.com>
Subject: Re: [PATCH v9 3/3] iotests: test nbd reconnect
Date: Mon, 7 Oct 2019 15:03:57 -0500	[thread overview]
Message-ID: <0be52afc-ca1b-99bf-ca34-71f800d2ecba@redhat.com> (raw)
In-Reply-To: <b8f833ed-5c0f-7cba-0cd4-acb96c1afbc9@virtuozzo.com>

On 10/7/19 5:48 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> We want to wait until listening socket is prepared..
>>
>> In shell:
>>
>> qemu-nbd --pid-file=/path/to/file ...
>> while [ ! -e /path/to/file ]; do
>>     sleep ... # fractional second, or exponential, or whatever...
>> done
>> # Now the listening socket is indeed prepared
>>
>> You'd have to translate that idiom to python.
> 
> Don't see, how it is better than what I've done in 04.. But I can resend with this.
> At least, the fact that socket is initialized before creating pid file is undocumented..

I just posted a patch to rectify that.

> 
>>
>> Or:
>>
>> pre-open Unix socket at /path/to/socket
>> LISTEN_PID=... LISTEN_FDS=1 qemu-nbd ... 3<>/path/to/socket
>>
>> Now the socket is pre-created and passed into qemu-nbd via systemd socket activation, so you know the listening socket is ready without having to do any loop at all.  Here's a patch in libnbd where we just switched from waiting for the port to appear (because the test predated qemu-nbd pidfile support) to instead using socket activation, for reference:
>> https://github.com/libguestfs/libnbd/commit/352331d177
>>
> 
> Hmm, I'm afraid I need more help in it, I don't know socket activation and googling for some time didn't help.
> How to pre-open the socket? How to set LISTEN_PID? Looking at the code I see that activation path failed if
> LISTEN_PID != getpid().. So I need to know qemu-nbd pid before starting it? o_O
> 

I'm not sure if a shell can open a Unix socket as a server.  The shell 
_does_ have the 'exec' builtin, such that you can manipulate the 
environment in shell and then use exec to guarantee the qemu-nbd process 
has the same pid as the shell process that just manipulated the 
environment; but it doesn't help unless you can also pass in the Unix 
socket pre-bound, and I'm not aware of command line tools that make that 
easy (maybe nc can do something like it, but then you have to wonder if 
nc is adding yet another layer of bounce-buffering).  But in other 
languages (C, Python, ...) you create a Unix socket, call bind() on it, 
then call fork().  In the parent process, you then close the just-bound 
fd, and call connect() on the socket - the connect will block until the 
child process uses the socket, but you are guaranteed that it won't fail 
because the server side is already bound.  In the child process, you use 
dup2() to move the fd to 3 and clear O_CLOEXEC, manipulate the 
environment to set LISTEN_PID to the current process id and LISTEN_FDS 
to 1, then exec.  The child process then has the correct id to realize 
that it has been handed a pre-bound socket, and skips any code that it 
would normally do to create the socket and bind it.

Note that setting LISTEN_PID in the child process after a fork() from a 
multi-threaded parent is _extremely_ difficult to do correctly (setenv() 
is not async-signal-safe, and anything that is not async-signal-safe can 
cause deadlock if invoked between fork() and exec() if the parent 
process was multi-threaded) - so it may even be easier to do a 
double-exec() - the first exec is to a shim to get rid of the 
async-signal-safe restrictions, and can then set LISTEN_PID without 
issues, and the second exec to the actual target.  But I don't know of 
any such shim designed for common use.

That said, socket activation isn't a necessity to use, just a 
convenience.  I don't care if the regression test uses socket 
activation, as long as it works at testing nbd reconnect.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


  reply	other threads:[~2019-10-07 20:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-17 17:13 [Qemu-devel] [PATCH v9 0/3] NBD reconnect Vladimir Sementsov-Ogievskiy
2019-09-17 17:13 ` [Qemu-devel] [PATCH v9 1/3] qemu-coroutine-sleep: introduce qemu_co_sleep_wake Vladimir Sementsov-Ogievskiy
2019-09-23 14:53   ` Eric Blake
2019-09-17 17:13 ` [Qemu-devel] [PATCH v9 2/3] block/nbd: nbd reconnect Vladimir Sementsov-Ogievskiy
2019-09-23 19:23   ` Eric Blake
2019-09-24  8:05     ` Vladimir Sementsov-Ogievskiy
2019-10-04 17:29     ` Vladimir Sementsov-Ogievskiy
2019-09-17 17:13 ` [Qemu-devel] [PATCH v9 3/3] iotests: test " Vladimir Sementsov-Ogievskiy
2019-09-23 19:51   ` Eric Blake
2019-09-24  8:31     ` Vladimir Sementsov-Ogievskiy
2019-10-04 18:05       ` Eric Blake
2019-10-07 10:48         ` Vladimir Sementsov-Ogievskiy
2019-10-07 20:03           ` Eric Blake [this message]
2019-09-24  9:27 ` [PATCH v9 4/3] " Vladimir Sementsov-Ogievskiy

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=0be52afc-ca1b-99bf-ca34-71f800d2ecba@redhat.com \
    --to=eblake@redhat.com \
    --cc=den@virtuozzo.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.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).