* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-19 19:08 ` Don Slutz
@ 2014-11-19 19:15 ` Don Slutz
2014-11-19 19:30 ` Eduardo Habkost
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Don Slutz @ 2014-11-19 19:15 UTC (permalink / raw)
To: Don Slutz, Paolo Bonzini, qemu-devel, Dr. David Alan Gilbert,
Michael S. Tsirkin, Eduardo Habkost
Cc: Michael Tokarev, Anthony Liguori, Stefan Hajnoczi
P.S. I think the title:
xen-common.c: If "-machine vmport=" is not specified turn vmport off
is better. Will send v2 of patch out soon.
-Don Slutz
On 11/19/14 14:08, Don Slutz wrote:
>
> On 11/19/14 13:08, Paolo Bonzini wrote:
>>
>> On 19/11/2014 19:07, Don Slutz wrote:
>>>> "-M pc -machine accel=xen" should work and, if that's what you want,
>>>> disable the vmport device. I think this patch is wrong.
>>>>
>>>> Paolo
>>> Well, I also want "-M pc -machine accel=xen,vmport=on" to work.
>> Right. So let's start by deciding what the desired semantics are for
>> all six cases: -M pc/xenfv, -machine vmport=on/off/absent.
>>
>> Paolo
>
> I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)):
>
>
>
> -M pc
> pcms->vmport is true
> -M pc -machine vmport=on
> pcms->vmport is true
> -M pc -machine vmport=off
> pcms->vmport is false
> -M xenfv
> pcms->vmport is false
> -M xenfv -machine vmport=on
> pcms->vmport is true
> -M xenfv -machine vmport=off
> pcms->vmport is false
>
> -M pc -machine accel=xen
> pcms->vmport is false
> -M pc -machine vmport=on,accel=xen
> pcms->vmport is true
> -M pc -machine vmport=off,accel=xen
> pcms->vmport is false
> -M xenfv -machine accel=xen
> pcms->vmport is false
> -M xenfv -machine vmport=on,accel=xen
> pcms->vmport is true
> -M xenfv -machine vmport=off,accel=xen
> pcms->vmport is false
>
>
> Which look like to me to solve down to xen_init() called via
> xen_accel_class_init()
> need to see if vmport has been specified and change it if needed.
>
> Which leads me to:
>
> commit 436d056420bb209d066d2378b628d297777d9f20
> Author: Don Slutz <dslutz@verizon.com>
> Date: Wed Nov 19 10:01:16 2014 -0500
>
> hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
>
> c/s 9b23cfb76b3a5e9eb5cc899eaf2f46bc46d33ba4
>
> or
>
> c/s b154537ad07598377ebf98252fb7d2aff127983b
>
> moved the testing of xen_enabled() from pc_init1() to
> pc_machine_initfn().
>
> xen_enabled() does not return the correct value in
> pc_machine_initfn().
>
> Add vmport_changed to track the state of vmport so that
> accel=xen can do the right thing.
>
> Drop the call to xen_enabled() in pc_machine_initfn() to reduce
> potential confusion.
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 1205db8..b400ac8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1723,6 +1723,7 @@ static void pc_machine_set_vmport(Object *obj,
> bool value, Error **errp)
> PCMachineState *pcms = PC_MACHINE(obj);
>
> pcms->vmport = value;
> + pcms->vmport_changed = true;
> }
>
> static void pc_machine_initfn(Object *obj)
> @@ -1737,7 +1738,7 @@ static void pc_machine_initfn(Object *obj)
> pc_machine_get_max_ram_below_4g,
> pc_machine_set_max_ram_below_4g,
> NULL, NULL, NULL);
> - pcms->vmport = !xen_enabled();
> + pcms->vmport = true;
> object_property_add_bool(obj, PC_MACHINE_VMPORT,
> pc_machine_get_vmport,
> pc_machine_set_vmport,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 7c3731f..5782406 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -38,6 +38,7 @@ struct PCMachineState {
>
> uint64_t max_ram_below_4g;
> bool vmport;
> + bool vmport_changed;
> };
>
> #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
> diff --git a/xen-common.c b/xen-common.c
> index 56359ca..bb11bb7 100644
> --- a/xen-common.c
> +++ b/xen-common.c
> @@ -12,6 +12,7 @@
> #include "qmp-commands.h"
> #include "sysemu/char.h"
> #include "sysemu/accel.h"
> +#include "hw/i386/pc.h"
>
> //#define DEBUG_XEN
>
> @@ -112,6 +113,11 @@ static void xen_change_state_handler(void
> *opaque, int running,
>
> static int xen_init(MachineState *ms)
> {
> + PCMachineState *pcms = PC_MACHINE(ms);
> +
> + if (!pcms->vmport_changed) {
> + pcms->vmport = false;
> + }
> xen_xc = xen_xc_interface_open(0, 0, 0);
> if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
> xen_be_printf(NULL, 0, "can't open xen interface\n");
>
>
> -Don Slutz
>
>
>
>>> However you bring up a good point and I will see if I can quickly
>>> handled it.
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-19 19:08 ` Don Slutz
2014-11-19 19:15 ` Don Slutz
@ 2014-11-19 19:30 ` Eduardo Habkost
2014-11-19 20:01 ` Don Slutz
2014-11-20 6:02 ` Paolo Bonzini
2014-11-20 15:13 ` Eduardo Habkost
3 siblings, 1 reply; 15+ messages in thread
From: Eduardo Habkost @ 2014-11-19 19:30 UTC (permalink / raw)
To: Don Slutz
Cc: Anthony Liguori, Michael S. Tsirkin, Michael Tokarev, qemu-devel,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini
On Wed, Nov 19, 2014 at 02:08:08PM -0500, Don Slutz wrote:
> On 11/19/14 13:08, Paolo Bonzini wrote:
> >On 19/11/2014 19:07, Don Slutz wrote:
> >>>"-M pc -machine accel=xen" should work and, if that's what you want,
> >>>disable the vmport device. I think this patch is wrong.
> >>>
> >>>Paolo
> >>Well, I also want "-M pc -machine accel=xen,vmport=on" to work.
> >Right. So let's start by deciding what the desired semantics are for
> >all six cases: -M pc/xenfv, -machine vmport=on/off/absent.
> >
> >Paolo
>
> I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)):
>
>
>
> -M pc
> pcms->vmport is true
> -M pc -machine vmport=on
> pcms->vmport is true
> -M pc -machine vmport=off
> pcms->vmport is false
> -M xenfv
> pcms->vmport is false
> -M xenfv -machine vmport=on
> pcms->vmport is true
> -M xenfv -machine vmport=off
> pcms->vmport is false
>
> -M pc -machine accel=xen
> pcms->vmport is false
> -M pc -machine vmport=on,accel=xen
> pcms->vmport is true
> -M pc -machine vmport=off,accel=xen
> pcms->vmport is false
> -M xenfv -machine accel=xen
> pcms->vmport is false
> -M xenfv -machine vmport=on,accel=xen
> pcms->vmport is true
> -M xenfv -machine vmport=off,accel=xen
> pcms->vmport is false
>
>
> Which look like to me to solve down to xen_init() called via
> xen_accel_class_init()
> need to see if vmport has been specified and change it if needed.
>
> Which leads me to:
>
[...]
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 7c3731f..5782406 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -38,6 +38,7 @@ struct PCMachineState {
>
> uint64_t max_ram_below_4g;
> bool vmport;
> + bool vmport_changed;
So, now the setting have three possible states: unset, on, and off.
If we can't avoid that (which seems to be the case, as "accel" is
changed after we already set the default value for "vmport" in instance
init), maybe we should make it an enum property that accepts on/off/auto
as values, instead of faking a boolean property that is not really a
boolean?
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-19 19:30 ` Eduardo Habkost
@ 2014-11-19 20:01 ` Don Slutz
2014-11-20 0:24 ` Eduardo Habkost
0 siblings, 1 reply; 15+ messages in thread
From: Don Slutz @ 2014-11-19 20:01 UTC (permalink / raw)
To: Eduardo Habkost, Don Slutz
Cc: Anthony Liguori, Michael S. Tsirkin, Michael Tokarev, qemu-devel,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini
On 11/19/14 14:30, Eduardo Habkost wrote:
> On Wed, Nov 19, 2014 at 02:08:08PM -0500, Don Slutz wrote:
>> On 11/19/14 13:08, Paolo Bonzini wrote:
>>> On 19/11/2014 19:07, Don Slutz wrote:
>>>>> "-M pc -machine accel=xen" should work and, if that's what you want,
>>>>> disable the vmport device. I think this patch is wrong.
>>>>>
>>>>> Paolo
>>>> Well, I also want "-M pc -machine accel=xen,vmport=on" to work.
>>> Right. So let's start by deciding what the desired semantics are for
>>> all six cases: -M pc/xenfv, -machine vmport=on/off/absent.
>>>
>>> Paolo
>> I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)):
>>
>>
>>
>> -M pc
>> pcms->vmport is true
>> -M pc -machine vmport=on
>> pcms->vmport is true
>> -M pc -machine vmport=off
>> pcms->vmport is false
>> -M xenfv
>> pcms->vmport is false
>> -M xenfv -machine vmport=on
>> pcms->vmport is true
>> -M xenfv -machine vmport=off
>> pcms->vmport is false
>>
>> -M pc -machine accel=xen
>> pcms->vmport is false
>> -M pc -machine vmport=on,accel=xen
>> pcms->vmport is true
>> -M pc -machine vmport=off,accel=xen
>> pcms->vmport is false
>> -M xenfv -machine accel=xen
>> pcms->vmport is false
>> -M xenfv -machine vmport=on,accel=xen
>> pcms->vmport is true
>> -M xenfv -machine vmport=off,accel=xen
>> pcms->vmport is false
>>
>>
>> Which look like to me to solve down to xen_init() called via
>> xen_accel_class_init()
>> need to see if vmport has been specified and change it if needed.
>>
>> Which leads me to:
>>
> [...]
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 7c3731f..5782406 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -38,6 +38,7 @@ struct PCMachineState {
>>
>> uint64_t max_ram_below_4g;
>> bool vmport;
>> + bool vmport_changed;
> So, now the setting have three possible states: unset, on, and off.
>
> If we can't avoid that (which seems to be the case, as "accel" is
> changed after we already set the default value for "vmport" in instance
> init), maybe we should make it an enum property that accepts on/off/auto
> as values, instead of faking a boolean property that is not really a
> boolean?
>
I would be happy to look into this. I have not done anything that used
an enum property, and so it is hard to guess on how long it will take to
convert the change to this.
-Don Slutz
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-19 20:01 ` Don Slutz
@ 2014-11-20 0:24 ` Eduardo Habkost
2014-11-20 0:49 ` Don Slutz
0 siblings, 1 reply; 15+ messages in thread
From: Eduardo Habkost @ 2014-11-20 0:24 UTC (permalink / raw)
To: Don Slutz
Cc: Anthony Liguori, Michael S. Tsirkin, Michael Tokarev, qemu-devel,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini
On Wed, Nov 19, 2014 at 03:01:24PM -0500, Don Slutz wrote:
> On 11/19/14 14:30, Eduardo Habkost wrote:
> >On Wed, Nov 19, 2014 at 02:08:08PM -0500, Don Slutz wrote:
> >>On 11/19/14 13:08, Paolo Bonzini wrote:
> >>>On 19/11/2014 19:07, Don Slutz wrote:
> >>>>>"-M pc -machine accel=xen" should work and, if that's what you want,
> >>>>>disable the vmport device. I think this patch is wrong.
> >>>>>
> >>>>>Paolo
> >>>>Well, I also want "-M pc -machine accel=xen,vmport=on" to work.
> >>>Right. So let's start by deciding what the desired semantics are for
> >>>all six cases: -M pc/xenfv, -machine vmport=on/off/absent.
> >>>
> >>>Paolo
> >>I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)):
> >>
> >>
> >>
> >>-M pc
> >> pcms->vmport is true
> >>-M pc -machine vmport=on
> >> pcms->vmport is true
> >>-M pc -machine vmport=off
> >> pcms->vmport is false
> >>-M xenfv
> >> pcms->vmport is false
> >>-M xenfv -machine vmport=on
> >> pcms->vmport is true
> >>-M xenfv -machine vmport=off
> >> pcms->vmport is false
> >>
> >>-M pc -machine accel=xen
> >> pcms->vmport is false
> >>-M pc -machine vmport=on,accel=xen
> >> pcms->vmport is true
> >>-M pc -machine vmport=off,accel=xen
> >> pcms->vmport is false
> >>-M xenfv -machine accel=xen
> >> pcms->vmport is false
> >>-M xenfv -machine vmport=on,accel=xen
> >> pcms->vmport is true
> >>-M xenfv -machine vmport=off,accel=xen
> >> pcms->vmport is false
> >>
> >>
> >>Which look like to me to solve down to xen_init() called via
> >>xen_accel_class_init()
> >>need to see if vmport has been specified and change it if needed.
> >>
> >>Which leads me to:
> >>
> >[...]
> >>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>index 7c3731f..5782406 100644
> >>--- a/include/hw/i386/pc.h
> >>+++ b/include/hw/i386/pc.h
> >>@@ -38,6 +38,7 @@ struct PCMachineState {
> >>
> >> uint64_t max_ram_below_4g;
> >> bool vmport;
> >>+ bool vmport_changed;
> >So, now the setting have three possible states: unset, on, and off.
> >
> >If we can't avoid that (which seems to be the case, as "accel" is
> >changed after we already set the default value for "vmport" in instance
> >init), maybe we should make it an enum property that accepts on/off/auto
> >as values, instead of faking a boolean property that is not really a
> >boolean?
> >
>
> I would be happy to look into this. I have not done anything that used
> an enum property, and so it is hard to guess on how long it will take to
> convert the change to this.
I think your patch may be a good intermediate solution, as long as you
add a comment noting that the return value of the "vmport" property may
be invalid before machine->init() is called, depending on the
machine-type.
Or, maybe a property without a getter would be appropriate?
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-20 0:24 ` Eduardo Habkost
@ 2014-11-20 0:49 ` Don Slutz
0 siblings, 0 replies; 15+ messages in thread
From: Don Slutz @ 2014-11-20 0:49 UTC (permalink / raw)
To: Eduardo Habkost, Don Slutz
Cc: Anthony Liguori, Michael S. Tsirkin, Michael Tokarev, qemu-devel,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini
On 11/19/14 19:24, Eduardo Habkost wrote:
> On Wed, Nov 19, 2014 at 03:01:24PM -0500, Don Slutz wrote:
>> On 11/19/14 14:30, Eduardo Habkost wrote:
>>> On Wed, Nov 19, 2014 at 02:08:08PM -0500, Don Slutz wrote:
>>>> On 11/19/14 13:08, Paolo Bonzini wrote:
>>>>> On 19/11/2014 19:07, Don Slutz wrote:
>>>>>>> "-M pc -machine accel=xen" should work and, if that's what you want,
>>>>>>> disable the vmport device. I think this patch is wrong.
>>>>>>>
>>>>>>> Paolo
>>>>>> Well, I also want "-M pc -machine accel=xen,vmport=on" to work.
>>>>> Right. So let's start by deciding what the desired semantics are for
>>>>> all six cases: -M pc/xenfv, -machine vmport=on/off/absent.
>>>>>
>>>>> Paolo
>>>> I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)):
>>>>
>>>>
>>>>
>>>> -M pc
>>>> pcms->vmport is true
>>>> -M pc -machine vmport=on
>>>> pcms->vmport is true
>>>> -M pc -machine vmport=off
>>>> pcms->vmport is false
>>>> -M xenfv
>>>> pcms->vmport is false
>>>> -M xenfv -machine vmport=on
>>>> pcms->vmport is true
>>>> -M xenfv -machine vmport=off
>>>> pcms->vmport is false
>>>>
>>>> -M pc -machine accel=xen
>>>> pcms->vmport is false
>>>> -M pc -machine vmport=on,accel=xen
>>>> pcms->vmport is true
>>>> -M pc -machine vmport=off,accel=xen
>>>> pcms->vmport is false
>>>> -M xenfv -machine accel=xen
>>>> pcms->vmport is false
>>>> -M xenfv -machine vmport=on,accel=xen
>>>> pcms->vmport is true
>>>> -M xenfv -machine vmport=off,accel=xen
>>>> pcms->vmport is false
>>>>
>>>>
>>>> Which look like to me to solve down to xen_init() called via
>>>> xen_accel_class_init()
>>>> need to see if vmport has been specified and change it if needed.
>>>>
>>>> Which leads me to:
>>>>
>>> [...]
>>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>>> index 7c3731f..5782406 100644
>>>> --- a/include/hw/i386/pc.h
>>>> +++ b/include/hw/i386/pc.h
>>>> @@ -38,6 +38,7 @@ struct PCMachineState {
>>>>
>>>> uint64_t max_ram_below_4g;
>>>> bool vmport;
>>>> + bool vmport_changed;
>>> So, now the setting have three possible states: unset, on, and off.
>>>
>>> If we can't avoid that (which seems to be the case, as "accel" is
>>> changed after we already set the default value for "vmport" in instance
>>> init), maybe we should make it an enum property that accepts on/off/auto
>>> as values, instead of faking a boolean property that is not really a
>>> boolean?
>>>
>> I would be happy to look into this. I have not done anything that used
>> an enum property, and so it is hard to guess on how long it will take to
>> convert the change to this.
> I think your patch may be a good intermediate solution, as long as you
> add a comment noting that the return value of the "vmport" property may
> be invalid before machine->init() is called, depending on the
> machine-type.
>
> Or, maybe a property without a getter would be appropriate?
>
Timing is everything. The convert to enum was not too hard. v3 posted.
-Don Slutz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-19 19:08 ` Don Slutz
2014-11-19 19:15 ` Don Slutz
2014-11-19 19:30 ` Eduardo Habkost
@ 2014-11-20 6:02 ` Paolo Bonzini
2014-11-20 15:09 ` Don Slutz
2014-11-20 15:13 ` Eduardo Habkost
3 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2014-11-20 6:02 UTC (permalink / raw)
To: Don Slutz, qemu-devel, Eduardo Habkost
On 19/11/2014 20:08, Don Slutz wrote:
>
> -M pc -machine accel=xen
> pcms->vmport is false
I think this should be true. Any reason why not?
Paolo
> -M pc -machine vmport=on,accel=xen
> pcms->vmport is true
> -M pc -machine vmport=off,accel=xen
> pcms->vmport is false
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-20 6:02 ` Paolo Bonzini
@ 2014-11-20 15:09 ` Don Slutz
0 siblings, 0 replies; 15+ messages in thread
From: Don Slutz @ 2014-11-20 15:09 UTC (permalink / raw)
To: Paolo Bonzini, Don Slutz, qemu-devel, Eduardo Habkost
On 11/20/14 01:02, Paolo Bonzini wrote:
>
> On 19/11/2014 20:08, Don Slutz wrote:
>> -M pc -machine accel=xen
>> pcms->vmport is false
> I think this should be true. Any reason why not?
>
> Paolo
Yes, QEMU will crash if xen is enabled and the guest tries to access the
VMware port.
(more on different thread).
-Don Slutz
>> -M pc -machine vmport=on,accel=xen
>> pcms->vmport is true
>> -M pc -machine vmport=off,accel=xen
>> pcms->vmport is false
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-19 19:08 ` Don Slutz
` (2 preceding siblings ...)
2014-11-20 6:02 ` Paolo Bonzini
@ 2014-11-20 15:13 ` Eduardo Habkost
2014-11-20 15:24 ` Don Slutz
3 siblings, 1 reply; 15+ messages in thread
From: Eduardo Habkost @ 2014-11-20 15:13 UTC (permalink / raw)
To: Don Slutz
Cc: Anthony Liguori, Michael S. Tsirkin, Michael Tokarev, qemu-devel,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini
On Wed, Nov 19, 2014 at 02:08:08PM -0500, Don Slutz wrote:
>
> On 11/19/14 13:08, Paolo Bonzini wrote:
> >
> >On 19/11/2014 19:07, Don Slutz wrote:
> >>>"-M pc -machine accel=xen" should work and, if that's what you want,
> >>>disable the vmport device. I think this patch is wrong.
> >>>
> >>>Paolo
> >>Well, I also want "-M pc -machine accel=xen,vmport=on" to work.
> >Right. So let's start by deciding what the desired semantics are for
> >all six cases: -M pc/xenfv, -machine vmport=on/off/absent.
> >
> >Paolo
>
> I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)):
We have more cases, if we consider "-M pc-2.1" too.
With this first patch (the one changing default_machine_opts), I expect
to get the following results:
-M pc
pcms->vmport is true
-M pc -machine vmport=on
pcms->vmport is true
-M pc -machine vmport=off
pcms->vmport is false
-M pc-2.1
pcms->vmport is true
-M pc-2.1 -machine vmport=on
pcms->vmport is true
(but it doesn't matter, QEMU 2.1 didn't have the vmport option)
-M pc-2.1 -machine vmport=off
pcms->vmport is false
(but it doesn't matter, QEMU 2.1 didn't have the vmport option)
-M xenfv
pcms->vmport is false
-M xenfv -machine vmport=on
pcms->vmport is true
-M xenfv -machine vmport=off
pcms->vmport is false
-M pc -machine accel=xen
pcms->vmport is true
-M pc -machine vmport=on,accel=xen
pcms->vmport is true
-M pc -machine vmport=off,accel=xen
pcms->vmport is false
-M pc-2.1 -machine accel=xen
pcms->vmport is true **
-M pc-2.1 -machine vmport=on,accel=xen
pcms->vmport is true
(but it doesn't matter, QEMU 2.1 didn't have the vmport option)
-M pc-2.1 -machine vmport=off,accel=xen
pcms->vmport is false
(but it doesn't matter, QEMU 2.1 didn't have the vmport option)
-M xenfv -machine accel=xen
pcms->vmport is false
-M xenfv -machine vmport=on,accel=xen
pcms->vmport is true
-M xenfv -machine vmport=off,accel=xen
pcms->vmport is false
I believe there's no consensus yet about the one marked with "**" above. It
boils to the question: do we need to keep guest ABI stability when using
"-M pc-2.1 -machine accel=xen"?
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-20 15:13 ` Eduardo Habkost
@ 2014-11-20 15:24 ` Don Slutz
2014-11-20 15:27 ` Eduardo Habkost
0 siblings, 1 reply; 15+ messages in thread
From: Don Slutz @ 2014-11-20 15:24 UTC (permalink / raw)
To: Eduardo Habkost, Don Slutz
Cc: Anthony Liguori, Michael S. Tsirkin, Michael Tokarev, qemu-devel,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini
On 11/20/14 10:13, Eduardo Habkost wrote:
> On Wed, Nov 19, 2014 at 02:08:08PM -0500, Don Slutz wrote:
>> On 11/19/14 13:08, Paolo Bonzini wrote:
>>> On 19/11/2014 19:07, Don Slutz wrote:
>>>>> "-M pc -machine accel=xen" should work and, if that's what you want,
>>>>> disable the vmport device. I think this patch is wrong.
>>>>>
>>>>> Paolo
>>>> Well, I also want "-M pc -machine accel=xen,vmport=on" to work.
>>> Right. So let's start by deciding what the desired semantics are for
>>> all six cases: -M pc/xenfv, -machine vmport=on/off/absent.
>>>
>>> Paolo
>> I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)):
> We have more cases, if we consider "-M pc-2.1" too.
>
> With this first patch (the one changing default_machine_opts), I expect
> to get the following results:
>
> -M pc
> pcms->vmport is true
> -M pc -machine vmport=on
> pcms->vmport is true
> -M pc -machine vmport=off
> pcms->vmport is false
> -M pc-2.1
> pcms->vmport is true
> -M pc-2.1 -machine vmport=on
> pcms->vmport is true
> (but it doesn't matter, QEMU 2.1 didn't have the vmport option)
> -M pc-2.1 -machine vmport=off
> pcms->vmport is false
> (but it doesn't matter, QEMU 2.1 didn't have the vmport option)
> -M xenfv
> pcms->vmport is false
> -M xenfv -machine vmport=on
> pcms->vmport is true
> -M xenfv -machine vmport=off
> pcms->vmport is false
>
> -M pc -machine accel=xen
> pcms->vmport is true
> -M pc -machine vmport=on,accel=xen
> pcms->vmport is true
> -M pc -machine vmport=off,accel=xen
> pcms->vmport is false
> -M pc-2.1 -machine accel=xen
> pcms->vmport is true **
> -M pc-2.1 -machine vmport=on,accel=xen
> pcms->vmport is true
> (but it doesn't matter, QEMU 2.1 didn't have the vmport option)
> -M pc-2.1 -machine vmport=off,accel=xen
> pcms->vmport is false
> (but it doesn't matter, QEMU 2.1 didn't have the vmport option)
> -M xenfv -machine accel=xen
> pcms->vmport is false
> -M xenfv -machine vmport=on,accel=xen
> pcms->vmport is true
> -M xenfv -machine vmport=off,accel=xen
> pcms->vmport is false
>
> I believe there's no consensus yet about the one marked with "**" above. It
> boils to the question: do we need to keep guest ABI stability when using
> "-M pc-2.1 -machine accel=xen"?
>
My answer is that for "**" vmport needs to be off. It looks like my
response:
> Subject: Re: [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass
vmport=off for xenfv machine
> Date: Thu, 20 Nov 2014 10:09:31 -0500
> From: Don Slutz <dslutz@terremark.com>
> To: Paolo Bonzini <pbonzini@redhat.com>, Don Slutz
<dslutz@verizon.com>, qemu-devel <qemu-devel@nongnu.org>, Eduardo
Habkost <ehabkost@redhat.com>
>
> On 11/20/14 01:02, Paolo Bonzini wrote:
> >
> > On 19/11/2014 20:08, Don Slutz wrote:
> >> -M pc -machine accel=xen
> >> pcms->vmport is false
> > I think this should be true. Any reason why not?
> >
> > Paolo
>
> Yes, QEMU will crash if xen is enabled and the guest tries to access the
> VMware port.
>
> (more on different thread).
>
> -Don Slutz
Was not seen before this.
-Don Slutz
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine
2014-11-20 15:24 ` Don Slutz
@ 2014-11-20 15:27 ` Eduardo Habkost
0 siblings, 0 replies; 15+ messages in thread
From: Eduardo Habkost @ 2014-11-20 15:27 UTC (permalink / raw)
To: Don Slutz
Cc: Anthony Liguori, Michael S. Tsirkin, Michael Tokarev, qemu-devel,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini
On Thu, Nov 20, 2014 at 10:24:15AM -0500, Don Slutz wrote:
[...]
> >-M pc-2.1 -machine accel=xen
> > pcms->vmport is true **
[...]
> >
> >I believe there's no consensus yet about the one marked with "**" above. It
> >boils to the question: do we need to keep guest ABI stability when using
> >"-M pc-2.1 -machine accel=xen"?
> >
>
> My answer is that for "**" vmport needs to be off. It looks like my
> response:
>
[...]
> > Yes, QEMU will crash if xen is enabled and the guest tries to access the
> > VMware port.
[...]
>
> Was not seen before this.
Yes, your answer arrived seconds after I sent my message. Ignore what I
said above. :)
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread