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 <[email protected]> >> --- >> 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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
