Re: [libvirt] [PATCH] qemu: add capability probing for block-stream
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)