All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Jason Andryuk <andryuk@aero.org>,
	Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	xen-devel@lists.xen.org
Subject: Re: [PATCH v2] libxl: Reset toolstack_save file position in libxl
Date: Wed, 21 May 2014 17:12:01 +0100	[thread overview]
Message-ID: <537CD051.4000901@citrix.com> (raw)
In-Reply-To: <21372.52866.217052.802311@mariner.uk.xensource.com>

On 21/05/14 17:04, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH v2] libxl: Reset toolstack_save file position in libxl"):
>> On 19/05/2014 19:36, Jason Andryuk wrote:
>>> toolstack_save data is written to a temporary file in libxl and read
>>> back in libxl-save-helper.  The file position must be reset prior to
>>> reading the file, which is done in libxl-save-helper with lseek.
>>>
>>> lseek is unsupported for pipes and sockets, so a wrapper passing such an
>>> fd to libxl-save-helper fails the lseek.  Moving the lseek to libxl
>>> avoids the error, allowing the save to continue.
>>>
>>> Signed-off-by: Jason Andryuk <andryuk@aero.org>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Ian: As noted previously, this should be a backport candidate.
> We normally do backports only for bugs (and not even all bugs).  I'm
> afraid that I don't see how the current arrangements are a bug.
>
> See my other mail.  Perhaps the answer to that will explain to me how
> the current are a bug, rather than simply less convenient and flexible
> than they could be.
>
> Thanks,
> Ian.

It is a layering violation.  libxl-save-helper cannot assume that the fd
is a file, so shouldn't seek on it.

libxl however should prepare a "socket-like" fd to the save-helper,
which means rewinding the file itself.


As indicated, "toolstack_data" is heading very much in the direction of
/dev/null with the migration v2 patches, as it itself is a gross
layering violation. If you don't deem this worthy for backport, its not
the end of the world.

~Andrew

  reply	other threads:[~2014-05-21 16:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-19 18:36 [PATCH v2] libxl: Reset toolstack_save file position in libxl Jason Andryuk
2014-05-19 22:07 ` Andrew Cooper
2014-05-21 16:04   ` Ian Jackson
2014-05-21 16:12     ` Andrew Cooper [this message]
2014-05-21 16:02 ` Ian Jackson
2014-05-22 12:07   ` Jason Andryuk
2014-05-22 15:33     ` Ian Jackson

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=537CD051.4000901@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=andryuk@aero.org \
    --cc=ian.campbell@citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xen.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.