On 05/19/2014 01:58 AM, Peter Krempa wrote:
[Sorry for not putting RFC in the subject line]
>> +++ b/src/qemu/qemu_driver.c
>> @@ -15377,6 +15377,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
>> const char *top_parent = NULL;
>> bool clean_access = false;
>>
>> + /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
>> virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
>>
>> if (!(vm = qemuDomObjFromDomain(dom)))
>> @@ -15423,6 +15424,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
>> &top_parent)))
>> goto endjob;
>>
>> + /* XXX Should we auto-pivot when COMMIT_ACTIVE is not specified? */
>
> I think we should. But I agree in postponing it to later patch.
>
>> + if (topSource == &disk->src && !(flags & VIR_DOMAIN_BLOCK_COPY_ACTIVE)) {
>
> As mentioned in the previous mail, this breaks build.
>
> s/BLOCK_COPY/BLOCK_COMMIT/
Blah - serves me right for sending a patch at the end of my day. But I'm
glad you were able to see the intent.
>
> Also shouldn't this hunk actually go in the patch that adds the active
> block commit feature? It seems a bit out of place here.
_This_ patch is fixing the qemu driver to not hang, by acknowledging
that we _don't_ support active block commit (the virCheckFlags() at the
beginning of the function fails if the new flag is specified, and this
check fails if the flag is not specified - which means you cannot do
active commits). The next few patches (when I develop it into a full
series) will first be to add support for the new flag (fixing the first
XXX) and then to relax things to auto-pivot when the flag is not present
(fixing the second XXX).
>
>> + virReportError(VIR_ERR_INVALID_ARG,
>> + _("commit of '%s' active layer requires active flag"),
>> + disk->dst);
>> + goto endjob;
>> + }
>> +
>> if (!topSource->backingStore) {
>> virReportError(VIR_ERR_INVALID_ARG,
>> _("top '%s' in chain for '%s' has no backing file"),
>>
>
> I think that this patch should also export this new flag in virsh as we
> usually do with API flag additions.
Yes, that's my next task before turning this from an RFC into an actual
patch series, even before the qemu implementation is done.
>
> Additionally a change in virsh is missing again breaks the build process:
Yep, my fault for not actually compile-testing in my rush to get it out
before the weekend :)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list