| Message ID | 20200207161948.15972-2-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make MachineClass::is_default boolean, refuse multiple default machines | expand |
Le 07/02/2020 à 17:19, Philippe Mathieu-Daudé a écrit : > The MachineClass is already zeroed on creation. > > Note: The code setting is_default=0 in hw/i386/pc_piix.c is > different (related to compat options). When adding a > new versioned machine, we want it to be the new default, > so we have to mark the previous one as not default. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > v3: new patch > --- > hw/lm32/lm32_boards.c | 1 - > hw/lm32/milkymist.c | 1 - > hw/m68k/q800.c | 1 - > hw/microblaze/petalogix_ml605_mmu.c | 1 - > hw/tricore/tricore_testboard.c | 1 - > 5 files changed, 5 deletions(-) > > diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c > index d1894adab8..156b050abc 100644 > --- a/hw/lm32/lm32_boards.c > +++ b/hw/lm32/lm32_boards.c > @@ -302,7 +302,6 @@ static void lm32_uclinux_class_init(ObjectClass *oc, void *data) > > mc->desc = "lm32 platform for uClinux and u-boot by Theobroma Systems"; > mc->init = lm32_uclinux_init; > - mc->is_default = 0; > mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full"); > } > > diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c > index 6d46134232..9fa5fd7050 100644 > --- a/hw/lm32/milkymist.c > +++ b/hw/lm32/milkymist.c > @@ -214,7 +214,6 @@ static void milkymist_machine_init(MachineClass *mc) > { > mc->desc = "Milkymist One"; > mc->init = milkymist_init; > - mc->is_default = 0; > mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full"); > } > > diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c > index 1e32363688..788b5d663f 100644 > --- a/hw/m68k/q800.c > +++ b/hw/m68k/q800.c > @@ -441,7 +441,6 @@ static void q800_machine_class_init(ObjectClass *oc, void *data) > mc->init = q800_init; > mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040"); > mc->max_cpus = 1; > - mc->is_default = 0; > mc->block_default_type = IF_SCSI; > } > > diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c > index 09486bc8bf..0a2640c40b 100644 > --- a/hw/microblaze/petalogix_ml605_mmu.c > +++ b/hw/microblaze/petalogix_ml605_mmu.c > @@ -216,7 +216,6 @@ static void petalogix_ml605_machine_init(MachineClass *mc) > { > mc->desc = "PetaLogix linux refdesign for xilinx ml605 little endian"; > mc->init = petalogix_ml605_init; > - mc->is_default = 0; > } > > DEFINE_MACHINE("petalogix-ml605", petalogix_ml605_machine_init) > diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c > index 20c9ccb3ce..8ec2b5bddd 100644 > --- a/hw/tricore/tricore_testboard.c > +++ b/hw/tricore/tricore_testboard.c > @@ -105,7 +105,6 @@ static void ttb_machine_init(MachineClass *mc) > { > mc->desc = "a minimal TriCore board"; > mc->init = tricoreboard_init; > - mc->is_default = 0; > mc->default_cpu_type = TRICORE_CPU_TYPE_NAME("tc1796"); > } > > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On Fri, Feb 07, 2020 at 05:19:46PM +0100, Philippe Mathieu-Daudé wrote: > The MachineClass is already zeroed on creation. > > Note: The code setting is_default=0 in hw/i386/pc_piix.c is > different (related to compat options). When adding a > new versioned machine, we want it to be the new default, > so we have to mark the previous one as not default. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > v3: new patch > --- > hw/lm32/lm32_boards.c | 1 - > hw/lm32/milkymist.c | 1 - > hw/m68k/q800.c | 1 - > hw/microblaze/petalogix_ml605_mmu.c | 1 - > hw/tricore/tricore_testboard.c | 1 - > 5 files changed, 5 deletions(-) > > diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c > index d1894adab8..156b050abc 100644 > --- a/hw/lm32/lm32_boards.c > +++ b/hw/lm32/lm32_boards.c > @@ -302,7 +302,6 @@ static void lm32_uclinux_class_init(ObjectClass *oc, void *data) > > mc->desc = "lm32 platform for uClinux and u-boot by Theobroma Systems"; > mc->init = lm32_uclinux_init; > - mc->is_default = 0; > mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full"); > } > > diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c > index 6d46134232..9fa5fd7050 100644 > --- a/hw/lm32/milkymist.c > +++ b/hw/lm32/milkymist.c > @@ -214,7 +214,6 @@ static void milkymist_machine_init(MachineClass *mc) > { > mc->desc = "Milkymist One"; > mc->init = milkymist_init; > - mc->is_default = 0; > mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full"); > } > > diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c > index 1e32363688..788b5d663f 100644 > --- a/hw/m68k/q800.c > +++ b/hw/m68k/q800.c > @@ -441,7 +441,6 @@ static void q800_machine_class_init(ObjectClass *oc, void *data) > mc->init = q800_init; > mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040"); > mc->max_cpus = 1; > - mc->is_default = 0; > mc->block_default_type = IF_SCSI; > } > > diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c > index 09486bc8bf..0a2640c40b 100644 > --- a/hw/microblaze/petalogix_ml605_mmu.c > +++ b/hw/microblaze/petalogix_ml605_mmu.c > @@ -216,7 +216,6 @@ static void petalogix_ml605_machine_init(MachineClass *mc) > { > mc->desc = "PetaLogix linux refdesign for xilinx ml605 little endian"; > mc->init = petalogix_ml605_init; > - mc->is_default = 0; > } > > DEFINE_MACHINE("petalogix-ml605", petalogix_ml605_machine_init) > diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c > index 20c9ccb3ce..8ec2b5bddd 100644 > --- a/hw/tricore/tricore_testboard.c > +++ b/hw/tricore/tricore_testboard.c > @@ -105,7 +105,6 @@ static void ttb_machine_init(MachineClass *mc) > { > mc->desc = "a minimal TriCore board"; > mc->init = tricoreboard_init; > - mc->is_default = 0; > mc->default_cpu_type = TRICORE_CPU_TYPE_NAME("tc1796"); > } >
On 07/02/2020 17.19, Philippe Mathieu-Daudé wrote: > The MachineClass is already zeroed on creation. > > Note: The code setting is_default=0 in hw/i386/pc_piix.c is > different (related to compat options). When adding a > new versioned machine, we want it to be the new default, > so we have to mark the previous one as not default. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > v3: new patch > --- > hw/lm32/lm32_boards.c | 1 - > hw/lm32/milkymist.c | 1 - > hw/m68k/q800.c | 1 - > hw/microblaze/petalogix_ml605_mmu.c | 1 - > hw/tricore/tricore_testboard.c | 1 - > 5 files changed, 5 deletions(-) [...] > diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c > index 20c9ccb3ce..8ec2b5bddd 100644 > --- a/hw/tricore/tricore_testboard.c > +++ b/hw/tricore/tricore_testboard.c > @@ -105,7 +105,6 @@ static void ttb_machine_init(MachineClass *mc) > { > mc->desc = "a minimal TriCore board"; > mc->init = tricoreboard_init; > - mc->is_default = 0; > mc->default_cpu_type = TRICORE_CPU_TYPE_NAME("tc1796"); > } I wonder whether we should simply make that machine the default for qemu-system-tricore? There is only one machine here, and not having a default machine always causes some headaches in the tests... (see e.g. tests/qemu-iotests/check for example) Thomas
On 2/10/20 10:35 AM, Thomas Huth wrote: > On 07/02/2020 17.19, Philippe Mathieu-Daudé wrote: >> The MachineClass is already zeroed on creation. >> >> Note: The code setting is_default=0 in hw/i386/pc_piix.c is >> different (related to compat options). When adding a >> new versioned machine, we want it to be the new default, >> so we have to mark the previous one as not default. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> v3: new patch >> --- >> hw/lm32/lm32_boards.c | 1 - >> hw/lm32/milkymist.c | 1 - >> hw/m68k/q800.c | 1 - >> hw/microblaze/petalogix_ml605_mmu.c | 1 - >> hw/tricore/tricore_testboard.c | 1 - >> 5 files changed, 5 deletions(-) > [...] >> diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c >> index 20c9ccb3ce..8ec2b5bddd 100644 >> --- a/hw/tricore/tricore_testboard.c >> +++ b/hw/tricore/tricore_testboard.c >> @@ -105,7 +105,6 @@ static void ttb_machine_init(MachineClass *mc) >> { >> mc->desc = "a minimal TriCore board"; >> mc->init = tricoreboard_init; >> - mc->is_default = 0; >> mc->default_cpu_type = TRICORE_CPU_TYPE_NAME("tc1796"); >> } > > I wonder whether we should simply make that machine the default for > qemu-system-tricore? There is only one machine here, and not having a > default machine always causes some headaches in the tests... > (see e.g. tests/qemu-iotests/check for example) Or make it generic? If a architecture has a single machine, use it by default?
On 10/02/2020 11.08, Philippe Mathieu-Daudé wrote: > On 2/10/20 10:35 AM, Thomas Huth wrote: >> On 07/02/2020 17.19, Philippe Mathieu-Daudé wrote: >>> The MachineClass is already zeroed on creation. >>> >>> Note: The code setting is_default=0 in hw/i386/pc_piix.c is >>> different (related to compat options). When adding a >>> new versioned machine, we want it to be the new default, >>> so we have to mark the previous one as not default. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> --- >>> v3: new patch >>> --- >>> hw/lm32/lm32_boards.c | 1 - >>> hw/lm32/milkymist.c | 1 - >>> hw/m68k/q800.c | 1 - >>> hw/microblaze/petalogix_ml605_mmu.c | 1 - >>> hw/tricore/tricore_testboard.c | 1 - >>> 5 files changed, 5 deletions(-) >> [...] >>> diff --git a/hw/tricore/tricore_testboard.c >>> b/hw/tricore/tricore_testboard.c >>> index 20c9ccb3ce..8ec2b5bddd 100644 >>> --- a/hw/tricore/tricore_testboard.c >>> +++ b/hw/tricore/tricore_testboard.c >>> @@ -105,7 +105,6 @@ static void ttb_machine_init(MachineClass *mc) >>> { >>> mc->desc = "a minimal TriCore board"; >>> mc->init = tricoreboard_init; >>> - mc->is_default = 0; >>> mc->default_cpu_type = TRICORE_CPU_TYPE_NAME("tc1796"); >>> } >> >> I wonder whether we should simply make that machine the default for >> qemu-system-tricore? There is only one machine here, and not having a >> default machine always causes some headaches in the tests... >> (see e.g. tests/qemu-iotests/check for example) > > Or make it generic? If a architecture has a single machine, use it by > default? Sounds like a good idea, too ... we've got a couple of targets that have only one machine. Thomas
On 2/10/20 11:26 AM, Thomas Huth wrote: > On 10/02/2020 11.08, Philippe Mathieu-Daudé wrote: >> On 2/10/20 10:35 AM, Thomas Huth wrote: >>> On 07/02/2020 17.19, Philippe Mathieu-Daudé wrote: >>>> The MachineClass is already zeroed on creation. >>>> >>>> Note: The code setting is_default=0 in hw/i386/pc_piix.c is >>>> different (related to compat options). When adding a >>>> new versioned machine, we want it to be the new default, >>>> so we have to mark the previous one as not default. >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>>> --- >>>> v3: new patch >>>> --- >>>> hw/lm32/lm32_boards.c | 1 - >>>> hw/lm32/milkymist.c | 1 - >>>> hw/m68k/q800.c | 1 - >>>> hw/microblaze/petalogix_ml605_mmu.c | 1 - >>>> hw/tricore/tricore_testboard.c | 1 - >>>> 5 files changed, 5 deletions(-) >>> [...] >>>> diff --git a/hw/tricore/tricore_testboard.c >>>> b/hw/tricore/tricore_testboard.c >>>> index 20c9ccb3ce..8ec2b5bddd 100644 >>>> --- a/hw/tricore/tricore_testboard.c >>>> +++ b/hw/tricore/tricore_testboard.c >>>> @@ -105,7 +105,6 @@ static void ttb_machine_init(MachineClass *mc) >>>> { >>>> mc->desc = "a minimal TriCore board"; >>>> mc->init = tricoreboard_init; >>>> - mc->is_default = 0; >>>> mc->default_cpu_type = TRICORE_CPU_TYPE_NAME("tc1796"); >>>> } >>> I wonder whether we should simply make that machine the default for >>> qemu-system-tricore? There is only one machine here, and not having a >>> default machine always causes some headaches in the tests... >>> (see e.g. tests/qemu-iotests/check for example) >> Or make it generic? If a architecture has a single machine, use it by >> default? > Sounds like a good idea, too ... we've got a couple of targets that have > only one machine. As far as I remember, I did not make it the default machine, since Peter Maydell advised against it. His argument was that defaults are really hard to get rid off since external tools (like libvirt) might rely on the defaults and we don't want to break those. Anyways, no objections from my side. Cheers, Bastian
On Mon, 10 Feb 2020 at 12:33, Bastian Koppelmann <kbastian@mail.uni-paderborn.de> wrote: > > > On 2/10/20 11:26 AM, Thomas Huth wrote: > > On 10/02/2020 11.08, Philippe Mathieu-Daudé wrote: > >> On 2/10/20 10:35 AM, Thomas Huth wrote: > >>> On 07/02/2020 17.19, Philippe Mathieu-Daudé wrote: > >>> I wonder whether we should simply make that machine the default for > >>> qemu-system-tricore? There is only one machine here, and not having a > >>> default machine always causes some headaches in the tests... > >>> (see e.g. tests/qemu-iotests/check for example) > >> Or make it generic? If a architecture has a single machine, use it by > >> default? > > Sounds like a good idea, too ... we've got a couple of targets that have > > only one machine. > As far as I remember, I did not make it the default machine, since Peter > Maydell advised against it. His argument was that defaults are really > hard to get rid off since external tools (like libvirt) might rely on > the defaults and we don't want to break those. Anyways, no objections > from my side. Yes; we have default machines partly for historical reasons and partly because x86 does, but unless there's a good reason for some architecture why this specific machine should be the default, I don't think we should have a default: making the user specify what they actually want helps to nudge them into thinking about what they do want, rather than assuming that QEMU will somehow magically be able to run guest images built for any random machine for the architecture. Anything in tests or whatever that breaks if there's no default machine for the architecture should be improved to handle that (it already needs to handle that case, though: arm does not have a defined default machine). thanks -- PMM
On 2/10/20 2:22 PM, Peter Maydell wrote: > On Mon, 10 Feb 2020 at 12:33, Bastian Koppelmann > <kbastian@mail.uni-paderborn.de> wrote: >> >> >> On 2/10/20 11:26 AM, Thomas Huth wrote: >>> On 10/02/2020 11.08, Philippe Mathieu-Daudé wrote: >>>> On 2/10/20 10:35 AM, Thomas Huth wrote: >>>>> On 07/02/2020 17.19, Philippe Mathieu-Daudé wrote: >>>>> I wonder whether we should simply make that machine the default for >>>>> qemu-system-tricore? There is only one machine here, and not having a >>>>> default machine always causes some headaches in the tests... >>>>> (see e.g. tests/qemu-iotests/check for example) >>>> Or make it generic? If a architecture has a single machine, use it by >>>> default? >>> Sounds like a good idea, too ... we've got a couple of targets that have >>> only one machine. > >> As far as I remember, I did not make it the default machine, since Peter >> Maydell advised against it. His argument was that defaults are really >> hard to get rid off since external tools (like libvirt) might rely on >> the defaults and we don't want to break those. Anyways, no objections >> from my side. > > Yes; we have default machines partly for historical reasons > and partly because x86 does, but unless there's a good > reason for some architecture why this specific machine > should be the default, I don't think we should have a default: > making the user specify what they actually want helps to nudge > them into thinking about what they do want, rather than > assuming that QEMU will somehow magically be able to run > guest images built for any random machine for the architecture. OK now it makes sense. > Anything in tests or whatever that breaks if there's no default > machine for the architecture should be improved to handle that > (it already needs to handle that case, though: arm does not > have a defined default machine). I tend to agree here.
diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c index d1894adab8..156b050abc 100644 --- a/hw/lm32/lm32_boards.c +++ b/hw/lm32/lm32_boards.c @@ -302,7 +302,6 @@ static void lm32_uclinux_class_init(ObjectClass *oc, void *data) mc->desc = "lm32 platform for uClinux and u-boot by Theobroma Systems"; mc->init = lm32_uclinux_init; - mc->is_default = 0; mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full"); } diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c index 6d46134232..9fa5fd7050 100644 --- a/hw/lm32/milkymist.c +++ b/hw/lm32/milkymist.c @@ -214,7 +214,6 @@ static void milkymist_machine_init(MachineClass *mc) { mc->desc = "Milkymist One"; mc->init = milkymist_init; - mc->is_default = 0; mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full"); } diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c index 1e32363688..788b5d663f 100644 --- a/hw/m68k/q800.c +++ b/hw/m68k/q800.c @@ -441,7 +441,6 @@ static void q800_machine_class_init(ObjectClass *oc, void *data) mc->init = q800_init; mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040"); mc->max_cpus = 1; - mc->is_default = 0; mc->block_default_type = IF_SCSI; } diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index 09486bc8bf..0a2640c40b 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -216,7 +216,6 @@ static void petalogix_ml605_machine_init(MachineClass *mc) { mc->desc = "PetaLogix linux refdesign for xilinx ml605 little endian"; mc->init = petalogix_ml605_init; - mc->is_default = 0; } DEFINE_MACHINE("petalogix-ml605", petalogix_ml605_machine_init) diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c index 20c9ccb3ce..8ec2b5bddd 100644 --- a/hw/tricore/tricore_testboard.c +++ b/hw/tricore/tricore_testboard.c @@ -105,7 +105,6 @@ static void ttb_machine_init(MachineClass *mc) { mc->desc = "a minimal TriCore board"; mc->init = tricoreboard_init; - mc->is_default = 0; mc->default_cpu_type = TRICORE_CPU_TYPE_NAME("tc1796"); }
The MachineClass is already zeroed on creation. Note: The code setting is_default=0 in hw/i386/pc_piix.c is different (related to compat options). When adding a new versioned machine, we want it to be the new default, so we have to mark the previous one as not default. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- v3: new patch --- hw/lm32/lm32_boards.c | 1 - hw/lm32/milkymist.c | 1 - hw/m68k/q800.c | 1 - hw/microblaze/petalogix_ml605_mmu.c | 1 - hw/tricore/tricore_testboard.c | 1 - 5 files changed, 5 deletions(-)