12:06 a.m.
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(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 81ada48..5674f4f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -269,6 +269,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"splash-timeout", /* 175 */
"iothread",
+ "block-stream",
);
@@ -1426,6 +1427,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-spice", QEMU_CAPS_SPICE },
{ "query-kvm", QEMU_CAPS_KVM },
{ "block-commit", QEMU_CAPS_BLOCK_COMMIT },
+ { "block-stream", QEMU_CAPS_BLOCK_STREAM },
{ "query-vnc", QEMU_CAPS_VNC },
{ "drive-mirror", QEMU_CAPS_DRIVE_MIRROR },
{ "blockdev-snapshot-sync", QEMU_CAPS_DISK_SNAPSHOT },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 0980c00..6701965 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -216,6 +216,7 @@ typedef enum {
QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor command */
QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */
QEMU_CAPS_OBJECT_IOTHREAD = 176, /* -object iothread */
+ QEMU_CAPS_BLOCK_STREAM = 177, /* block-stream */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 73edda3..b38bc91 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14980,15 +14980,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("block jobs not supported with this QEMU binary"));
goto cleanup;
- } else if (base) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("partial block pull not supported with this "
- "QEMU binary"));
- goto cleanup;
- } else if (mode == BLOCK_JOB_PULL && bandwidth) {
+ }
+
+ if (mode == BLOCK_JOB_PULL &&
+ !(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_STREAM))){
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("setting bandwidth at start of block pull not "
- "supported with this QEMU binary"));
+ _("block pull is not supported with this QEMU
binary"));
goto cleanup;
}
--
1.8.3.1
2:05 p.m.
New subject: [libvirt] [PATCH] qemu: add capability probing for block-stream
Ping?
On 09/16/2014 01: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(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 81ada48..5674f4f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -269,6 +269,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"splash-timeout", /* 175 */
"iothread",
+ "block-stream",
);
@@ -1426,6 +1427,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-spice", QEMU_CAPS_SPICE },
{ "query-kvm", QEMU_CAPS_KVM },
{ "block-commit", QEMU_CAPS_BLOCK_COMMIT },
+ { "block-stream", QEMU_CAPS_BLOCK_STREAM },
{ "query-vnc", QEMU_CAPS_VNC },
{ "drive-mirror", QEMU_CAPS_DRIVE_MIRROR },
{ "blockdev-snapshot-sync", QEMU_CAPS_DISK_SNAPSHOT },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 0980c00..6701965 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -216,6 +216,7 @@ typedef enum {
QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor command */
QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */
QEMU_CAPS_OBJECT_IOTHREAD = 176, /* -object iothread */
+ QEMU_CAPS_BLOCK_STREAM = 177, /* block-stream */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 73edda3..b38bc91 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14980,15 +14980,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("block jobs not supported with this QEMU binary"));
goto cleanup;
- } else if (base) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("partial block pull not supported with this "
- "QEMU binary"));
- goto cleanup;
- } else if (mode == BLOCK_JOB_PULL && bandwidth) {
+ }
+
+ if (mode == BLOCK_JOB_PULL &&
+ !(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_STREAM))){
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("setting bandwidth at start of block pull not "
- "supported with this QEMU binary"));
+ _("block pull is not supported with this QEMU
binary"));
goto cleanup;
}
--
Regards
shyu
2:25 p.m.
New subject: [libvirt] [PATCH] qemu: add capability probing for block-stream
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(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 81ada48..5674f4f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -269,6 +269,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"splash-timeout", /* 175 */
"iothread",
+ "block-stream",
);
@@ -1426,6 +1427,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-spice", QEMU_CAPS_SPICE },
{ "query-kvm", QEMU_CAPS_KVM },
{ "block-commit", QEMU_CAPS_BLOCK_COMMIT },
+ { "block-stream", QEMU_CAPS_BLOCK_STREAM },
This part seems okay (we really haven't tracked this before?)...
+++ b/src/qemu/qemu_driver.c
@@ -14980,15 +14980,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("block jobs not supported with this QEMU binary"));
goto cleanup;
- } else if (base) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("partial block pull not supported with this "
- "QEMU binary"));
- goto cleanup;
- } else if (mode == BLOCK_JOB_PULL && bandwidth) {
+ }
+
+ if (mode == BLOCK_JOB_PULL &&
+ !(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_STREAM))){
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
}
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:36 p.m.
New subject: [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
3716
days inactive
3754
days old
3 comments
2 participants
participants (2)
-
Eric Blake -
Shanzhi Yu