4:19 a.m.
In the current Perl library we have a function called
resolve_windows_path which handles the useful [for Windows guests]
case where we need to resolve the true, case-sensitive path behind a
case-insensitive Windows path.
For example, a Windows path like "C:\Windows\System32" can be
presented to Linux by NTFS 3g in a variety of ways, eg:
/WINDOWS/system32
/WINDOWS/SYSTEM32
depending entirely on the inconsequential details of how it was
originally created. Windows doesn't care about case sensitivity, but
NTFS 3g does.
Since resolve_windows_path is a very useful function, the first patch
moves it into the daemon. The new API is called
"guestfs_case_sensitive_path".
The second patch adds some useful functionality to guestfish, so you
can use Windows-style paths, like this:
touch win:C:\windows\foo
(Note that all ordinary paths must begin with a '/' char, so this
can't conflict with any existing, working scripts).
The third patch deprecates resolve_windows_path in the Perl code and
replaces it with calls to $g->case_sensitive_path.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
4:20 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
5:41 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
Richard W.M. Jones wrote:
...
diff --git a/daemon/realpath.c b/daemon/realpath.c
...
Hi Rich,
@@ -42,3 +46,111 @@ do_realpath (const char *path)
return ret; /* caller frees */
}
+
+char *
+do_case_sensitive_path (const char *path)
+{
+ char ret[PATH_MAX+1] = "/";
+ size_t next = 1;
+
+ /* MUST chdir ("/") before leaving this function. */
+ if (chdir (sysroot) == -1) {
If this function might ever be used from a multi-threaded
application, then you'll want to change it not to use chdir,
since chdir changes the process-wide current directory.
That can cause rare but particularly hard to debug problems.
Another (albeit small) advantage of not performing the chdir/readdir is
that the function will be able to handle those rare names
with a directory component that grants "x" but not "r" access.
+ reply_with_perror ("%s", sysroot);
+ return NULL;
+ }
+
+ /* First character is a '/'. Take each subsequent path element
+ * and follow it.
+ */
+ path++;
+ while (*path) {
+ size_t i = strcspn (path, "/");
+ if (i == 0) { /* "//" in path */
+ path++;
+ continue;
+ }
+ if ((i == 1 && path[0] == '.') ||
+ (i == 2 && path[0] == '.' && path[1] == '.')) {
+ reply_with_error ("case_sensitive_path: path contained . or ..
elements");
+ goto error;
+ }
+ if (i > NAME_MAX) {
+ reply_with_error ("case_sensitive_path: path element too long");
+ goto error;
+ }
+
+ char name[NAME_MAX+1];
+ memcpy (name, path, i);
+ name[i] = '\0';
+
+ /* Skip to next element in path (for the next loop iteration). */
+ path += i;
+ if (*path == '/') path++;
+
+ /* Read the current directory looking (case insensitively) for
+ * this element of the path.
+ */
+ DIR *dir = opendir (".");
+ if (dir == NULL) {
+ reply_with_perror ("opendir");
+ goto error;
+ }
+
+ struct dirent *d = NULL;
+
+ while ((d = readdir (dir)) != NULL) {
If you want to distinguish readdir "EOF" from failure (e.g., EIO),
you'll have to set errno=0 before each call and test whether it's
still 0 upon NULL return.
+ if (strcasecmp (d->d_name, name) == 0)
+ break;
+ }
+
+ if (closedir (dir) == -1) {
+ reply_with_perror ("closedir");
+ goto error;
+ }
+
+ if (d == NULL) {
+ reply_with_error ("%s: no file or directory found with this name",
name);
+ goto error;
+ }
+
+ /* Add the real name of this path element to the return value. */
+ if (next > 1)
+ ret[next++] = '/';
+
+ i = strlen (d->d_name);
+ if (next + i >= PATH_MAX) {
+ reply_with_error ("final path too long");
+ goto error;
+ }
+
+ strcpy (&ret[next], d->d_name);
+ next += i;
+
+ /* Is it a directory? Try going into it. */
+ if (chdir (d->d_name) == 0)
+ continue;
+
+ /* This is OK provided we've reached the end of the path. */
+ if (errno == ENOTDIR) {
+ if (*path) {
+ reply_with_error ("non-directory element in path");
+ goto error;
+ }
+ break;
+ }
What about other errno values?
+ }
+
+ ignore_value (chdir ("/"));
+
+ ret[next] = '\0';
+ char *retp = strdup (ret);
+ if (retp == NULL) {
+ reply_with_perror ("strdup");
+ return NULL;
+ }
+ return retp; /* caller frees */
+
+ error:
+ ignore_value (chdir ("/"));
+ return NULL;
5:52 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On Mon, Oct 26, 2009 at 11:41:22AM +0100, Jim Meyering wrote:
Richard W.M. Jones wrote:
> +char *
> +do_case_sensitive_path (const char *path)
> +{
> + char ret[PATH_MAX+1] = "/";
> + size_t next = 1;
> +
> + /* MUST chdir ("/") before leaving this function. */
> + if (chdir (sysroot) == -1) {
If this function might ever be used from a multi-threaded
application, then you'll want to change it not to use chdir,
since chdir changes the process-wide current directory.
That can cause rare but particularly hard to debug problems.
This code would be much harder to write if we don't use chdir. Any
suggestions on that? If there was a "readdirat" variant then that
might be promising.
Note that the daemon isn't likely to be multithreaded any time soon
(more likely to be pre-forking multiprocess if anything).
Another (albeit small) advantage of not performing the chdir/readdir
is
that the function will be able to handle those rare names
with a directory component that grants "x" but not "r" access.
This shouldn't be an issue because the daemon always runs as root.
> + while ((d = readdir (dir)) != NULL) {
If you want to distinguish readdir "EOF" from failure (e.g., EIO),
you'll have to set errno=0 before each call and test whether it's
still 0 upon NULL return.
OK, I'll fix that.
> + /* This is OK provided we've reached the end of the
path. */
> + if (errno == ENOTDIR) {
> + if (*path) {
> + reply_with_error ("non-directory element in path");
> + goto error;
> + }
> + break;
> + }
What about other errno values?
.. and I'll fix that.
Thanks for looking at this.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
6:04 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On 26/10/09 10:52, Richard W.M. Jones wrote:
On Mon, Oct 26, 2009 at 11:41:22AM +0100, Jim Meyering wrote:
> Richard W.M. Jones wrote:
>> +char *
>> +do_case_sensitive_path (const char *path)
>> +{
>> + char ret[PATH_MAX+1] = "/";
>> + size_t next = 1;
>> +
>> + /* MUST chdir ("/") before leaving this function. */
>> + if (chdir (sysroot) == -1) {
>
> If this function might ever be used from a multi-threaded
> application, then you'll want to change it not to use chdir,
> since chdir changes the process-wide current directory.
> That can cause rare but particularly hard to debug problems.
This code would be much harder to write if we don't use chdir. Any
suggestions on that? If there was a "readdirat" variant then that
might be promising.
There's an fdopendir(), which returns the DIR* required by readdir().
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
6:11 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
Richard W.M. Jones wrote:
On Mon, Oct 26, 2009 at 11:41:22AM +0100, Jim Meyering wrote:
> Richard W.M. Jones wrote:
> > +char *
> > +do_case_sensitive_path (const char *path)
> > +{
> > + char ret[PATH_MAX+1] = "/";
> > + size_t next = 1;
> > +
> > + /* MUST chdir ("/") before leaving this function. */
> > + if (chdir (sysroot) == -1) {
>
> If this function might ever be used from a multi-threaded
> application, then you'll want to change it not to use chdir,
> since chdir changes the process-wide current directory.
> That can cause rare but particularly hard to debug problems.
This code would be much harder to write if we don't use chdir. Any
suggestions on that? If there was a "readdirat" variant then that
might be promising.
To traverse without chdir, you can use the combination
of openat and fdopendir, and then readdir as usual.
With those gnulib modules, you use both openat and fdopendir portably.
6:03 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On 26/10/09 09:20, Richard W.M. Jones wrote:
+char *
+do_case_sensitive_path (const char *path)
+{
+ char ret[PATH_MAX+1] = "/";
Is PATH_MAX on Windows <= POSIX PATH_MAX? Does ntfs_3g honour this?
Seems like a grey area. It would be safer to realloc this buffer as
necessary. You also wouldn't need the strdup() at the end.
+ size_t next = 1;
+
+ /* MUST chdir ("/") before leaving this function. */
+ if (chdir (sysroot) == -1) {
+ reply_with_perror ("%s", sysroot);
+ return NULL;
+ }
I'm not convinced chdir is necessary in this function if you use
openat() throughout.
+ /* First character is a '/'. Take each subsequent path
element
+ * and follow it.
+ */
A check that *path == '/' wouldn't go amiss here. I'd stick an assert
in, but then DV might shoot me ;)
+ path++;
+ while (*path) {
+ size_t i = strcspn (path, "/");
+ if (i == 0) { /* "//" in path */
+ path++;
+ continue;
+ }
+ if ((i == 1&& path[0] == '.') ||
+ (i == 2&& path[0] == '.'&& path[1] == '.')) {
+ reply_with_error ("case_sensitive_path: path contained . or ..
elements");
This string isn't internationalised. I won't mention all of these.
+ goto error;
+ }
+ if (i> NAME_MAX) {
+ reply_with_error ("case_sensitive_path: path element too long");
+ goto error;
+ }
+
+ char name[NAME_MAX+1];
+ memcpy (name, path, i);
+ name[i] = '\0';
+
+ /* Skip to next element in path (for the next loop iteration). */
+ path += i;
+ if (*path == '/') path++;
+
+ /* Read the current directory looking (case insensitively) for
+ * this element of the path.
+ */
+ DIR *dir = opendir (".");
+ if (dir == NULL) {
+ reply_with_perror ("opendir");
+ goto error;
+ }
+
+ struct dirent *d = NULL;
+
+ while ((d = readdir (dir)) != NULL) {
+ if (strcasecmp (d->d_name, name) == 0)
+ break;
+ }
+
+ if (closedir (dir) == -1) {
+ reply_with_perror ("closedir");
+ goto error;
+ }
+
+ if (d == NULL) {
+ reply_with_error ("%s: no file or directory found with this name",
name);
Non-internationalised string. This message is definitely for the user.
+ goto error;
+ }
+
+ /* Add the real name of this path element to the return value. */
+ if (next> 1)
+ ret[next++] = '/';
+
+ i = strlen (d->d_name);
+ if (next + i>= PATH_MAX) {
+ reply_with_error ("final path too long");
+ goto error;
+ }
+
+ strcpy (&ret[next], d->d_name);
+ next += i;
+
+ /* Is it a directory? Try going into it. */
+ if (chdir (d->d_name) == 0)
+ continue;
openat() and fstat() would be much nicer here.
+
+ /* This is OK provided we've reached the end of the path. */
+ if (errno == ENOTDIR) {
+ if (*path) {
+ reply_with_error ("non-directory element in path");
+ goto error;
+ }
+ break;
+ }
+ }
+
+ ignore_value (chdir ("/"));
+
+ ret[next] = '\0';
+ char *retp = strdup (ret);
+ if (retp == NULL) {
+ reply_with_perror ("strdup");
+ return NULL;
+ }
+ return retp; /* caller frees */
+
+ error:
+ ignore_value (chdir ("/"));
+ return NULL;
+}
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 0f11735..5381652 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-196
+197
diff --git a/src/generator.ml b/src/generator.ml
index cea5178..668002e 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -3645,6 +3645,45 @@ The result list is not sorted.
=back");
+ ("case_sensitive_path", (RString "rpath", [Pathname
"path"]), 197, [],
+ [InitISOFS, Always, TestOutput (
+ [["case_sensitive_path"; "/DIRECTORY"]],
"/directory");
+ InitISOFS, Always, TestOutput (
+ [["case_sensitive_path"; "/Known-1"]], "/known-1");
+ InitBasicFS, Always, TestOutput (
+ [["mkdir"; "/a"];
+ ["mkdir"; "/a/bbb"];
+ ["touch"; "/a/bbb/c"];
+ ["case_sensitive_path"; "/A/bbB/C"]], "/a/bbb/c")],
These don't exercise the following cases you've coded for:
/
/foo//bar
/foo/../bar
/foo///bar/
+ "return true path on case-insensitive filesystem",
+ "\
+This command handles a peculiarity of the Linux ntfs-3g
+filesystem driver (and probably others), which is that although
+the underlying filesystem is case-insensitive, the driver
+exports the filesystem to Linux as case-sensitive.
+
+One consequence of this is that special directories such
+as C<c:\\windows> may appear as C</WINDOWS> or C</windows>
+(or other things) depending on the precise details of how
+they were created. In Windows itself this would not be
+a problem.
+
+Bug or feature? You decide:
+L<http://www.tuxera.com/community/ntfs-3g-faq/#posixfilenames1>
It's definitely a bug ;)
+
+This function resolves the true case of each element in the
+path and returns the case-sensitive path.
+
+Thus C<guestfs_case_sensitive_path> (\"/Windows/System32\")
+might return C<\"/WINDOWS/system32\"> (the exact return value
+would depend on details of how the directories were originally
+created under Windows).
+
+I<Note>:
+This function does not handle drive names, backslashes etc.
+
+See also C<guestfs_realpath>.");
+
]
let all_functions = non_daemon_functions @ daemon_functions
-- 1.6.5.rc2
This API seems like an unfortunate pimple. Would it not be better to
*not* export this API, and instead call this automatically everywhere
the daemon opens a file on ntfs?
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
6:08 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On Mon, Oct 26, 2009 at 11:03:04AM +0000, Matthew Booth wrote:
On 26/10/09 09:20, Richard W.M. Jones wrote:
> +char *
> +do_case_sensitive_path (const char *path)
> +{
> + char ret[PATH_MAX+1] = "/";
Is PATH_MAX on Windows <= POSIX PATH_MAX? Does ntfs_3g honour this?
Seems like a grey area. It would be safer to realloc this buffer as
necessary. You also wouldn't need the strdup() at the end.
PATH_MAX is a Linux thing. NTFS 3g couldn't export something through
the Linux VFS unless it honoured this.
> + size_t next = 1;
> +
> + /* MUST chdir ("/") before leaving this function. */
> + if (chdir (sysroot) == -1) {
> + reply_with_perror ("%s", sysroot);
> + return NULL;
> + }
I'm not convinced chdir is necessary in this function if you use
openat() throughout.
I'm pretty sure I need opendirat to make this work, and that function
doesn't seem to exist (checked on Fedora 11).
> + /* First character is a '/'. Take each subsequent
path element
> + * and follow it.
> + */
A check that *path == '/' wouldn't go amiss here. I'd stick an assert
in, but then DV might shoot me ;)
This is already checked in the generated code.
> + reply_with_error ("case_sensitive_path: path
contained . or .. elements");
This string isn't internationalised. I won't mention all of these.
We don't internationalize any string in the daemon (yet).
> + ("case_sensitive_path", (RString "rpath",
[Pathname "path"]), 197, [],
> + [InitISOFS, Always, TestOutput (
> + [["case_sensitive_path"; "/DIRECTORY"]],
"/directory");
> + InitISOFS, Always, TestOutput (
> + [["case_sensitive_path"; "/Known-1"]],
"/known-1");
> + InitBasicFS, Always, TestOutput (
> + [["mkdir"; "/a"];
> + ["mkdir"; "/a/bbb"];
> + ["touch"; "/a/bbb/c"];
> + ["case_sensitive_path"; "/A/bbB/C"]],
"/a/bbb/c")],
These don't exercise the following cases you've coded for:
/
/foo//bar
/foo/../bar
/foo///bar/
Agreed that more tests are needed here. I will add them in the next
version.
This API seems like an unfortunate pimple. Would it not be better to
*not* export this API, and instead call this automatically everywhere
the daemon opens a file on ntfs?
But how does it know what the underlying filesystem is up to? I agree
it's a bug in NTFS 3g, but changing all the code to do very expensive
magic doesn't seem like the right way, and you can't detect which
filesystems are broken like this.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
6:18 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On 26/10/09 11:08, Richard W.M. Jones wrote:
>> + size_t next = 1;
>> +
>> + /* MUST chdir ("/") before leaving this function. */
>> + if (chdir (sysroot) == -1) {
>> + reply_with_perror ("%s", sysroot);
>> + return NULL;
>> + }
>
> I'm not convinced chdir is necessary in this function if you use
> openat() throughout.
I'm pretty sure I need opendirat to make this work, and that function
doesn't seem to exist (checked on Fedora 11).
You only need openat() followed by fdopendir().
> This API seems like an unfortunate pimple. Would it not be better
to
> *not* export this API, and instead call this automatically everywhere
> the daemon opens a file on ntfs?
But how does it know what the underlying filesystem is up to? I agree
it's a bug in NTFS 3g, but changing all the code to do very expensive
magic doesn't seem like the right way, and you can't detect which
filesystems are broken like this.
Problem is, if you don't put the magic in you're pushing the problem on
to the library user. The code's going to look pretty much the same every
time, so it might as well be in the library. You could take 2 approaches
here:
1. Always use case-sensitive-path: slow, but should be safe.
2. Detect fs type and call case-sensitive-path as required: probably
tricky to get absolutely right (think: ntfs mounted as an additional
volume under /mnt/windows).
I'd go for 1. It should be safe, and it's not as if the slowness is
likely to be noticeable here.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
7:13 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On Mon, Oct 26, 2009 at 11:18:26AM +0000, Matthew Booth wrote:
Problem is, if you don't put the magic in you're pushing the
problem on
to the library user.
This is not true. It's only used for the case where you are resolving
paths given to you by Windows -- eg. coming from a Windows
configuration file or from the Windows Registry. The case we use it
is precisely this: we read the Windows %systemroot% from NTLDR and
then have to resolve that against the filesystem. If Windows allowed
its root filesystem to be on ext3 it would work the same way.
In any other case, using case_sensitive_path is almost certainly
wrong, in fact not just slow but quite likely to be insecure too. You
don't want a Linux config file that specifies /etc/something to
randomly pick /etc/SOMETHING.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
8:16 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On 26/10/09 12:13, Richard W.M. Jones wrote:
On Mon, Oct 26, 2009 at 11:18:26AM +0000, Matthew Booth wrote:
> Problem is, if you don't put the magic in you're pushing the problem on
> to the library user.
This is not true. It's only used for the case where you are resolving
paths given to you by Windows -- eg. coming from a Windows
configuration file or from the Windows Registry. The case we use it
is precisely this: we read the Windows %systemroot% from NTLDR and
then have to resolve that against the filesystem. If Windows allowed
its root filesystem to be on ext3 it would work the same way.
.. or the guestfish user who types 'cat
win:c:\windows\system32\blah.txt' and doesn't get the result they
intended. Essentially you need to do this in 2 cases: you read a path
from a config file, or you're accessing a file whose location you
already know. In either case, the additional call is just untidiness.
In any other case, using case_sensitive_path is almost certainly
wrong, in fact not just slow but quite likely to be insecure too. You
don't want a Linux config file that specifies /etc/something to
randomly pick /etc/SOMETHING.
It's a very good point. You can fix this, though. When iterating down
the directory structure, you'll open a file to check if it's a
directory. At the same time, to fstatfs() to check what filesystem it's
on. By doing this you can choose directory-by-directory whether or not
to be case-sensitive. This should always be safe.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
8:25 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On Mon, Oct 26, 2009 at 01:16:15PM +0000, Matthew Booth wrote:
.. or the guestfish user who types 'cat
win:c:\windows\system32\blah.txt' and doesn't get the result they
intended. Essentially you need to do this in 2 cases: you read a path
from a config file, or you're accessing a file whose location you
already know. In either case, the additional call is just untidiness.
I don't understand this point.
Who told them to look at "c:\windows\system32\blah.txt"?
It must have come from a Windows config file, a Windows document or a
book, and that path is not case-sensitive.
> In any other case, using case_sensitive_path is almost certainly
> wrong, in fact not just slow but quite likely to be insecure too. You
> don't want a Linux config file that specifies /etc/something to
> randomly pick /etc/SOMETHING.
It's a very good point. You can fix this, though. When iterating down
the directory structure, you'll open a file to check if it's a
directory. At the same time, to fstatfs() to check what filesystem it's
on. By doing this you can choose directory-by-directory whether or not
to be case-sensitive. This should always be safe.
fstatfs doesn't tell you if a filesystem is case sensitive, and in any
case that is irrelevant. Think of the imaginary Windows-on-ext3 case
where you'd still have Windows containing case-insensitive
configuration paths, even if the filesystem is case-sensitive.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
8:54 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On 26/10/09 13:25, Richard W.M. Jones wrote:
On Mon, Oct 26, 2009 at 01:16:15PM +0000, Matthew Booth wrote:
> .. or the guestfish user who types 'cat
> win:c:\windows\system32\blah.txt' and doesn't get the result they
> intended. Essentially you need to do this in 2 cases: you read a path
> from a config file, or you're accessing a file whose location you
> already know. In either case, the additional call is just untidiness.
I don't understand this point.
Who told them to look at "c:\windows\system32\blah.txt"?
It must have come from a Windows config file, a Windows document or a
book, and that path is not case-sensitive.
>> In any other case, using case_sensitive_path is almost certainly
>> wrong, in fact not just slow but quite likely to be insecure too. You
>> don't want a Linux config file that specifies /etc/something to
>> randomly pick /etc/SOMETHING.
>
> It's a very good point. You can fix this, though. When iterating down
> the directory structure, you'll open a file to check if it's a
> directory. At the same time, to fstatfs() to check what filesystem it's
> on. By doing this you can choose directory-by-directory whether or not
> to be case-sensitive. This should always be safe.
fstatfs doesn't tell you if a filesystem is case sensitive, and in any
case that is irrelevant. Think of the imaginary Windows-on-ext3 case
where you'd still have Windows containing case-insensitive
configuration paths, even if the filesystem is case-sensitive.
Bleargh. Let me start again on this one.
So I have 2 real objections here:
1. It's *really* ugly
2. It can be done automatically
So, let's say that some file on Windows has a well-defined location.
Maybe it's some driver in system32 or something. The library user can't
just use this location, they have to manually 'resolve' it first. It's
the same case as getting a path from a config file, only less explicit.
Either way, there's an ugly additional step required.
It's also an entirely guest OS specific api addition. I know we have
others (SELinux, for eg), but that's no excuse to add more without
trying very hard not to.
The alternative here, is to do 2 things:
1. Change the implementation of every existing API call which takes a
Guest path to call the new path resolution function first.
2. Fix up the path resolution function so this is safe to do.
The first shouldn't be too hard because it's all auto-generated. The
second should be hard to do either, because it's a relatively small
change to your existing patch to make it check the filesystem type
first, before deciding whether to be case-sensitive or case-insensitive.
The result is that reading a file out of a windows config file and then
accessing it will Just Work, as will accessing a file with a
well-defined path on Windows. The library user doesn't need to do
anything extra, and there is no addition to the API.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
9:23 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
On Mon, Oct 26, 2009 at 01:54:23PM +0000, Matthew Booth wrote:
The alternative here, is to do 2 things:
1. Change the implementation of every existing API call which takes a
Guest path to call the new path resolution function first.
2. Fix up the path resolution function so this is safe to do.
The first shouldn't be too hard because it's all auto-generated. The
second should[n't] be hard to do either, because it's a relatively small
change to your existing patch to make it check the filesystem type
first, before deciding whether to be case-sensitive or case-insensitive.
The result is that reading a file out of a windows config file and then
accessing it will Just Work, as will accessing a file with a
well-defined path on Windows. The library user doesn't need to do
anything extra, and there is no addition to the API.
OK I now understand what you mean. I think that such a large change
to the API is very risky, and I think a lot harder than you imagine
(not all the path parsing code is autogenerated, unfortunately[1], and
also it's not clear what filesystems are case-{in,}sensitive).
And in order to support something which is only used in one very
narrow case: getting a string from a Windows configuration file.
That's why it's better to confine this change to a single API function
that people who do that can use, and everyone else can ignore.
Rich.
[1] Look at recursive_mkdir for example, or mkdtemp, or the functions
that can take either a path or a device.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
8:40 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
Richard W.M. Jones wrote:
On Mon, Oct 26, 2009 at 11:03:04AM +0000, Matthew Booth wrote:
> On 26/10/09 09:20, Richard W.M. Jones wrote:
>> +char *
>> +do_case_sensitive_path (const char *path)
>> +{
>> + char ret[PATH_MAX+1] = "/";
>
> Is PATH_MAX on Windows <= POSIX PATH_MAX? Does ntfs_3g honour this?
> Seems like a grey area. It would be safer to realloc this buffer as
> necessary. You also wouldn't need the strdup() at the end.
PATH_MAX is a Linux thing. NTFS 3g couldn't export something through
the Linux VFS unless it honoured this.
>> + size_t next = 1;
>> +
>> + /* MUST chdir ("/") before leaving this function. */
>> + if (chdir (sysroot) == -1) {
>> + reply_with_perror ("%s", sysroot);
>> + return NULL;
>> + }
>
> I'm not convinced chdir is necessary in this function if you use
> openat() throughout.
I'm pretty sure I need opendirat to make this work, and that function
doesn't seem to exist (checked on Fedora 11).
It's in lib/fts.c:
/* file-descriptor-relative opendir. */
/* FIXME: if others need this function, move it into lib/openat.c */
static inline DIR *
internal_function
opendirat (int fd, char const *dir)
{
int new_fd = openat (fd, dir,
O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
DIR *dirp;
if (new_fd < 0)
return NULL;
set_cloexec_flag (new_fd, true);
dirp = fdopendir (new_fd);
if (dirp == NULL)
{
int saved_errno = errno;
close (new_fd);
errno = saved_errno;
}
return dirp;
}
8:38 a.m.
New subject: [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs
Matthew Booth wrote:
On 26/10/09 09:20, Richard W.M. Jones wrote:
> +char *
> +do_case_sensitive_path (const char *path)
> +{
> + char ret[PATH_MAX+1] = "/";
Is PATH_MAX on Windows <= POSIX PATH_MAX? Does ntfs_3g honour this?
Seems like a grey area. It would be safer to realloc this buffer as
necessary. You also wouldn't need the strdup() at the end.
Good point.
There's another reason to avoid using PATH_MAX:
some systems don't define it at all,
and others define it to an outrageously (for stack use) large value.
4:20 a.m.
New subject: [PATCH 2/3] guestfish: Add win: prefix to use Windows paths.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
5:16 a.m.
New subject: [PATCH 2/3] guestfish: Add win: prefix to use Windows paths.
Richard W.M. Jones wrote:
Subject: [PATCH 2/3] guestfish: Add win: prefix to use Windows
paths.
Add a win: prefix for path arguments in guestfish:
A prefix of "lose:" would be more apt ;-)
Did you consider simply letting a "drive_letter:" prefix
serve this purpose?
Given the "win:" choice, the patch looks fine.
5:55 a.m.
New subject: [PATCH 2/3] guestfish: Add win: prefix to use Windows paths.
On Mon, Oct 26, 2009 at 11:16:57AM +0100, Jim Meyering wrote:
Did you consider simply letting a "drive_letter:" prefix
serve this purpose?
There's a subtle point here which means we need a real prefix.
Ordinary paths should all be prefixed by "/".
It seems we could say that paths prefixed by "\" or "C:" would be
treated as Windows paths, although I don't think I like the
implicitness going on. But in any case this doesn't work. We might
have some mixed path like: "/windows\system". "win:/windows\system"
works (and "/windows\system" also works under Windows).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
4:21 a.m.
New subject: [PATCH 3/3] Deprecate Sys::Guestfs::Lib::resolve_windows_path.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
4:28 a.m.
New subject: [PATCH 3/3 version 2] Modify Sys::Guestfs::Lib::resolve_windows_path to use case_sensitive_path
Umm ... they're not quite the same because the error case is
different. Try this updated patch.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
5:06 a.m.
New subject: [PATCH 3/3] Deprecate Sys::Guestfs::Lib::resolve_windows_path.
Richard W.M. Jones wrote:
...
diff --git a/perl/lib/Sys/Guestfs/Lib.pm
b/perl/lib/Sys/Guestfs/Lib.pm
index 8ea2c1b..f7aaf07 100644
--- a/perl/lib/Sys/Guestfs/Lib.pm
+++ b/perl/lib/Sys/Guestfs/Lib.pm
@@ -247,6 +247,9 @@ sub _is_pv {
=head2 resolve_windows_path
+I<This function is deprecated>.
+Use C<$g-E<gt>case_sensitive_path> as a direct substitute.
+
$path = resolve_windows_path ($g, $path);
$path = resolve_windows_path ($g, "/windows/system");
@@ -267,38 +270,10 @@ by C</> characters. Do not use C<\>, drive names, etc.
Hi Rich,
This looks fine.
sub resolve_windows_path
{
- local $_;
my $g = shift;
my $path = shift;
If you're interested in stylistic suggestions, you might
prefer the one-line style to repeated uses of "shift".
Saving N-1 lines per function is a Good Thing:
my ($g, $path) = @_;
Also, a general guideline I've found useful is to say "file" or
"filename"
in place of "path" (as long as we're not talking about shell search paths,
a la $PATH) with the implication that "filename" is general enough
to encompass names of things like directories and non-regular files.
POSIX does this, and it makes it a lot easier to write about
"file system objects" as "files" without having to say "regular
files or
directories or special devices", etc.
i.e.,
my ($g, $filename) = @_;
- if (substr ($path, 0, 1) ne "/") {
- warn __"resolve_windows_path: path must start with a / character";
- return undef;
- }
...
5510
days inactive
5510
days old
21 comments
3 participants
participants (3)
-
Jim Meyering -
Matthew Booth -
Richard W.M. Jones