Index: /trunk/src/VBox/Main/MediumImpl.cpp =================================================================== --- /trunk/src/VBox/Main/MediumImpl.cpp (revision 24724) +++ /trunk/src/VBox/Main/MediumImpl.cpp (revision 24725) @@ -103,5 +103,4 @@ implicit(false), numCreateDiffTasks(0), - vdProgress(NULL), vdDiskIfaces(NULL) {} @@ -150,11 +149,7 @@ Utf8Str vdError; /*< Error remembered by the VD error callback. */ - Progress *vdProgress; /*< Progress for the VD progress callback. */ VDINTERFACE vdIfError; VDINTERFACEERROR vdIfCallsError; - - VDINTERFACE vdIfProgress; - VDINTERFACEPROGRESS vdIfCallsProgress; VDINTERFACE vdIfConfig; @@ -798,9 +793,4 @@ m->vdIfCallsError.pfnMessage = NULL; - /* Initialize the callbacks of the VD progress interface */ - m->vdIfCallsProgress.cbSize = sizeof(VDINTERFACEPROGRESS); - m->vdIfCallsProgress.enmInterface = VDINTERFACETYPE_PROGRESS; - m->vdIfCallsProgress.pfnProgress = vdProgressCall; - /* Initialize the callbacks of the VD config interface */ m->vdIfCallsConfig.cbSize = sizeof(VDINTERFACECONFIG); @@ -811,5 +801,5 @@ /* Initialize the callbacks of the VD TCP interface (we always use the host - * * IP stack for now) */ + * IP stack for now) */ m->vdIfCallsTcpNet.cbSize = sizeof(VDINTERFACETCPNET); m->vdIfCallsTcpNet.enmInterface = VDINTERFACETYPE_TCPNET; @@ -829,9 +819,4 @@ AssertRCReturn(vrc, E_FAIL); - vrc = VDInterfaceAdd(&m->vdIfProgress, - "Medium::vdInterfaceProgress", - VDINTERFACETYPE_PROGRESS, - &m->vdIfCallsProgress, this, &m->vdDiskIfaces); - AssertRCReturn(vrc, E_FAIL); vrc = VDInterfaceAdd(&m->vdIfConfig, "Medium::vdInterfaceConfig", @@ -4767,12 +4752,11 @@ void *pvUser) { - Medium *that = static_cast(pvUser); - AssertReturn(that != NULL, VERR_GENERAL_FAILURE); - - if (that->m->vdProgress != NULL) + Progress *that = static_cast(pvUser); + + if (that != NULL) { /* update the progress object, capping it at 99% as the final percent * is used for additional operations like setting the UUIDs and similar. */ - HRESULT rc = that->m->vdProgress->SetCurrentOperationProgress(uPercent * 99 / 100); + HRESULT rc = that->SetCurrentOperationProgress(uPercent * 99 / 100); if (FAILED(rc)) { @@ -4866,4 +4850,18 @@ Medium *that = task->that; + + /* Set up a per-operation progress interface, can be used freely (for + * binary operations you can use it either on the source or target). */ + VDINTERFACEPROGRESS vdIfCallsProgress; + vdIfCallsProgress.cbSize = sizeof(VDINTERFACEPROGRESS); + vdIfCallsProgress.enmInterface = VDINTERFACETYPE_PROGRESS; + vdIfCallsProgress.pfnProgress = Medium::vdProgressCall; + VDINTERFACE vdIfProgress; + PVDINTERFACE vdOperationIfaces = NULL; + int vrc = VDInterfaceAdd(&vdIfProgress, + "Medium::vdInterfaceProgress", + VDINTERFACETYPE_PROGRESS, + &vdIfCallsProgress, task->progress, &vdOperationIfaces); + AssertRCReturn(vrc, E_FAIL); /// @todo ugly hack, fix ComAssert... later @@ -4923,7 +4921,4 @@ PDMMEDIAGEOMETRY geo = { 0 }; /* auto-detect */ - /* needed for vdProgressCallback */ - that->m->vdProgress = task->progress; - vrc = VDCreateBase(hdd, format.c_str(), location.c_str(), task->d.size * _1M, @@ -4931,6 +4926,5 @@ NULL, &geo, &geo, id.raw(), VD_OPEN_FLAGS_NORMAL, - NULL, that->m->vdDiskIfaces); - + NULL, vdOperationIfaces); if (RT_FAILURE(vrc)) { @@ -5042,7 +5036,4 @@ CheckComRCThrowRC(rc); - /* needed for vdProgressCallback */ - that->m->vdProgress = task->progress; - /** @todo add VD_IMAGE_FLAGS_DIFF to the image flags, to * be on the safe side. */ @@ -5054,8 +5045,5 @@ VD_OPEN_FLAGS_NORMAL, target->m->vdDiskIfaces, - that->m->vdDiskIfaces); - - that->m->vdProgress = NULL; - + vdOperationIfaces); if (RT_FAILURE(vrc)) { @@ -5079,5 +5067,5 @@ * here), but lock VirtualBox first to follow the rule */ AutoMultiWriteLock2 alock(that->mVirtualBox->lockHandle(), - that->treeLock()); + that->treeLock()); Assert(target->mParent.isNull()); @@ -5089,4 +5077,6 @@ target->mVirtualBox->removeDependentChild(target); + /** @todo r=klaus neither target nor that->base() are locked, + * potential race! */ /* diffs for immutable hard disks are auto-reset by default */ target->m->autoReset = @@ -5215,7 +5205,4 @@ } - /* needed for vdProgressCallback */ - that->m->vdProgress = task->progress; - unsigned start = chain->isForward() ? 0 : (unsigned)chain->size() - 1; @@ -5225,8 +5212,5 @@ LogFlow(("*** MERGE from %d to %d\n", start, end)); #endif - vrc = VDMerge(hdd, start, end, that->m->vdDiskIfaces); - - that->m->vdProgress = NULL; - + vrc = VDMerge(hdd, start, end, vdOperationIfaces); if (RT_FAILURE(vrc)) throw vrc; @@ -5507,7 +5491,4 @@ } - /* unlock before the potentially lengthy operation */ - thatLock.leave(); - Utf8Str targetFormat(target->m->strFormat); Utf8Str targetLocation(target->m->strLocationFull); @@ -5518,10 +5499,10 @@ Assert(parent.isNull() || parent->m->state == MediumState_LockedRead); + /* unlock before the potentially lengthy operation */ + thatLock.leave(); + /* ensure the target directory exists */ rc = VirtualBox::ensureFilePathExists(targetLocation); CheckComRCThrowRC(rc); - - /* needed for vdProgressCallback */ - that->m->vdProgress = task->progress; PVBOXHDD targetHdd; @@ -5535,4 +5516,6 @@ it != parentChain->end(); ++ it) { + /** @todo r=klaus (*it) is not locked, lots of + * race opportunities below */ /* sanity check */ Assert( (*it)->m->state == MediumState_LockedRead @@ -5553,4 +5536,5 @@ } + /** @todo r=klaus target isn't locked, race getting the state */ vrc = VDCopy(hdd, VD_LAST_IMAGE, targetHdd, targetFormat.c_str(), @@ -5559,8 +5543,5 @@ task->d.variant, targetId.raw(), NULL, target->m->vdDiskIfaces, - that->m->vdDiskIfaces); - - that->m->vdProgress = NULL; - + vdOperationIfaces); if (RT_FAILURE(vrc)) { @@ -5770,7 +5751,4 @@ } - /* needed for vdProgressCallback */ - that->m->vdProgress = task->progress; - vrc = VDCreateDiff(hdd, format.c_str(), location.c_str(), /// @todo use the same image variant as before @@ -5780,8 +5758,5 @@ VD_OPEN_FLAGS_NORMAL, that->m->vdDiskIfaces, - that->m->vdDiskIfaces); - - that->m->vdProgress = NULL; - + vdOperationIfaces); if (RT_FAILURE(vrc)) { @@ -5863,16 +5838,12 @@ } + Assert(that->m->state == MediumState_LockedWrite); + + Utf8Str location(that->m->strLocationFull); + /* unlock before the potentially lengthy operation */ thatLock.leave(); - Assert(that->m->state == MediumState_LockedWrite); - - /* needed for vdProgressCallback */ - that->m->vdProgress = task->progress; - - vrc = VDCompact(hdd, VD_LAST_IMAGE, that->m->vdDiskIfaces); - - that->m->vdProgress = NULL; - + vrc = VDCompact(hdd, VD_LAST_IMAGE, vdOperationIfaces); if (RT_FAILURE(vrc)) { @@ -5880,13 +5851,13 @@ throw setError(VBOX_E_NOT_SUPPORTED, tr("Compacting is not yet supported for hard disk '%s'"), - that->m->strLocationFull.raw()); + location.raw()); else if (vrc == VERR_NOT_IMPLEMENTED) throw setError(E_NOTIMPL, tr("Compacting is not implemented, hard disk '%s'"), - that->m->strLocationFull.raw()); + location.raw()); else throw setError(E_FAIL, tr("Could not compact hard disk '%s'%s"), - that->m->strLocationFull.raw(), + location.raw(), that->vdError(vrc).raw()); }