[libvirt] [PATCH] qemu: add capability probing for block-stream

[Eric Blake]

On 10/24/2014 01:25 PM, Eric Blake wrote:
> On 09/15/2014 11:06 PM, Shanzhi Yu wrote:
>> Since block-stream is not supported on qemu-kvm, so libvirt should
>> post more accurate error info when do blockpull with qemu-kvm but
>> not "Command 'block-stream' is not found"
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140981
>> Signed-off-by: Shanzhi Yu <shyu at redhat.com>
>> ---
>>  src/qemu/qemu_capabilities.c |  2 ++
>>  src/qemu/qemu_capabilities.h |  1 +
>>  src/qemu/qemu_driver.c       | 13 +++++--------
>>  3 files changed, 8 insertions(+), 8 deletions(-)
>>

Hmm, I did a bit more digging.  When I originally wrote the code, I used
the presence of 'block-job-cancel' (new style, BLOCKJOB_ASYNC) vs.
'block_job_cancel' (old style, BLOCKJOB_SYNC) as a witness whether
either style of block jobs was supported.  However, it appears that RHEL
qemu-kvm is providing block-job-cancel EVEN THOUGH it lacks all methods
for starting a block job, which is different from upstream (in upstream,
even with the older spelling block_job_cancel, cancel was only
introduced at the same time as block-stream).

So this is more of a downstream issue - if RHEL qemu-kvm is going to
cripple block jobs, they should do so by also crippling cancellation of
jobs.


> But here you are throwing away existing useful error messages for other
> situations (such as when block-stream exists, but is too old to support
> a base).  I think the flow would look a bit better as:
> 
>   if (..._ASYNC) {
>       async = true;
>   } else if (!..._SYNC) {
>       error: block jobs unsupported
> + } else if (mode == BLOCK_JOB_PULL && !...STREAM) {
> +     error: block pull unsupported
>   } else if (base) {
>       error: partial pull unsupported
>   } else if (mode == BLOCK_JOB_PULL && bandwidth) {
>       error: bandwidth unsupported
>   }

Or even simpler - change how we compute whether BLOCKJOB_SYNC capability
is probed, to refuse to set it if block-stream is missing.  By changing
_just_ qemu_capabilities.c, the existing code here in qemu_driver.c
would automatically start printing a nice error message when targetting
crippled RHEL qemu.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141024/5388b379/attachment-0001.sig>