Re: [libvirt] [PATCH] blockcommit: document semantics of committing active layer
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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)