Re: [libvirt] [PATCHv4 08/18] blockjob: react to active block copy

On 04/13/2012 05:35 AM, Jiri Denemark wrote: > On Mon, Apr 09, 2012 at 21:52:17 -0600, Eric Blake wrote: >> For now, disk migration via block copy job is not implemented. But >> when we do implement it, we have to deal with the fact that qemu does >> not provide an easy way to re-start a qemu process with mirroring >> still intact (it _might_ be possible by using qemu -S then an >> initial 'drive-mirror' with disk reuse before starting the domain, >> but that gets hairy). Even something like 'virDomainSave' becomes >> hairy, if you realize the implications that 'virDomainRestore' would >> be stuck with recreating the same mirror layout. >> >> @@ -4947,6 +4952,12 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { >> goto cleanup; >> } >> def = NULL; >> + if (virDomainHasDiskMirror(vm)) { >> + qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s", >> + _("domain has active block copy job")); >> + virDomainObjAssignDef(vm, NULL, false); >> + goto cleanup; >> + } >> vm->persistent = 1; > > I think it would be better to do this check a bit earlier in the process to > avoid the need to undo virDomainObjAssignDef(). I see where you are coming from in the limited context shown in this patch (and it even matches my initial thoughts when first trying to write the patch), but look at the bigger picture: if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, false))) { goto cleanup; } def = NULL; + if (virDomainHasDiskMirror(vm)) { + qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s", + _("domain has active block copy job")); + virDomainObjAssignDef(vm, NULL, false); + goto cleanup; + } vm->persistent = 1; That is, we don't know what vm is, to check if it has a disk mirror, until after we have already associated a potential persistent definition