Re: [libvirt] [PATCH 3/3] block rebase: initial qemu implementation
On 02/01/2012 12:53 PM, Laine Stump wrote:
On 02/01/2012 12:06 AM, Eric Blake wrote:
> This is a trivial implementation, which works with the current
> released qemu 1.0 with backports of preliminary block pull but
> no partial rebase. Future patches will update the monitor handling
> to support an optional parameter for partial rebase; but as qemu
> 1.1 is unreleased, it can be in later patches, designed to be
> backported on top of the supported API.
>
> * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Add parameter,
> @@ -11328,16 +11328,18 @@ qemuDomainBlockJobImpl(virDomainPtr dom,
> const char *path,
> goto cleanup;
> }
>
> - if (!virDomainObjIsActive(vm)) {
> - qemuReportError(VIR_ERR_OPERATION_INVALID,
> - "%s", _("domain is not running"));
> - goto cleanup;
> - }
> -
For all the existing virDomainBlockxxx() APIs I think this is a change
in behavior, right? THey used to fial for inactive domains, and now they
may succeed. Could this create any problems?
No. This was a case of redundant code - we checked for isactive, then
obtained the job, then re-checked for isactive (since obtaining the job
involves dropping lock, so the domain could have quit in the meantime).
The post-job check is mandatory (we've had bugs before where libvirtd
locks up if we forget the post-job check), and the pre-job check can be
safely be eliminated since the post-job check will always do the right
thing; the only difference in eliminating the pre-job check is that the
API might take a bit longer due to trying to obtain the job on an
inactive domain compared to where it used to error out without even
trying to get the job.
If not, then ACK. This is all pretty mechanical.
I think I answered your question, and thus I'm pushing the series.
> device = qemuDiskPathToAlias(vm, path);
> if (!device) {
> goto cleanup;
> }
> + /* XXX - add a qemu capability check; if qemu 1.1 or newer, then
> + * validate and convert non-NULL base into something that can
> + * be passed as optional base argument. */
> + if (base) {
> + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> + _("partial block pull is not supported with
> this QEMU binary"));
> + goto cleanup;
> + }
So the API is in, and the actual functionality missing until qemu that
supports it is available.
And I'm in the process of writing/testing that as well, it's just that
it's a bit lower on my priority queue :)
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)