On 11/27/13 09:41, Hu Tao wrote: > qemu removes the builtin pvpanic device for all qemu versions since 1.7, > in order to support <on_crash>, '-device pvpanic' has to be added to > qemu command line. > > Signed-off-by: Hu Tao <[email protected]> > --- > src/qemu/qemu_capabilities.c | 8 ++++++++ > src/qemu/qemu_capabilities.h | 2 ++ > src/qemu/qemu_command.c | 4 ++++ > 3 files changed, 14 insertions(+) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 548b988..7783997 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -243,6 +243,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, > "virtio-mmio", > "ich9-intel-hda", > "kvm-pit-lost-tick-policy", > + > + "pvpanic", /* 160 */ > ); > > struct _virQEMUCaps { > @@ -1198,6 +1200,9 @@ virQEMUCapsComputeCmdFlags(const char *help, > virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); > } > > + if (version >= 1005000) > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PVPANIC); > +
Hard coding the version number is not a good idea. Libvirt uses qmp
monitor queries to determine supported devices.
Please add this in the virQEMUCapsObjectTypes structure instead, which
will do the qmp detection for you.
> return 0;
> }
>
> @@ -2561,6 +2566,9 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
> if (qemuCaps->version >= 1006000)
> virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
>
> + if (qemuCaps->version >= 1005000)
> + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PVPANIC);
> +
Same here ... this should be autoprobed by a function using the
structure above.
> if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
> goto cleanup;
> if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0)
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 02d47c6..06d2fac 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -199,6 +199,8 @@ enum virQEMUCapsFlags {
> QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */
> QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */
>
> + QEMU_CAPS_PVPANIC = 160, /* -device pvpanic */
> +
> QEMU_CAPS_LAST, /* this must always be the last item */
> };
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 763417f..b1307a3 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9588,6 +9588,10 @@ qemuBuildCommandLine(virConnectPtr conn,
> goto error;
> }
>
> + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PVPANIC)) {
> + virCommandAddArgList(cmd, "-device", "pvpanic", NULL);
> + }
> +
I remember discussions saying that it's NOT a good idea to enable this
stuff always. As a result, this device is not being added by qemu as you
described above. Shouldn't we only add this if the user enables
<on_crash> actions?
> if (mlock) {
> unsigned long long memKB;
>
>
Peter
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
