7:38 a.m.
In commit c4bbaaf8, caps->arch was checked uninitialized, rendering the
whole check useless.
This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
qemuCapsInitQMP, and removes the no longer needed exception for S390.
It also clears the flag for all non-x86 archs instead of just S390 in
qemuCapsInitHelp.
---
src/qemu/qemu_capabilities.c | 29 ++++++++---------------------
1 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 167cdb2..e0d0c2a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2175,13 +2175,11 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid)
if (caps->arch == VIR_ARCH_X86_64 ||
caps->arch == VIR_ARCH_I686) {
qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS);
- }
-
- /* S390 and probably other archs do not support no-acpi -
- maybe the qemu option parsing should be re-thought. */
- if (caps->arch == VIR_ARCH_S390 ||
- caps->arch == VIR_ARCH_S390X)
+ } else {
+ /* -no-acpi is not supported on other archs
+ * even if qemu reports it in -help */
qemuCapsClear(caps, QEMU_CAPS_NO_ACPI);
+ }
/* qemuCapsExtractDeviceStr will only set additional caps if qemu
* understands the 0.13.0+ notion of "-device driver,". */
@@ -2260,13 +2258,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
qemuCapsSet(caps, QEMU_CAPS_NO_SHUTDOWN);
qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_UNSAFE);
-
- /* ACPI is only supported on x86, PPC or
- * other platforms don't support it*/
- if (caps->arch == VIR_ARCH_I686 ||
- caps->arch == VIR_ARCH_X86_64)
- qemuCapsSet(caps, QEMU_CAPS_NO_ACPI);
-
qemuCapsSet(caps, QEMU_CAPS_FSDEV_READONLY);
qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_SG_IO);
qemuCapsSet(caps, QEMU_CAPS_DRIVE_COPY_ON_READ);
@@ -2402,16 +2393,12 @@ qemuCapsInitQMP(qemuCapsPtr caps,
}
VIR_FREE(archstr);
- /* Currently only x86_64 and i686 support PCI-multibus. */
+ /* Currently only x86_64 and i686 support PCI-multibus and -no-acpi. */
if (caps->arch == VIR_ARCH_X86_64 ||
- caps->arch == VIR_ARCH_I686)
+ caps->arch == VIR_ARCH_I686) {
qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS);
-
- /* S390 and probably other archs do not support no-acpi -
- maybe the qemu option parsing should be re-thought. */
- if (caps->arch == VIR_ARCH_S390 ||
- caps->arch == VIR_ARCH_S390X)
- qemuCapsClear(caps, QEMU_CAPS_NO_ACPI);
+ qemuCapsSet(caps, QEMU_CAPS_NO_ACPI);
+ }
if (qemuCapsProbeQMPCommands(caps, mon) < 0)
goto cleanup;
--
1.7.8.6
7:55 a.m.
On 12/21/2012 02:38 PM, Ján Tomko wrote:
In commit c4bbaaf8, caps->arch was checked uninitialized,
rendering the
whole check useless.
This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
qemuCapsInitQMP, and removes the no longer needed exception for S390.
It also clears the flag for all non-x86 archs instead of just S390 in
qemuCapsInitHelp.
---
src/qemu/qemu_capabilities.c | 29 ++++++++---------------------
1 files changed, 8 insertions(+), 21 deletions(-)
Definitely good to see any S390 special handling become obsolete.
+1 from my side.
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:07 a.m.
On Fri, Dec 21, 2012 at 9:38 PM, Ján Tomko <jtomko(a)redhat.com> wrote:
In commit c4bbaaf8, caps->arch was checked uninitialized,
rendering the
whole check useless.
This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
qemuCapsInitQMP, and removes the no longer needed exception for S390.
Will it get QEMU_CAPS_NO_ACPI capability by QMP if S390 doesn't support
ACPI?
I test it on PPC, it won't get this capability by QMP.
Thanks.
It also clears the flag for all non-x86 archs instead of just S390 in
qemuCapsInitHelp.
---
src/qemu/qemu_capabilities.c | 29 ++++++++---------------------
1 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 167cdb2..e0d0c2a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2175,13 +2175,11 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid,
gid_t runGid)
if (caps->arch == VIR_ARCH_X86_64 ||
caps->arch == VIR_ARCH_I686) {
qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS);
- }
-
- /* S390 and probably other archs do not support no-acpi -
- maybe the qemu option parsing should be re-thought. */
- if (caps->arch == VIR_ARCH_S390 ||
- caps->arch == VIR_ARCH_S390X)
+ } else {
+ /* -no-acpi is not supported on other archs
+ * even if qemu reports it in -help */
qemuCapsClear(caps, QEMU_CAPS_NO_ACPI);
+ }
/* qemuCapsExtractDeviceStr will only set additional caps if qemu
* understands the 0.13.0+ notion of "-device driver,". */
@@ -2260,13 +2258,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
qemuCapsSet(caps, QEMU_CAPS_NO_SHUTDOWN);
qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_UNSAFE);
-
- /* ACPI is only supported on x86, PPC or
- * other platforms don't support it*/
- if (caps->arch == VIR_ARCH_I686 ||
- caps->arch == VIR_ARCH_X86_64)
- qemuCapsSet(caps, QEMU_CAPS_NO_ACPI);
-
qemuCapsSet(caps, QEMU_CAPS_FSDEV_READONLY);
qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_SG_IO);
qemuCapsSet(caps, QEMU_CAPS_DRIVE_COPY_ON_READ);
@@ -2402,16 +2393,12 @@ qemuCapsInitQMP(qemuCapsPtr caps,
}
VIR_FREE(archstr);
- /* Currently only x86_64 and i686 support PCI-multibus. */
+ /* Currently only x86_64 and i686 support PCI-multibus and -no-acpi.
*/
if (caps->arch == VIR_ARCH_X86_64 ||
- caps->arch == VIR_ARCH_I686)
+ caps->arch == VIR_ARCH_I686) {
qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS);
-
- /* S390 and probably other archs do not support no-acpi -
- maybe the qemu option parsing should be re-thought. */
- if (caps->arch == VIR_ARCH_S390 ||
- caps->arch == VIR_ARCH_S390X)
- qemuCapsClear(caps, QEMU_CAPS_NO_ACPI);
+ qemuCapsSet(caps, QEMU_CAPS_NO_ACPI);
+ }
if (qemuCapsProbeQMPCommands(caps, mon) < 0)
goto cleanup;
--
1.7.8.6
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Best Regards
-Li
8:20 a.m.
On 12/21/12 15:07, Li Zhang wrote:
On Fri, Dec 21, 2012 at 9:38 PM, Ján Tomko <jtomko(a)redhat.com
<mailto:jtomko@redhat.com>> wrote:
In commit c4bbaaf8, caps->arch was checked uninitialized, rendering the
whole check useless.
This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
qemuCapsInitQMP, and removes the no longer needed exception for S390.
Will it get QEMU_CAPS_NO_ACPI capability by QMP if S390 doesn't support
ACPI?
I test it on PPC, it won't get this capability by QMP.
Thanks.
No, QEMU_CAPS_NO_ACPI capability will only be set on i686 and x86_64.
Jan
8:40 a.m.
On Fri, Dec 21, 2012 at 10:20 PM, Ján Tomko <jtomko(a)redhat.com> wrote:
--
Best Regards
-Li
On 12/21/12 15:07, Li Zhang wrote:
> On Fri, Dec 21, 2012 at 9:38
PM, Ján Tomko <jtomko(a)redhat.com
> <mailto:jtomko@redhat.com>> wrote:
> In commit c4bbaaf8, caps->arch was checked
uninitialized, rendering
the
> whole check useless.
> This patch moves the conditional setting of
QEMU_CAPS_NO_ACPI to
> qemuCapsInitQMP, and removes the no longer needed exception for S390.
> Will it get QEMU_CAPS_NO_ACPI
capability by QMP if S390 doesn't support
> ACPI?
> I test it on PPC, it won't get this capability by QMP.
> Thanks.
No, QEMU_CAPS_NO_ACPI capability will only be set on i686 and x86_64.
So I think there shouldn't be the conditional setting in qemuCapsInitQMP.
It doesn't need to clear this capability in this place.
Jan
8:37 a.m.
On 12/21/2012 03:07 PM, Li Zhang wrote:
Will it get QEMU_CAPS_NO_ACPI capability by QMP if S390 doesn't support
ACPI?
I test it on PPC, it won't get this capability by QMP.
In the QMP case, basic capabilities are not being probed, but assumed
to be present, however -no-acpi is only valid for x86, hence the
conditional setting.
The old help parsing code on older QEMUs could report -no-acpi for
S390 (respectively non-x86), which has to be removed from the
capability set to avoid errors starting the guest.
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:45 a.m.
On Fri, Dec 21, 2012 at 10:37 PM, Viktor Mihajlovski <
mihajlov(a)linux.vnet.ibm.com> wrote:
On 12/21/2012 03:07 PM, Li Zhang wrote:
>
> Will it get QEMU_CAPS_NO_ACPI capability by QMP if S390 doesn't support
> ACPI?
> I test it on PPC, it won't get this capability by QMP.
>
> In the QMP case, basic capabilities are not being probed, but assumed
to be present, however -no-acpi is only valid for x86, hence the
conditional setting.
You mean it still is present. I didn't get it on POWER.
The old help parsing code on older QEMUs could report -no-acpi for
S390 (respectively non-x86), which has to be removed from the
capability set to avoid errors starting the guest.
On POWER, QEMU's help string doesn't include -no-acpi.
It seems that QEMU handles this differently. :)
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
Best Regards
-Li
9:14 a.m.
On 12/21/2012 03:45 PM, Li Zhang wrote:
On POWER, QEMU's help string doesn't include -no-acpi.
It seems that QEMU handles this differently. :)
it depends on the version: newer QEMUs for S390 don't include
-no-acpi either ... but libvirt should remain compatible
with older QEMUs
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
5:47 a.m.
On 12/21/12 14:38, Ján Tomko wrote:
In commit c4bbaaf8, caps->arch was checked uninitialized,
rendering the
whole check useless.
This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
qemuCapsInitQMP, and removes the no longer needed exception for S390.
It also clears the flag for all non-x86 archs instead of just S390 in
qemuCapsInitHelp.
---
src/qemu/qemu_capabilities.c | 29 ++++++++---------------------
1 files changed, 8 insertions(+), 21 deletions(-)
gentle ping
6:01 a.m.
On 2013年01月15日 19:47, Ján Tomko wrote:
On 12/21/12 14:38, Ján Tomko wrote:
> In commit c4bbaaf8, caps->arch was checked uninitialized, rendering the
> whole check useless.
>
> This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
> qemuCapsInitQMP, and removes the no longer needed exception for S390.
>
> It also clears the flag for all non-x86 archs instead of just S390 in
> qemuCapsInitHelp.
> ---
> src/qemu/qemu_capabilities.c | 29 ++++++++---------------------
> 1 files changed, 8 insertions(+), 21 deletions(-)
>
gentle ping
This looks good for PPC. :)
Thanks.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5:04 p.m.
On 12/21/2012 06:38 AM, Ján Tomko wrote:
In commit c4bbaaf8, caps->arch was checked uninitialized,
rendering the
whole check useless.
This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
qemuCapsInitQMP, and removes the no longer needed exception for S390.
It also clears the flag for all non-x86 archs instead of just S390 in
qemuCapsInitHelp.
---
src/qemu/qemu_capabilities.c | 29 ++++++++---------------------
1 files changed, 8 insertions(+), 21 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:52 a.m.
On 01/16/13 00:04, Eric Blake wrote:
On 12/21/2012 06:38 AM, Ján Tomko wrote:
> In commit c4bbaaf8, caps->arch was checked uninitialized, rendering the
> whole check useless.
>
> This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
> qemuCapsInitQMP, and removes the no longer needed exception for S390.
>
> It also clears the flag for all non-x86 archs instead of just S390 in
> qemuCapsInitHelp.
> ---
> src/qemu/qemu_capabilities.c | 29 ++++++++---------------------
> 1 files changed, 8 insertions(+), 21 deletions(-)
ACK.
Thanks, pushed.
4329
days inactive
4355
days old
11 comments
4 participants
participants (4)
-
Eric Blake -
Ján Tomko -
Li Zhang -
Viktor Mihajlovski