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@xxxxxxxxxx +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