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@xxxxxxxxxx> >> --- >> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list