All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
To: "Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Cc: containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Subject: Re: [PATCH] [RFC] Add c/r support for listening INET sockets
Date: Tue, 29 Sep 2009 12:56:56 -0700	[thread overview]
Message-ID: <87r5tpo77b.fsf@caffeine.danplanet.com> (raw)
In-Reply-To: <20090929195025.GA14956-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> (Serge E. Hallyn's message of "Tue\, 29 Sep 2009 14\:50\:25 -0500")

>> +	if ((h->sock_common.family == AF_INET) &&
>> +	    (h->sock.state != TCP_LISTEN)) {
>> +		/* Temporary hack to enable restore of TCP_LISTEN sockets
>> +		 * while forcing anything else to a closed state
>> +		 */
>> +		sock->sk->sk_state = TCP_CLOSE;

SH> I'll admit by this point I'm a bit confused about the order in
SH> which these things are restored.  Does do_sock_restore() run
SH> before inet_restore() below (curious whether its check for
SH> TCP_CLOSE will catch these particular sockets).

It runs after inet_restore(), which was called above via
sock->ops->restore().  Thus, inet_restore() can largely ignore the
connected sockets and then they're forced closed here (since
inet_restore() runs before the common restore routine, otherwise it
could set them closed itself).

>> +int inet_checkpoint(struct ckpt_ctx *ctx, struct socket *sock)
>> +{
>> +	struct ckpt_hdr_socket_inet *in;
>> +	int ret = -EINVAL;
>> +
>> +       in = ckpt_hdr_get_type(ctx, sizeof(*in), CKPT_HDR_SOCKET_INET);
>> +       if (!in)
>> +               goto out;

SH> Better to just return...  I don't think this'll cause a crash (it
SH> only makes ckpt_hdr_put() pass ((struct ckpt_hdr *)NULL)->len to
SH> _ckpt_hdr_put which then ignores it and does a safe kfree(NULL)),
SH> but it won't be safe if the implementation of the ckpt_hdr_*()
SH> calls end up changing.

Sure.  Also note my broken whitespace that came as a result of me
cutting patches into pieces.  I'll fix that too.

-- 
Dan Smith
IBM Linux Technology Center
email: danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org

  parent reply	other threads:[~2009-09-29 19:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-28 18:58 [PATCH] [RFC] Add c/r support for listening INET sockets Dan Smith
     [not found] ` <1254164330-23072-1-git-send-email-danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-29 19:50   ` Serge E. Hallyn
     [not found]     ` <20090929195025.GA14956-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-29 19:56       ` Dan Smith [this message]
2009-09-29 21:49   ` Oren Laadan
     [not found]     ` <4AC280D6.3020302-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-30 14:20       ` Dan Smith
2009-09-29 22:16   ` Oren Laadan

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=87r5tpo77b.fsf@caffeine.danplanet.com \
    --to=danms-r/jw6+rmf7hqt0dzr+alfa@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org \
    --cc=serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.