Page 1 of 1

[Fixed] VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Posted: 30. Mar 2018, 19:43
by jchatham
The first bug is on line 341 of VBox/Main/src-client/GuestSessionImplTasks.cpp - we need to treat both VERR_NOT_A_DIRECTORY and VERR_FILE_NOT_FOUND as non-error states (rather than just VERR_NOT_A_DIRECTORY), otherwise attempting to copy a file from host to guest will always fail if the destination file doesn't already exist. (Or at least, that's what happens when talking to a win7 guest from an ubuntu 16.04 host. I wouldn't expect this issue to be platform-dependent, but I haven't checked other platforms.)
This bug is an issue for non-recursive copy commands as well as recursive ones.

All of the rest of the problems with recursive copy are in VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp and are fixed by the following patch:

Code: Select all

### Eclipse Workspace Patch 1.0
#P VirtualBox-5.2.8
Index: src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp
===================================================================
--- src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp	(revision 112533)
+++ src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp	(working copy)
@@ -2007,6 +2007,9 @@
         HRESULT rc = pContext->pCmdCtx->pGuestSession->DirectoryExists(Bstr(pszDir).raw(), FALSE /*followSymlinks*/, &fDirExists);
         if (SUCCEEDED(rc))
             *fExists = fDirExists != FALSE;
+        /* The DirectoryExists command always returns this error code if the directory doesn't exist; need to treat it as a non-error. */
+        else if(rc == VBOX_E_IPRT_ERROR)
+            *fExists = FALSE;
         else
             vrc = gctlPrintError(pContext->pCmdCtx->pGuestSession, COM_IIDOF(IGuestSession));
     }
@@ -2214,9 +2217,17 @@
     if (pContext->pCmdCtx->cVerbose)
         RTPrintf("Processing host directory: %s\n", szCurDir);
 
-    /* Flag indicating whether the current directory was created on the
-     * target or not. */
-    bool fDirCreated = false;
+    /*
+     * Create the target directory.  Trying to do this "lazily" inside
+     * the loop fails to copy empty directories.
+     */
+    char *pszDestDir;
+    vrc = gctlCopyTranslatePath(pszSource, szCurDir, pszDest, &pszDestDir);
+    if (RT_SUCCESS(vrc))
+    {
+        vrc = gctlCopyDirCreate(pContext, pszDestDir);
+        RTStrFree(pszDestDir);
+    }
 
     /*
      * Open directory without a filter - RTDirOpenFiltered unfortunately
@@ -2235,7 +2246,8 @@
             PRTDIRENTRYEX pDirEntry  = NULL;
             while (RT_SUCCESS(vrc))
             {
-                vrc = RTDirReadExA(hDir, &pDirEntry, &cbDirEntry, RTFSOBJATTRADD_NOTHING, 0);
+                /* Calling this with zero for its flags returns an error. */
+                vrc = RTDirReadExA(hDir, &pDirEntry, &cbDirEntry, RTFSOBJATTRADD_NOTHING, ((enmFlags & kGctlCopyFlags_FollowLinks)? RTPATH_F_FOLLOW_LINK : RTPATH_F_ON_LINK) );
                 if (RT_FAILURE(vrc))
                 {
                     if (vrc == VERR_NO_MORE_FILES)
@@ -2297,19 +2309,6 @@
                         if (pContext->pCmdCtx->cVerbose)
                             RTPrintf("File: %s\n", pDirEntry->szName);
 
-                        if (!fDirCreated)
-                        {
-                            char *pszDestDir;
-                            vrc = gctlCopyTranslatePath(pszSource, szCurDir, pszDest, &pszDestDir);
-                            if (RT_SUCCESS(vrc))
-                            {
-                                vrc = gctlCopyDirCreate(pContext, pszDestDir);
-                                RTStrFree(pszDestDir);
-
-                                fDirCreated = true;
-                            }
-                        }
-
                         if (RT_SUCCESS(vrc))
                         {
                             char *pszFileSource = RTPathJoinA(szCurDir, pDirEntry->szName);
@@ -2380,9 +2379,19 @@
     if (pContext->pCmdCtx->cVerbose)
         RTPrintf("Processing guest directory: %s\n", szCurDir);
 
-    /* Flag indicating whether the current directory was created on the
-     * target or not. */
-    bool fDirCreated = false;
+    /*
+     * Create the target directory.  Trying to do this "lazily" inside
+     * the loop fails to copy empty directories.
+     */
+    char *pszDestDir;
+    vrc = gctlCopyTranslatePath(pszSource, szCurDir,
+                                pszDest, &pszDestDir);
+    if (RT_SUCCESS(vrc))
+    {
+        vrc = gctlCopyDirCreate(pContext, pszDestDir);
+        RTStrFree(pszDestDir);
+    }
+
     SafeArray<DirectoryOpenFlag_T> dirOpenFlags; /* No flags supported yet. */
     ComPtr<IGuestDirectory> pDirectory;
     HRESULT rc = pContext->pCmdCtx->pGuestSession->DirectoryOpen(Bstr(szCurDir).raw(), Bstr(pszFilter).raw(),
@@ -2467,20 +2476,6 @@
                 if (pContext->pCmdCtx->cVerbose)
                     RTPrintf("File: %s\n", strFile.c_str());
 
-                if (!fDirCreated)
-                {
-                    char *pszDestDir;
-                    vrc = gctlCopyTranslatePath(pszSource, szCurDir,
-                                                pszDest, &pszDestDir);
-                    if (RT_SUCCESS(vrc))
-                    {
-                        vrc = gctlCopyDirCreate(pContext, pszDestDir);
-                        RTStrFree(pszDestDir);
-
-                        fDirCreated = true;
-                    }
-                }
-
                 if (RT_SUCCESS(vrc))
                 {
                     char *pszFileSource = RTPathJoinA(szCurDir, strFile.c_str());
@@ -2518,6 +2513,7 @@
         switch (rc)
         {
             case E_ABORT: /* No more directory entries left to process. */
+            case VBOX_E_OBJECT_NOT_FOUND: /* Also indicates no more entries to process. */
                 break;
 
             case VBOX_E_FILE_ERROR: /* Current entry cannot be accessed to
As a last note: according to the documentation here, I need to specify that my contribution is submitted under the MIT license. As such, please consider the above to be submitted under that license. (Or, if those instructions are as out of date as the linux build instructions, please let me know what I actually need to do to make it so these fixes can be legally patched into virtualbox trunk code.)

Re: VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Posted: 17. Apr 2018, 19:07
by jchatham
Addendum to the above: in guestSessionImplTasks.cpp, it's guestRc that comes in as VERR_FILE_NOT_FOUND, not rc.

Re: VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Posted: 2. May 2018, 14:50
by pentagonik
Thanks for the patch! However, I've fixed the API method to returning FALSE instead of returning an error. The fix will be available in the next upcoming maintenance release.

Re: VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Posted: 2. May 2018, 15:41
by socratis
Marking as [Fixed] then...