diff mbox series

[RFC,2/9] replace machine phase_check with machine_is_initialized/ready calls

Message ID 20210513082549.114275-3-mirela.grujic@greensocs.com
State New
Headers show
Series Initial support for machine creation via QMP | expand

Commit Message

Mirela Grujic May 13, 2021, 8:25 a.m. UTC
Once we define MachineInitPhase in qapi, the generated enumeration
constants will be prefixed with the MACHINE_INIT_PHASE_.
We need to define the MachineInitPhase enum in qapi in order to
add the QMP command that will query current machine init phase.

Since in the existing definition of enum MachineInitPhase the
enumeration constants are prefixed with PHASE_, there will be a
massive find/replace to rename the existing enum constants.
We'll do this in 2 phases:
1) hide explicit use of PHASE_ prefixed enums by replacing
    phase_check(PHASE_MACHINE_INITIALIZED) -> machine_is_initialized()
    phase_check(PHASE_MACHINE_READY) -> machine_is_ready()
2) rename enums

Once 1) and 2) are done MachineInitPhase enum will be generated.

Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>
---
 include/hw/qdev-core.h     |  2 ++
 hw/core/machine-qmp-cmds.c |  2 +-
 hw/core/machine.c          |  2 +-
 hw/core/qdev.c             | 12 +++++++++++-
 hw/pci/pci.c               |  2 +-
 hw/usb/core.c              |  2 +-
 hw/virtio/virtio-iommu.c   |  2 +-
 monitor/hmp.c              |  2 +-
 softmmu/qdev-monitor.c     |  9 ++++-----
 softmmu/vl.c               |  2 +-
 ui/console.c               |  2 +-
 11 files changed, 25 insertions(+), 14 deletions(-)

Comments

Paolo Bonzini May 13, 2021, 5:46 p.m. UTC | #1
On 13/05/21 10:25, Mirela Grujic wrote:
> Once we define MachineInitPhase in qapi, the generated enumeration
> constants will be prefixed with the MACHINE_INIT_PHASE_.
> We need to define the MachineInitPhase enum in qapi in order to
> add the QMP command that will query current machine init phase.
> 
> Since in the existing definition of enum MachineInitPhase the
> enumeration constants are prefixed with PHASE_, there will be a
> massive find/replace to rename the existing enum constants.
> We'll do this in 2 phases:
> 1) hide explicit use of PHASE_ prefixed enums by replacing
>      phase_check(PHASE_MACHINE_INITIALIZED) -> machine_is_initialized()
>      phase_check(PHASE_MACHINE_READY) -> machine_is_ready()
> 2) rename enums
> 
> Once 1) and 2) are done MachineInitPhase enum will be generated.

Is it so much churn to just rename everything to MACHINE_INIT_PHASE_* 
and keep phase_check() as is?  Or is it because the QAPI-generated names 
are quite long?

Paolo

> Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>
> ---
>   include/hw/qdev-core.h     |  2 ++
>   hw/core/machine-qmp-cmds.c |  2 +-
>   hw/core/machine.c          |  2 +-
>   hw/core/qdev.c             | 12 +++++++++++-
>   hw/pci/pci.c               |  2 +-
>   hw/usb/core.c              |  2 +-
>   hw/virtio/virtio-iommu.c   |  2 +-
>   monitor/hmp.c              |  2 +-
>   softmmu/qdev-monitor.c     |  9 ++++-----
>   softmmu/vl.c               |  2 +-
>   ui/console.c               |  2 +-
>   11 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 6e52240d92..5e3c6d4482 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -842,5 +842,7 @@ typedef enum MachineInitPhase {
>   extern bool phase_check(MachineInitPhase phase);
>   extern void phase_advance(MachineInitPhase phase);
>   extern MachineInitPhase phase_get(void);
> +extern bool machine_is_initialized(void);
> +extern bool machine_is_ready(void);
>   
>   #endif
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 68a942595a..be286882cc 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> @@ -149,7 +149,7 @@ HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
>   
>   void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
>   {
> -    if (phase_check(PHASE_MACHINE_INITIALIZED)) {
> +    if (machine_is_initialized()) {
>           error_setg(errp, "The command is permitted only before the machine has been created");
>           return;
>       }
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 40def78183..eba046924d 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1239,7 +1239,7 @@ static NotifierList machine_init_done_notifiers =
>   void qemu_add_machine_init_done_notifier(Notifier *notify)
>   {
>       notifier_list_add(&machine_init_done_notifiers, notify);
> -    if (phase_check(PHASE_MACHINE_READY)) {
> +    if (machine_is_ready()) {
>           notify->notify(notify, NULL);
>       }
>   }
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4a4a4d8c52..71906170f9 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -904,7 +904,7 @@ static void device_initfn(Object *obj)
>   {
>       DeviceState *dev = DEVICE(obj);
>   
> -    if (phase_check(PHASE_MACHINE_READY)) {
> +    if (machine_is_ready()) {
>           dev->hotplugged = 1;
>           qdev_hot_added = true;
>       }
> @@ -1155,6 +1155,16 @@ MachineInitPhase phase_get(void)
>       return machine_phase;
>   }
>   
> +bool machine_is_initialized(void)
> +{
> +    return machine_phase >= PHASE_MACHINE_INITIALIZED;
> +}
> +
> +bool machine_is_ready(void)
> +{
> +    return machine_phase >= PHASE_MACHINE_READY;
> +}
> +
>   static const TypeInfo device_type_info = {
>       .name = TYPE_DEVICE,
>       .parent = TYPE_OBJECT,
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 8f35e13a0c..19b584c3d1 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1071,7 +1071,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
>       address_space_init(&pci_dev->bus_master_as,
>                          &pci_dev->bus_master_container_region, pci_dev->name);
>   
> -    if (phase_check(PHASE_MACHINE_READY)) {
> +    if (machine_is_ready()) {
>           pci_init_bus_master(pci_dev);
>       }
>       pci_dev->irq_state = 0;
> diff --git a/hw/usb/core.c b/hw/usb/core.c
> index 975f76250a..2ec0dea6a0 100644
> --- a/hw/usb/core.c
> +++ b/hw/usb/core.c
> @@ -97,7 +97,7 @@ void usb_wakeup(USBEndpoint *ep, unsigned int stream)
>       USBDevice *dev = ep->dev;
>       USBBus *bus = usb_bus_from_device(dev);
>   
> -    if (!phase_check(PHASE_MACHINE_READY)) {
> +    if (!machine_is_ready()) {
>           /*
>            * This is machine init cold plug.  No need to wakeup anyone,
>            * all devices will be reset anyway.  And trying to wakeup can
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1b23e8e18c..8b1bcb2848 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -948,7 +948,7 @@ static int virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
>        * accept it. Having a different masks is possible but the guest will use
>        * sub-optimal block sizes, so warn about it.
>        */
> -    if (phase_check(PHASE_MACHINE_READY)) {
> +    if (machine_is_ready()) {
>           int new_granule = ctz64(new_mask);
>           int cur_granule = ctz64(cur_mask);
>   
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 6c0b33a0b1..c24511db6d 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -216,7 +216,7 @@ static bool cmd_can_preconfig(const HMPCommand *cmd)
>   
>   static bool cmd_available(const HMPCommand *cmd)
>   {
> -    return phase_check(PHASE_MACHINE_READY) || cmd_can_preconfig(cmd);
> +    return machine_is_ready() || cmd_can_preconfig(cmd);
>   }
>   
>   static void help_cmd_dump_one(Monitor *mon,
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index a9955b97a0..be8a892517 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -254,7 +254,7 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
>   
>       dc = DEVICE_CLASS(oc);
>       if (!dc->user_creatable ||
> -        (phase_check(PHASE_MACHINE_READY) && !dc->hotpluggable)) {
> +        (machine_is_ready() && !dc->hotpluggable)) {
>           error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
>                      "a pluggable device type");
>           return NULL;
> @@ -636,7 +636,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
>           }
>       }
>   
> -    if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {
> +    if (machine_is_ready() && bus && !qbus_is_hotpluggable(bus)) {
>           error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
>           return NULL;
>       }
> @@ -650,7 +650,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
>       dev = qdev_new(driver);
>   
>       /* Check whether the hotplug is allowed by the machine */
> -    if (phase_check(PHASE_MACHINE_READY)) {
> +    if (machine_is_ready()) {
>           if (!qdev_hotplug_allowed(dev, errp)) {
>               goto err_del_dev;
>           }
> @@ -998,8 +998,7 @@ int qemu_global_option(const char *str)
>   
>   bool qmp_command_available(const QmpCommand *cmd, Error **errp)
>   {
> -    if (!phase_check(PHASE_MACHINE_READY) &&
> -        !(cmd->options & QCO_ALLOW_PRECONFIG)) {
> +    if (!machine_is_ready() && !(cmd->options & QCO_ALLOW_PRECONFIG)) {
>           error_setg(errp, "The command '%s' is permitted only after machine initialization has completed",
>                      cmd->name);
>           return false;
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index cbf62abeb4..3af9743ceb 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2636,7 +2636,7 @@ static void qemu_machine_enter_phase(MachineInitPhase target_phase,
>   
>   void qmp_x_exit_preconfig(Error **errp)
>   {
> -    if (phase_check(PHASE_MACHINE_INITIALIZED)) {
> +    if (machine_is_initialized()) {
>           error_setg(errp, "The command is permitted only before machine initialization");
>           return;
>       }
> diff --git a/ui/console.c b/ui/console.c
> index 2de5f4105b..3513da6a54 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1353,7 +1353,7 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type,
>       if (QTAILQ_EMPTY(&consoles)) {
>           s->index = 0;
>           QTAILQ_INSERT_TAIL(&consoles, s, next);
> -    } else if (console_type != GRAPHIC_CONSOLE || phase_check(PHASE_MACHINE_READY)) {
> +    } else if (console_type != GRAPHIC_CONSOLE || machine_is_ready()) {
>           QemuConsole *last = QTAILQ_LAST(&consoles);
>           s->index = last->index + 1;
>           QTAILQ_INSERT_TAIL(&consoles, s, next);
>
Mirela Grujic May 14, 2021, 1:13 p.m. UTC | #2
On 5/13/21 7:46 PM, Paolo Bonzini wrote:
> On 13/05/21 10:25, Mirela Grujic wrote:
>> Once we define MachineInitPhase in qapi, the generated enumeration
>> constants will be prefixed with the MACHINE_INIT_PHASE_.
>> We need to define the MachineInitPhase enum in qapi in order to
>> add the QMP command that will query current machine init phase.
>>
>> Since in the existing definition of enum MachineInitPhase the
>> enumeration constants are prefixed with PHASE_, there will be a
>> massive find/replace to rename the existing enum constants.
>> We'll do this in 2 phases:
>> 1) hide explicit use of PHASE_ prefixed enums by replacing
>>      phase_check(PHASE_MACHINE_INITIALIZED) -> machine_is_initialized()
>>      phase_check(PHASE_MACHINE_READY) -> machine_is_ready()
>> 2) rename enums
>>
>> Once 1) and 2) are done MachineInitPhase enum will be generated.
>
> Is it so much churn to just rename everything to MACHINE_INIT_PHASE_* 
> and keep phase_check() as is?  Or is it because the QAPI-generated 
> names are quite long?
>

It's easier to just rename everything to MACHINE_INIT_PHASE_* :) I 
believed it was better to first hide enums, and thus avoid the need for 
such a massive find&replace in future. The names were also quite long, 
required additional line breaks...

If this is too much churn I can squash commits. When squashed, I don't 
see the big difference between just renaming and this approach, pretty 
much the same lines are modified. I believed it was easier to review 
without squash.


However, if you believe it should rather be just renamed I can do so.


> Paolo
>
>> Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>
>> ---
>>   include/hw/qdev-core.h     |  2 ++
>>   hw/core/machine-qmp-cmds.c |  2 +-
>>   hw/core/machine.c          |  2 +-
>>   hw/core/qdev.c             | 12 +++++++++++-
>>   hw/pci/pci.c               |  2 +-
>>   hw/usb/core.c              |  2 +-
>>   hw/virtio/virtio-iommu.c   |  2 +-
>>   monitor/hmp.c              |  2 +-
>>   softmmu/qdev-monitor.c     |  9 ++++-----
>>   softmmu/vl.c               |  2 +-
>>   ui/console.c               |  2 +-
>>   11 files changed, 25 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index 6e52240d92..5e3c6d4482 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -842,5 +842,7 @@ typedef enum MachineInitPhase {
>>   extern bool phase_check(MachineInitPhase phase);
>>   extern void phase_advance(MachineInitPhase phase);
>>   extern MachineInitPhase phase_get(void);
>> +extern bool machine_is_initialized(void);
>> +extern bool machine_is_ready(void);
>>     #endif
>> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
>> index 68a942595a..be286882cc 100644
>> --- a/hw/core/machine-qmp-cmds.c
>> +++ b/hw/core/machine-qmp-cmds.c
>> @@ -149,7 +149,7 @@ HotpluggableCPUList 
>> *qmp_query_hotpluggable_cpus(Error **errp)
>>     void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
>>   {
>> -    if (phase_check(PHASE_MACHINE_INITIALIZED)) {
>> +    if (machine_is_initialized()) {
>>           error_setg(errp, "The command is permitted only before the 
>> machine has been created");
>>           return;
>>       }
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 40def78183..eba046924d 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1239,7 +1239,7 @@ static NotifierList machine_init_done_notifiers =
>>   void qemu_add_machine_init_done_notifier(Notifier *notify)
>>   {
>>       notifier_list_add(&machine_init_done_notifiers, notify);
>> -    if (phase_check(PHASE_MACHINE_READY)) {
>> +    if (machine_is_ready()) {
>>           notify->notify(notify, NULL);
>>       }
>>   }
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 4a4a4d8c52..71906170f9 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -904,7 +904,7 @@ static void device_initfn(Object *obj)
>>   {
>>       DeviceState *dev = DEVICE(obj);
>>   -    if (phase_check(PHASE_MACHINE_READY)) {
>> +    if (machine_is_ready()) {
>>           dev->hotplugged = 1;
>>           qdev_hot_added = true;
>>       }
>> @@ -1155,6 +1155,16 @@ MachineInitPhase phase_get(void)
>>       return machine_phase;
>>   }
>>   +bool machine_is_initialized(void)
>> +{
>> +    return machine_phase >= PHASE_MACHINE_INITIALIZED;
>> +}
>> +
>> +bool machine_is_ready(void)
>> +{
>> +    return machine_phase >= PHASE_MACHINE_READY;
>> +}
>> +
>>   static const TypeInfo device_type_info = {
>>       .name = TYPE_DEVICE,
>>       .parent = TYPE_OBJECT,
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index 8f35e13a0c..19b584c3d1 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -1071,7 +1071,7 @@ static PCIDevice 
>> *do_pci_register_device(PCIDevice *pci_dev,
>>       address_space_init(&pci_dev->bus_master_as,
>> &pci_dev->bus_master_container_region, pci_dev->name);
>>   -    if (phase_check(PHASE_MACHINE_READY)) {
>> +    if (machine_is_ready()) {
>>           pci_init_bus_master(pci_dev);
>>       }
>>       pci_dev->irq_state = 0;
>> diff --git a/hw/usb/core.c b/hw/usb/core.c
>> index 975f76250a..2ec0dea6a0 100644
>> --- a/hw/usb/core.c
>> +++ b/hw/usb/core.c
>> @@ -97,7 +97,7 @@ void usb_wakeup(USBEndpoint *ep, unsigned int stream)
>>       USBDevice *dev = ep->dev;
>>       USBBus *bus = usb_bus_from_device(dev);
>>   -    if (!phase_check(PHASE_MACHINE_READY)) {
>> +    if (!machine_is_ready()) {
>>           /*
>>            * This is machine init cold plug.  No need to wakeup anyone,
>>            * all devices will be reset anyway.  And trying to wakeup can
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index 1b23e8e18c..8b1bcb2848 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -948,7 +948,7 @@ static int 
>> virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
>>        * accept it. Having a different masks is possible but the 
>> guest will use
>>        * sub-optimal block sizes, so warn about it.
>>        */
>> -    if (phase_check(PHASE_MACHINE_READY)) {
>> +    if (machine_is_ready()) {
>>           int new_granule = ctz64(new_mask);
>>           int cur_granule = ctz64(cur_mask);
>>   diff --git a/monitor/hmp.c b/monitor/hmp.c
>> index 6c0b33a0b1..c24511db6d 100644
>> --- a/monitor/hmp.c
>> +++ b/monitor/hmp.c
>> @@ -216,7 +216,7 @@ static bool cmd_can_preconfig(const HMPCommand *cmd)
>>     static bool cmd_available(const HMPCommand *cmd)
>>   {
>> -    return phase_check(PHASE_MACHINE_READY) || cmd_can_preconfig(cmd);
>> +    return machine_is_ready() || cmd_can_preconfig(cmd);
>>   }
>>     static void help_cmd_dump_one(Monitor *mon,
>> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
>> index a9955b97a0..be8a892517 100644
>> --- a/softmmu/qdev-monitor.c
>> +++ b/softmmu/qdev-monitor.c
>> @@ -254,7 +254,7 @@ static DeviceClass *qdev_get_device_class(const 
>> char **driver, Error **errp)
>>         dc = DEVICE_CLASS(oc);
>>       if (!dc->user_creatable ||
>> -        (phase_check(PHASE_MACHINE_READY) && !dc->hotpluggable)) {
>> +        (machine_is_ready() && !dc->hotpluggable)) {
>>           error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
>>                      "a pluggable device type");
>>           return NULL;
>> @@ -636,7 +636,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, 
>> Error **errp)
>>           }
>>       }
>>   -    if (phase_check(PHASE_MACHINE_READY) && bus && 
>> !qbus_is_hotpluggable(bus)) {
>> +    if (machine_is_ready() && bus && !qbus_is_hotpluggable(bus)) {
>>           error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
>>           return NULL;
>>       }
>> @@ -650,7 +650,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, 
>> Error **errp)
>>       dev = qdev_new(driver);
>>         /* Check whether the hotplug is allowed by the machine */
>> -    if (phase_check(PHASE_MACHINE_READY)) {
>> +    if (machine_is_ready()) {
>>           if (!qdev_hotplug_allowed(dev, errp)) {
>>               goto err_del_dev;
>>           }
>> @@ -998,8 +998,7 @@ int qemu_global_option(const char *str)
>>     bool qmp_command_available(const QmpCommand *cmd, Error **errp)
>>   {
>> -    if (!phase_check(PHASE_MACHINE_READY) &&
>> -        !(cmd->options & QCO_ALLOW_PRECONFIG)) {
>> +    if (!machine_is_ready() && !(cmd->options & QCO_ALLOW_PRECONFIG)) {
>>           error_setg(errp, "The command '%s' is permitted only after 
>> machine initialization has completed",
>>                      cmd->name);
>>           return false;
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index cbf62abeb4..3af9743ceb 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2636,7 +2636,7 @@ static void 
>> qemu_machine_enter_phase(MachineInitPhase target_phase,
>>     void qmp_x_exit_preconfig(Error **errp)
>>   {
>> -    if (phase_check(PHASE_MACHINE_INITIALIZED)) {
>> +    if (machine_is_initialized()) {
>>           error_setg(errp, "The command is permitted only before 
>> machine initialization");
>>           return;
>>       }
>> diff --git a/ui/console.c b/ui/console.c
>> index 2de5f4105b..3513da6a54 100644
>> --- a/ui/console.c
>> +++ b/ui/console.c
>> @@ -1353,7 +1353,7 @@ static QemuConsole *new_console(DisplayState 
>> *ds, console_type_t console_type,
>>       if (QTAILQ_EMPTY(&consoles)) {
>>           s->index = 0;
>>           QTAILQ_INSERT_TAIL(&consoles, s, next);
>> -    } else if (console_type != GRAPHIC_CONSOLE || 
>> phase_check(PHASE_MACHINE_READY)) {
>> +    } else if (console_type != GRAPHIC_CONSOLE || machine_is_ready()) {
>>           QemuConsole *last = QTAILQ_LAST(&consoles);
>>           s->index = last->index + 1;
>>           QTAILQ_INSERT_TAIL(&consoles, s, next);
>>
>
Paolo Bonzini May 14, 2021, 9:14 p.m. UTC | #3
On 14/05/21 15:13, Mirela Grujic wrote:
> However, if you believe it should rather be just renamed I can do so.

I am just not sure it's such an advantage to replace phase_check with 
separate functions.  The rename is a constraint of QAPI, so we have to 
live with the long names.

Paolo
Eric Blake June 7, 2021, 4:03 p.m. UTC | #4
On Fri, May 14, 2021 at 11:14:23PM +0200, Paolo Bonzini wrote:
> On 14/05/21 15:13, Mirela Grujic wrote:
> > However, if you believe it should rather be just renamed I can do so.
> 
> I am just not sure it's such an advantage to replace phase_check with
> separate functions.  The rename is a constraint of QAPI, so we have to live
> with the long names.

You can also use 'prefix':'...' as part of the QAPI 'enum' declaration
to pick a shorter name for the C enum (see for example BlkdebugEvent
in block-core.json).
diff mbox series

Patch

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 6e52240d92..5e3c6d4482 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -842,5 +842,7 @@  typedef enum MachineInitPhase {
 extern bool phase_check(MachineInitPhase phase);
 extern void phase_advance(MachineInitPhase phase);
 extern MachineInitPhase phase_get(void);
+extern bool machine_is_initialized(void);
+extern bool machine_is_ready(void);
 
 #endif
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 68a942595a..be286882cc 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -149,7 +149,7 @@  HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
 
 void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
 {
-    if (phase_check(PHASE_MACHINE_INITIALIZED)) {
+    if (machine_is_initialized()) {
         error_setg(errp, "The command is permitted only before the machine has been created");
         return;
     }
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 40def78183..eba046924d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1239,7 +1239,7 @@  static NotifierList machine_init_done_notifiers =
 void qemu_add_machine_init_done_notifier(Notifier *notify)
 {
     notifier_list_add(&machine_init_done_notifiers, notify);
-    if (phase_check(PHASE_MACHINE_READY)) {
+    if (machine_is_ready()) {
         notify->notify(notify, NULL);
     }
 }
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4a4a4d8c52..71906170f9 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -904,7 +904,7 @@  static void device_initfn(Object *obj)
 {
     DeviceState *dev = DEVICE(obj);
 
-    if (phase_check(PHASE_MACHINE_READY)) {
+    if (machine_is_ready()) {
         dev->hotplugged = 1;
         qdev_hot_added = true;
     }
@@ -1155,6 +1155,16 @@  MachineInitPhase phase_get(void)
     return machine_phase;
 }
 
+bool machine_is_initialized(void)
+{
+    return machine_phase >= PHASE_MACHINE_INITIALIZED;
+}
+
+bool machine_is_ready(void)
+{
+    return machine_phase >= PHASE_MACHINE_READY;
+}
+
 static const TypeInfo device_type_info = {
     .name = TYPE_DEVICE,
     .parent = TYPE_OBJECT,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8f35e13a0c..19b584c3d1 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1071,7 +1071,7 @@  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
     address_space_init(&pci_dev->bus_master_as,
                        &pci_dev->bus_master_container_region, pci_dev->name);
 
-    if (phase_check(PHASE_MACHINE_READY)) {
+    if (machine_is_ready()) {
         pci_init_bus_master(pci_dev);
     }
     pci_dev->irq_state = 0;
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 975f76250a..2ec0dea6a0 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -97,7 +97,7 @@  void usb_wakeup(USBEndpoint *ep, unsigned int stream)
     USBDevice *dev = ep->dev;
     USBBus *bus = usb_bus_from_device(dev);
 
-    if (!phase_check(PHASE_MACHINE_READY)) {
+    if (!machine_is_ready()) {
         /*
          * This is machine init cold plug.  No need to wakeup anyone,
          * all devices will be reset anyway.  And trying to wakeup can
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1b23e8e18c..8b1bcb2848 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -948,7 +948,7 @@  static int virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
      * accept it. Having a different masks is possible but the guest will use
      * sub-optimal block sizes, so warn about it.
      */
-    if (phase_check(PHASE_MACHINE_READY)) {
+    if (machine_is_ready()) {
         int new_granule = ctz64(new_mask);
         int cur_granule = ctz64(cur_mask);
 
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 6c0b33a0b1..c24511db6d 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -216,7 +216,7 @@  static bool cmd_can_preconfig(const HMPCommand *cmd)
 
 static bool cmd_available(const HMPCommand *cmd)
 {
-    return phase_check(PHASE_MACHINE_READY) || cmd_can_preconfig(cmd);
+    return machine_is_ready() || cmd_can_preconfig(cmd);
 }
 
 static void help_cmd_dump_one(Monitor *mon,
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index a9955b97a0..be8a892517 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -254,7 +254,7 @@  static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
 
     dc = DEVICE_CLASS(oc);
     if (!dc->user_creatable ||
-        (phase_check(PHASE_MACHINE_READY) && !dc->hotpluggable)) {
+        (machine_is_ready() && !dc->hotpluggable)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
                    "a pluggable device type");
         return NULL;
@@ -636,7 +636,7 @@  DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
         }
     }
 
-    if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {
+    if (machine_is_ready() && bus && !qbus_is_hotpluggable(bus)) {
         error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
         return NULL;
     }
@@ -650,7 +650,7 @@  DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
     dev = qdev_new(driver);
 
     /* Check whether the hotplug is allowed by the machine */
-    if (phase_check(PHASE_MACHINE_READY)) {
+    if (machine_is_ready()) {
         if (!qdev_hotplug_allowed(dev, errp)) {
             goto err_del_dev;
         }
@@ -998,8 +998,7 @@  int qemu_global_option(const char *str)
 
 bool qmp_command_available(const QmpCommand *cmd, Error **errp)
 {
-    if (!phase_check(PHASE_MACHINE_READY) &&
-        !(cmd->options & QCO_ALLOW_PRECONFIG)) {
+    if (!machine_is_ready() && !(cmd->options & QCO_ALLOW_PRECONFIG)) {
         error_setg(errp, "The command '%s' is permitted only after machine initialization has completed",
                    cmd->name);
         return false;
diff --git a/softmmu/vl.c b/softmmu/vl.c
index cbf62abeb4..3af9743ceb 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2636,7 +2636,7 @@  static void qemu_machine_enter_phase(MachineInitPhase target_phase,
 
 void qmp_x_exit_preconfig(Error **errp)
 {
-    if (phase_check(PHASE_MACHINE_INITIALIZED)) {
+    if (machine_is_initialized()) {
         error_setg(errp, "The command is permitted only before machine initialization");
         return;
     }
diff --git a/ui/console.c b/ui/console.c
index 2de5f4105b..3513da6a54 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1353,7 +1353,7 @@  static QemuConsole *new_console(DisplayState *ds, console_type_t console_type,
     if (QTAILQ_EMPTY(&consoles)) {
         s->index = 0;
         QTAILQ_INSERT_TAIL(&consoles, s, next);
-    } else if (console_type != GRAPHIC_CONSOLE || phase_check(PHASE_MACHINE_READY)) {
+    } else if (console_type != GRAPHIC_CONSOLE || machine_is_ready()) {
         QemuConsole *last = QTAILQ_LAST(&consoles);
         s->index = last->index + 1;
         QTAILQ_INSERT_TAIL(&consoles, s, next);