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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org