On Sun, Feb 21, 2016 at 11:22:23PM +0200, Matteo Cafasso wrote: > Adding ntfscat_i command for downloading files based on their inode number. > > This allows the dowload of files unaccessible otherwise from a NTFS guest disk image. The patch seems fine, but it really needs a test otherwise this feature could silently break.

Have a look at the tests/ntfsclone/ subdirectory for the general idea.

One problem with writing the test (indeed, with the general idea) is how do you discover which inode numbers can be downloaded? Does NTFS have some standard inode numbers for things like the MFT?

Rich. > --- > daemon/ntfs.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > generator/actions.ml | 15 +++++++++++++ > 2 files changed, 77 insertions(+) > > diff --git a/daemon/ntfs.c b/daemon/ntfs.c > index 568899e..58f62fa 100644 > --- a/daemon/ntfs.c > +++ b/daemon/ntfs.c > @@ -266,3 +266,65 @@ do_ntfsfix (const char *device, int clearbadsectors) > > return 0; > } > + > +int > +do_ntfscat_i (const mountable_t *mountable, int64_t inode) > +{ > + int r; > + FILE *fp; > + CLEANUP_FREE char *cmd = NULL; > + char buffer[GUESTFS_MAX_CHUNK_SIZE]; > + > + /* Inode must be greater than 0 */ > + if (inode < 0) { > + reply_with_error("Inode must be greater than 0"); > + return -1; > + } > + > + /* Construct the command. */ > + if (asprintf_nowarn (&cmd, "ntfscat -i %ld %s", > + inode, mountable->device) == -1) { > + reply_with_perror ("asprintf"); > + return -1; > + } > + > + if (verbose) > + fprintf (stderr, "%s\n", cmd); > + > + fp = popen (cmd, "r"); > + if (fp == NULL) { > + reply_with_perror ("%s", cmd); > + return -1; > + } > + > + /* Now we must send the reply message, before the file contents. After > + * this there is no opportunity in the protocol to send any error > + * message back. Instead we can only cancel the transfer. > + */ > + reply (NULL, NULL); > + > + while ((r = fread (buffer, 1, sizeof buffer, fp)) > 0) { > + if (send_file_write (buffer, r) < 0) { > + pclose (fp); > + return -1; > + } > + } > + > + if (ferror (fp)) { > + fprintf (stderr, "fread: %ld: %m\n", inode); > + send_file_end (1); /* Cancel. */ > + pclose (fp); > + return -1; > + } > + > + if (pclose (fp) != 0) { > + fprintf (stderr, "pclose: %ld: %m\n", inode); > + send_file_end (1); /* Cancel. */ > + return -1; > + } > + > + if (send_file_end (0)) /* Normal end of file. */ > + return -1; > + > + return 0; > +} > diff --git a/generator/actions.ml b/generator/actions.ml > index eb45392..18418aa 100644 > --- a/generator/actions.ml > +++ b/generator/actions.ml > @@ -12891,6 +12891,21 @@ This is equivalent to C<sgdisk -e>. > > See also L<sgdisk(8)>." }; > > + { defaults with > + name = "ntfscat_i"; added = (1, 33, 12); > + style = RErr, [Mountable "device"; Int64 "inode"; FileOut "filename"], []; > + proc_nr = Some 463; > + progress = true; cancellable = true; > + shortdesc = "download a file to the local machine given its inode"; > + longdesc = "\ > +Download a file given its inode from a NTFS filesystem and save it as F<filename> > +on the local machine. > + > +This allows to download some otherwise unaccessible files such as the ones > +within the $Extend folder. > + > +F<filename> can also be a named pipe." }; > + > ] > > (* Non-API meta-commands available only in guestfish. > -- > 2.7.0

On Mon, Feb 22, 2016 at 07:23:45PM +0200, noxdafox wrote: > On 22/02/16 17:26, Richard W.M. Jones wrote: >> On Sun, Feb 21, 2016 at 11:22:23PM +0200, Matteo Cafasso wrote: >>> Adding ntfscat_i command for downloading files based on their inode number. >>> >>> This allows the dowload of files unaccessible otherwise from a NTFS guest disk image. >> The patch seems fine, but it really needs a test otherwise this >> feature could silently break. > I was thinking the same but I stumbled over an issue which prevented > me from running the tests. > I'm not sure this is the right place where to discuss the problem > but here's few lines about it. > > make blank-disk.img blank-part.img blank-fs.img blank-bootroot.img > blank-bootrootlv.img debian.img fedora.img fedora-md1.img > fedora-md2.img fedora-btrfs.img ubuntu.img archlinux.img coreos.img > windows.img guests-all-good.xml > make[3]: Entering directory > '/home/noxdafox/development/libguestfs/test-data/phony-guests' > make[3]: 'blank-disk.img' is up to date. > make[3]: 'blank-part.img' is up to date. > make[3]: 'blank-fs.img' is up to date. > make[3]: 'blank-bootroot.img' is up to date. > make[3]: 'blank-bootrootlv.img' is up to date. > make[3]: 'debian.img' is up to date. > rm -f fedora-name.db fedora-name.db-t > no fedora-name.db-t < fedora-name.db.txt > /bin/bash: no: command not found What happens here is it's trying to run ``$(DB_LOAD) fedora-name.db-t'' but the db_load program wasn't found by your configure. I think you need to install whatever is the db4-utils (Berkeley DB 4) on your OS. Alternately you can grab the pre-built databases from a recent tarball, since we distribute this stuff in the tarball so it doesn't need to be rebuilt for most end users. Rich.

