New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qvm-copy-to-vm: improve error handling #743

Closed
marmarek opened this Issue Mar 8, 2015 · 7 comments

Comments

Projects
None yet
1 participant
@marmarek
Member

marmarek commented Mar 8, 2015

Reported by joanna on 9 Aug 2013 11:53 UTC

  1. When the VM name given is wrong, then no error message is displayed to the user, and the copy operation just dies with a mysterious EOT being printed and nothing more that could have hinted the user that the VM name was incorrect.

  2. When copying some directories (notably the fresh qubes-builder dir) the agent displayed the following error:

qfile-agent: Fatal error: File copy: File name too long (error type: Connection reset by peer)

First, this should probably not be treated as an error. Second, if we must abort, at least print the name of the file that was found to be too long.

Migrated-From: https://wiki.qubes-os.org/ticket/743

@marmarek marmarek added this to the Release 2 Beta 3 milestone Mar 8, 2015

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by joanna on 9 Aug 2013 11:55 UTC
Apparently any symlink causes the qvm-copy-to-vm to abort with the above mentioned File Too Long error.

Member

marmarek commented Mar 8, 2015

Comment by joanna on 9 Aug 2013 11:55 UTC
Apparently any symlink causes the qvm-copy-to-vm to abort with the above mentioned File Too Long error.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by joanna on 9 Aug 2013 11:55 UTC
Apparently any symlink causes the qvm-copy-to-vm to abort with the above mentioned File Too Long error.

Member

marmarek commented Mar 8, 2015

Comment by joanna on 9 Aug 2013 11:55 UTC
Apparently any symlink causes the qvm-copy-to-vm to abort with the above mentioned File Too Long error.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by joanna on 9 Aug 2013 12:05 UTC
Ah, this symlink problem only manifests itself when copying from Linux AppVM to Win AppVM.

Member

marmarek commented Mar 8, 2015

Comment by joanna on 9 Aug 2013 12:05 UTC
Ah, this symlink problem only manifests itself when copying from Linux AppVM to Win AppVM.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by marmarek on 12 Aug 2013 23:18 UTC
The first one is easy - dom0 (qrexec-policy) can show message to the user.

The second one requires some change in file copy protocol (namely ''struct result_header'') to add the name of failed file. But this is easily done in backward and forward compatible way, so not really problem.

Regarding symlink problem itself, IMHO it's better to fail the transfer (with some more meaningful error...) rather than leave the user with missing files and pretend that everything is ok. There is no simple way to report non-fatal errors (only one transfer-global status code)...

Member

marmarek commented Mar 8, 2015

Comment by marmarek on 12 Aug 2013 23:18 UTC
The first one is easy - dom0 (qrexec-policy) can show message to the user.

The second one requires some change in file copy protocol (namely ''struct result_header'') to add the name of failed file. But this is easily done in backward and forward compatible way, so not really problem.

Regarding symlink problem itself, IMHO it's better to fail the transfer (with some more meaningful error...) rather than leave the user with missing files and pretend that everything is ok. There is no simple way to report non-fatal errors (only one transfer-global status code)...

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by joanna on 13 Aug 2013 08:27 UTC
Perhaps we could introduce a switch, such as "--force" that, if cannot recreate symlinks, will just make regular files in their place (the actual copier). Or perhaps:
--ignore-symlinks -- just skip symlinks, but continue the copy operation for other files
--force-symlinks -- make regular files in place of symlinks if the dest VM/fs doesn't allow for them.

Hm, except that Windows NTFS actually does allow symlinks...

Member

marmarek commented Mar 8, 2015

Comment by joanna on 13 Aug 2013 08:27 UTC
Perhaps we could introduce a switch, such as "--force" that, if cannot recreate symlinks, will just make regular files in their place (the actual copier). Or perhaps:
--ignore-symlinks -- just skip symlinks, but continue the copy operation for other files
--force-symlinks -- make regular files in place of symlinks if the dest VM/fs doesn't allow for them.

Hm, except that Windows NTFS actually does allow symlinks...

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by marmarek on 13 Aug 2013 10:07 UTC
Replying to joanna:

Perhaps we could introduce a switch, such as "--force" that, if cannot recreate symlinks, will just make regular files in their place (the actual copier). Or perhaps:
--ignore-symlinks -- just skip symlinks, but continue the copy operation for other files
--force-symlinks -- make regular files in place of symlinks if the dest VM/fs doesn't allow for them.

First of all source VM doesn't know remote filesystem. And we don't want to let it know (this will make filecopy protocol much more complex).

Making regular files in place of symlinks isn't straightforward for directories... But --ignore-symlinks is doable.

Hm, except that Windows NTFS actually does allow symlinks...

Indeed NTFS support symlinks (starting from Vista). The only catch is that it has two types of them: one for files and one for directories. If the symlink comes before link target it will be hard to guess what type it should be. Maybe default to directory?

In any case handling symlinks in windows file unpacker isn't implemented at all - not even as ignoring them, so symlink content is parsed as the next file header... So this is regular bug.

Member

marmarek commented Mar 8, 2015

Comment by marmarek on 13 Aug 2013 10:07 UTC
Replying to joanna:

Perhaps we could introduce a switch, such as "--force" that, if cannot recreate symlinks, will just make regular files in their place (the actual copier). Or perhaps:
--ignore-symlinks -- just skip symlinks, but continue the copy operation for other files
--force-symlinks -- make regular files in place of symlinks if the dest VM/fs doesn't allow for them.

First of all source VM doesn't know remote filesystem. And we don't want to let it know (this will make filecopy protocol much more complex).

Making regular files in place of symlinks isn't straightforward for directories... But --ignore-symlinks is doable.

Hm, except that Windows NTFS actually does allow symlinks...

Indeed NTFS support symlinks (starting from Vista). The only catch is that it has two types of them: one for files and one for directories. If the symlink comes before link target it will be hard to guess what type it should be. Maybe default to directory?

In any case handling symlinks in windows file unpacker isn't implemented at all - not even as ignoring them, so symlink content is parsed as the next file header... So this is regular bug.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by marmarek on 14 Aug 2013 23:44 UTC
One additional problem with windows symlinks: CreateSymbolicLink function requires the SE_CREATE_SYMBOLIC_LINK_NAME (defined as "SeCreateSymbolicLinkPrivilege" in <WinNT.h>), otherwise the function will fail and GetLastError will return ERROR_PRIVILEGE_NOT_HELD (1314). This means the process must be run in an elevated state (UAC).

So effectively creating symbolic links isn't working on windows for now (although it is implemented now - so no strange, random error anymore).
Besides that this ticket is solved:

  1. added option --ignore-symlinks to qvm-copy-to-vm
  2. added reporting of invalid destination VM name
  3. added reporting name of failed file transfer (both Windows and Linux)
Member

marmarek commented Mar 8, 2015

Comment by marmarek on 14 Aug 2013 23:44 UTC
One additional problem with windows symlinks: CreateSymbolicLink function requires the SE_CREATE_SYMBOLIC_LINK_NAME (defined as "SeCreateSymbolicLinkPrivilege" in <WinNT.h>), otherwise the function will fail and GetLastError will return ERROR_PRIVILEGE_NOT_HELD (1314). This means the process must be run in an elevated state (UAC).

So effectively creating symbolic links isn't working on windows for now (although it is implemented now - so no strange, random error anymore).
Besides that this ticket is solved:

  1. added option --ignore-symlinks to qvm-copy-to-vm
  2. added reporting of invalid destination VM name
  3. added reporting name of failed file transfer (both Windows and Linux)

@marmarek marmarek closed this Mar 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment