[Fixed] VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Discussions related to using the OSE version of VirtualBox.
Post Reply
jchatham
Posts: 19
Joined: 15. Jul 2013, 20:57

[Fixed] VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Post 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.)
Last edited by socratis on 2. May 2018, 15:41, edited 1 time in total.
Reason: Marked as [Fixed].
jchatham
Posts: 19
Joined: 15. Jul 2013, 20:57

Re: VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Post by jchatham »

Addendum to the above: in guestSessionImplTasks.cpp, it's guestRc that comes in as VERR_FILE_NOT_FOUND, not rc.
pentagonik
Oracle Corporation
Posts: 283
Joined: 19. May 2008, 16:09
Primary OS: Ubuntu other
VBox Version: PUEL
Guest OSses: Too many to specify!
Contact:

Re: VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Post 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.
socratis
Site Moderator
Posts: 27330
Joined: 22. Oct 2010, 11:03
Primary OS: Mac OS X other
VBox Version: PUEL
Guest OSses: Win(*>98), Linux*, OSX>10.5
Location: Greece

Re: VirtualBox 5.2.8 - fixing vboxmanage recursive copy

Post by socratis »

Marking as [Fixed] then...
Do NOT send me Personal Messages (PMs) for troubleshooting, they are simply deleted.
Do NOT reply with the "QUOTE" button, please use the "POST REPLY", at the bottom of the form.
If you obfuscate any information requested, I will obfuscate my response. These are virtual UUIDs, not real ones.
Post Reply