> Makefile:1793: recipe for target 'fedora-name.db' failed > make[3]: *** [fedora-name.db] Error 127 > > Searching around it seems some dependency is missing but ./configure > is not helping me to find which one. > >> Have a look at the tests/ntfsclone/ subdirectory for the general idea. > This was also another issue, I was not sure ntfs was supported by > the test suite and I didn't find any documentation about it. I'll > take a look at these ones. >> One problem with writing the test (indeed, with the general idea) is >> how do you discover which inode numbers can be downloaded? Does NTFS >> have some standard inode numbers for things like the MFT? > The $MFT file has alway 0 as index number. >> Rich. >> >>> --- >>> daemon/ntfs.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> generator/actions.ml | 15 +++++++++++++ >>> 2 files changed, 77 insertions(+) >>> >>> diff --git a/daemon/ntfs.c b/daemon/ntfs.c >>> index 568899e..58f62fa 100644 >>> --- a/daemon/ntfs.c >>> +++ b/daemon/ntfs.c >>> @@ -266,3 +266,65 @@ do_ntfsfix (const char *device, int clearbadsectors) >>> >>> return 0; >>> } >>> + >>> +int >>> +do_ntfscat_i (const mountable_t *mountable, int64_t inode) >>> +{ >>> + int r; >>> + FILE *fp; >>> + CLEANUP_FREE char *cmd = NULL; >>> + char buffer[GUESTFS_MAX_CHUNK_SIZE]; >>> + >>> + /* Inode must be greater than 0 */ >>> + if (inode < 0) { >>> + reply_with_error("Inode must be greater than 0"); >>> + return -1; >>> + } >>> + >>> + /* Construct the command. */ >>> + if (asprintf_nowarn (&cmd, "ntfscat -i %ld %s", >>> + inode, mountable->device) == -1) { >>> + reply_with_perror ("asprintf"); >>> + return -1; >>> + } >>> + >>> + if (verbose) >>> + fprintf (stderr, "%s\n", cmd); >>> + >>> + fp = popen (cmd, "r"); >>> + if (fp == NULL) { >>> + reply_with_perror ("%s", cmd); >>> + return -1; >>> + } >>> + >>> + /* Now we must send the reply message, before the file contents. After >>> + * this there is no opportunity in the protocol to send any error >>> + * message back. Instead we can only cancel the transfer. >>> + */ >>> + reply (NULL, NULL); >>> + >>> + while ((r = fread (buffer, 1, sizeof buffer, fp)) > 0) { >>> + if (send_file_write (buffer, r) < 0) { >>> + pclose (fp); >>> + return -1; >>> + } >>> + } >>> + >>> + if (ferror (fp)) { >>> + fprintf (stderr, "fread: %ld: %m\n", inode); >>> + send_file_end (1); /* Cancel. */ >>> + pclose (fp); >>> + return -1; >>> + } >>> + >>> + if (pclose (fp) != 0) { >>> + fprintf (stderr, "pclose: %ld: %m\n", inode); >>> + send_file_end (1); /* Cancel. */ >>> + return -1; >>> + } >>> + >>> + if (send_file_end (0)) /* Normal end of file. */ >>> + return -1; >>> + >>> + return 0; >>> +} >>> diff --git a/generator/actions.ml b/generator/actions.ml >>> index eb45392..18418aa 100644 >>> --- a/generator/actions.ml >>> +++ b/generator/actions.ml >>> @@ -12891,6 +12891,21 @@ This is equivalent to C<sgdisk -e>. >>> >>> See also L<sgdisk(8)>." }; >>> >>> + { defaults with >>> + name = "ntfscat_i"; added = (1, 33, 12); >>> + style = RErr, [Mountable "device"; Int64 "inode"; FileOut "filename"], []; >>> + proc_nr = Some 463; >>> + progress = true; cancellable = true; >>> + shortdesc = "download a file to the local machine given its inode"; >>> + longdesc = "\ >>> +Download a file given its inode from a NTFS filesystem and save it as F<filename> >>> +on the local machine. >>> + >>> +This allows to download some otherwise unaccessible files such as the ones >>> +within the $Extend folder. >>> + >>> +F<filename> can also be a named pipe." }; >>> + >>> ] >>> >>> (* Non-API meta-commands available only in guestfish. >>> -- >>> 2.7.0