From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 63D6DC77B61 for ; Thu, 27 Apr 2023 13:45:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243511AbjD0NpM (ORCPT ); Thu, 27 Apr 2023 09:45:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51382 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242993AbjD0NpL (ORCPT ); Thu, 27 Apr 2023 09:45:11 -0400 Received: from siwi.pair.com (siwi.pair.com [209.68.5.199]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1557319AC for ; Thu, 27 Apr 2023 06:45:10 -0700 (PDT) Received: from siwi.pair.com (localhost [127.0.0.1]) by siwi.pair.com (Postfix) with ESMTP id 89E02CA1253; Thu, 27 Apr 2023 09:45:08 -0400 (EDT) Received: from [IPV6:2600:1700:840:e768:69d8:c39b:c604:54bf] (unknown [IPv6:2600:1700:840:e768:69d8:c39b:c604:54bf]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by siwi.pair.com (Postfix) with ESMTPSA id 4B875CC839C; Thu, 27 Apr 2023 09:45:08 -0400 (EDT) Message-ID: Date: Thu, 27 Apr 2023 09:45:07 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2] fsmonitor: handle differences between Windows named pipe functions Content-Language: en-US From: Jeff Hostetler To: Eric DeCosta , Eric DeCosta via GitGitGadget , "git@vger.kernel.org" Cc: Johannes Schindelin References: <48d44c06-34ba-0a1f-dd0d-7d66bd8dfcb9@jeffhostetler.com> In-Reply-To: <48d44c06-34ba-0a1f-dd0d-7d66bd8dfcb9@jeffhostetler.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Scanned-By: mailmunge 3.11 on 209.68.5.199 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On 4/26/23 4:33 PM, Jeff Hostetler wrote: ... >>> >>> + /* UNC Path, skip leading slash */ >>> + if (realpath.buf[0] == '/' && realpath.buf[1] == '/') real_off = 1; >>> + >>> off = swprintf(wpath, alloc, L"\\\\.\\pipe\\"); >>> - if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0) >>> + if (xutftowcs(wpath + off, realpath.buf + real_off, alloc - off) < 0) >>> return -1; > > I haven't had a chance to test this, but this does look > like a minimal solution for the pathname confusion in the > MSFT APIs. > > Do you need to test for realpath.buf[0] and [1] being a forward OR > a backslash ? > > Should we set real_off to 2 rather than 1 because we already > appended a trailing backslash in the swprintf() ? On second thought, you might actually need the second slash in //./pipe//server/mount/whatever so that the GfW installer can find remote repo path to CWD and stop the daemon. Testing will tell here. Jeff > > You should run one of those NPFS directory listing tools to > confirm the exact spelling of the pipe matches your expectation > here.  Yes, if both functions now work, we should be good, but > it would be good to confirm your real_off choice, right? > > If would be good to (at least interactively) test that the > git-for-windows installer can find the path and stop the daemon > on an upgrade or uninstall.  See Johannes' earlier point. > > We should state somewhere that we are running the fsmonitor > daemon locally and it is watching a remote file system. > > You should run a few stress tests to ensure that the > MAX_RDCW_BUF_FALLBACK throttling works and that the daemon > doesn't fall behind on a very busy remote file system.  (There > are SMB/CIFS wire protocol limits.  See the source.)  (I did > test this between the combination of systems that I had, but > YMMV.) > > During the stress test, it would also be good to test that > IO generated by a client process on your local machine to the > remote file system is reported, but also that random IO from > remote processes on the remote system are seen in the event > stream.  Again, I tested the combinations of machines that I > had available at the time. > > Hope this helps, > Jeff > > >>> >>> /* Handle drive prefix */ >>> >>> base-commit: f285f68a132109c234d93490671c00218066ace9 >>> -- >>> gitgitgadget >> >> Are there any other thoughts about this? >> >> I believe that this is the simplest change possible that will ensure that >> fsmonitor correctly handles network repos. >> >> -Eric