<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
On 04/15/2016 04:48 AM, Dmitry Mishin wrote:<br>
<blockquote cite="mid:D336ADAC.C54C5%25dim@virtuozzo.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<span style="font-weight:bold"></span><span
style="font-weight:bold"></span>I am reviewing recent VZ7
libploop commits, in particular, this one:<br>
<span id="OLK_SRC_BODY_SECTION">
<blockquote id="MAC_OUTLOOK_ATTRIBUTION_BLOCKQUOTE"
style="BORDER-LEFT: #b5c4df 5 solid; PADDING:0 0 0 5; MARGIN:0
0 0 5;">
<div>
<div text="#000000" bgcolor="#FFFFFF">
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://src.openvz.org/projects/OVZ/repos/ploop/commits/36df847b9">
https://src.openvz.org/projects/OVZ/repos/ploop/commits/36df847b9</a><br>
<br>
I left a question there, let me repeat it here in a hope
someone answers.<br>
<br>
<blockquote type="cite">
<div class="changeset-badge-oneline" style="margin: 0px;
padding: 0px; display: inline-block; font-size: 14px;
color: rgb(51, 51, 51); font-family: Arial,
sans-serif; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: 20px; orphans: auto; text-align: left;
text-indent: 0px; text-transform: none; white-space:
normal; widows: 1; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color:
rgb(255, 255, 255);">
<span class="changeset-author" title="Igor Sukhih"
style="font-weight: bold; margin-right: 5px; color:
rgb(51, 51, 51);">Igor Sukhih</span><span
class="Apple-converted-space"> </span>committed<span
class="Apple-converted-space"> </span><a
moz-do-not-send="true" class="changesetid"
href="https://src.openvz.org/projects/OVZ/repos/ploop/commits/36df847b99c92557c69255ebfb00d4cc74cb51ac"
data-changeset-id="36df847b99c92557c69255ebfb00d4cc74cb51ac"
original-title="" style="color: rgb(59, 115, 175);
text-decoration: none; font-family: monospace;
display: inline-block; vertical-align: baseline;
margin-left: 5px;">36df847b99c</a><time title="13
April 2016 05:04 PM"
datetime="2016-04-13T17:04:27+0000"
style="margin-left: 5px; font-size: 12px; color:
rgb(112, 112, 112);">Yesterday</time></div>
<div class="commit-message" style="margin: 10px 0px 0px;
padding: 0px; color: rgb(51, 51, 51); font-family:
Arial, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 20px; orphans:
auto; text-align: left; text-indent: 0px;
text-transform: none; white-space: normal; widows: 1;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255);">
<pre style="margin: 0px; padding: 0px; white-space: pre-wrap;">ploop_copy_init(): open folder with O_DIRECTORY flag
</pre>
</div>
<br class="Apple-interchange-newline">
</blockquote>
...<br>
<blockquote type="cite"><tt>- _h->mntfd =
open(mnt, O_RDONLY);</tt><tt><br>
</tt><tt>+</tt><tt> _h->mntfd = open(mnt,
O_RDONLY|O_NONBLOCK|O_DIRECTORY);</tt><tt><br>
</tt></blockquote>
<br>
1. What's the reason for adding O_NONBLOCK here? As far as
I can see, it doesn't<br>
change anything at all (neither in this open(), nor in
subsequent syncfs(), ioctl()<br>
and close())? I went as far as the kernel sources to check
that O_NONBLOCK<br>
doesn't affect syncfs() call, but maybe I'm mistaken?</div>
</div>
</blockquote>
</span>
<div><br>
</div>
<div>Added accidentally, you are right, it doesn't change
anything.</div>
<div><br>
</div>
<span id="OLK_SRC_BODY_SECTION">
<blockquote id="MAC_OUTLOOK_ATTRIBUTION_BLOCKQUOTE"
style="BORDER-LEFT: #b5c4df 5 solid; PADDING:0 0 0 5; MARGIN:0
0 0 5;">
<div>
<div text="#000000" bgcolor="#FFFFFF"><br>
<br>
2. What's the reason for adding O_DIRECTORY? Ideally, the
changelog<br>
should say why we're doing it, not what we do (as it's
pretty clear<br>
from the patch itself).<br>
<br>
</div>
</div>
</blockquote>
</span>
<div>It does exactly as specified – enforces open file to be a
directory. </div>
</blockquote>
<br>
See, I am not asking what it does -- it's pretty clear from open(2)
man page.<br>
<br>
What I am interested in is the reason _why_ it was added. A bug
report<br>
this change is trying to fix, or a train of thought leading to this
change, etc.<br>
Ideally this is what the changelog message should tell us.<br>
<br>
<blockquote cite="mid:D336ADAC.C54C5%25dim@virtuozzo.com"
type="cite">
<div>Any problems with this enforcement?</div>
</blockquote>
<br>
No, I don't foresee any problems, more to say, this extra check
might<br>
actually do some good.<br>
<br>
Kir.<br>
<br>
</body>
</html>