hw/scsi/vmw_pvscsi.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
rings_info_valid, msg_ring_info_valid and use_msg fields of struct
PVSCSIState are using gboolean TRUE/FALSE values with the type uint8_t.
Change their type to bool along with the usage of initialization macro
VMSTATE_BOOL during initialization of vmstate_pvscsi and
pvscsi_properties.
Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
---
hw/scsi/vmw_pvscsi.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index c071e0c7aa..86f00e3d7e 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -123,9 +123,9 @@ typedef struct {
/* Collector for current command data */
uint32_t curr_cmd_data[PVSCSI_MAX_CMD_DATA_WORDS];
- uint8_t rings_info_valid; /* Whether data rings initialized */
- uint8_t msg_ring_info_valid; /* Whether message ring initialized */
- uint8_t use_msg; /* Whether to use message ring */
+ bool rings_info_valid; /* Whether data rings initialized */
+ bool msg_ring_info_valid; /* Whether message ring initialized */
+ bool use_msg; /* Whether to use message ring */
uint8_t msi_used; /* For migration compatibility */
PVSCSIRingInfo rings; /* Data transfer rings manager */
@@ -349,8 +349,8 @@ pvscsi_reset_state(PVSCSIState *s)
s->reg_command_status = PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
s->reg_interrupt_status = 0;
pvscsi_ring_cleanup(&s->rings);
- s->rings_info_valid = FALSE;
- s->msg_ring_info_valid = FALSE;
+ s->rings_info_valid = false;
+ s->msg_ring_info_valid = false;
QTAILQ_INIT(&s->pending_queue);
QTAILQ_INIT(&s->completion_queue);
}
@@ -792,7 +792,7 @@ pvscsi_on_cmd_setup_rings(PVSCSIState *s)
pvscsi_dbg_dump_tx_rings_config(rc);
pvscsi_ring_init_data(&s->rings, rc);
- s->rings_info_valid = TRUE;
+ s->rings_info_valid = true;
return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
}
@@ -874,7 +874,7 @@ pvscsi_on_cmd_setup_msg_ring(PVSCSIState *s)
if (pvscsi_ring_init_msg(&s->rings, rc) < 0) {
return PVSCSI_COMMAND_PROCESSING_FAILED;
}
- s->msg_ring_info_valid = TRUE;
+ s->msg_ring_info_valid = true;
}
return sizeof(PVSCSICmdDescSetupMsgRing) / sizeof(uint32_t);
}
@@ -1232,9 +1232,9 @@ static const VMStateDescription vmstate_pvscsi = {
VMSTATE_UINT32(curr_cmd_data_cntr, PVSCSIState),
VMSTATE_UINT32_ARRAY(curr_cmd_data, PVSCSIState,
ARRAY_SIZE(((PVSCSIState *)NULL)->curr_cmd_data)),
- VMSTATE_UINT8(rings_info_valid, PVSCSIState),
- VMSTATE_UINT8(msg_ring_info_valid, PVSCSIState),
- VMSTATE_UINT8(use_msg, PVSCSIState),
+ VMSTATE_BOOL(rings_info_valid, PVSCSIState),
+ VMSTATE_BOOL(msg_ring_info_valid, PVSCSIState),
+ VMSTATE_BOOL(use_msg, PVSCSIState),
VMSTATE_UINT64(rings.rs_pa, PVSCSIState),
VMSTATE_UINT32(rings.txr_len_mask, PVSCSIState),
@@ -1255,7 +1255,7 @@ static const VMStateDescription vmstate_pvscsi = {
};
static Property pvscsi_properties[] = {
- DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
+ DEFINE_PROP_BOOL("use_msg", PVSCSIState, use_msg, true),
DEFINE_PROP_BIT("x-old-pci-configuration", PVSCSIState, compat_flags,
PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT, false),
DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
--
2.28.0
This is my first qemu patch. Let know if there are any mistakes
Hi Amey,
On 9/14/20 1:44 AM, Amey Narkhede wrote:
> rings_info_valid, msg_ring_info_valid and use_msg fields of struct
> PVSCSIState are using gboolean TRUE/FALSE values with the type uint8_t.
> Change their type to bool along with the usage of initialization macro
> VMSTATE_BOOL during initialization of vmstate_pvscsi and
> pvscsi_properties.
>
> Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> ---
> hw/scsi/vmw_pvscsi.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index c071e0c7aa..86f00e3d7e 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -123,9 +123,9 @@ typedef struct {
> /* Collector for current command data */
> uint32_t curr_cmd_data[PVSCSI_MAX_CMD_DATA_WORDS];
>
> - uint8_t rings_info_valid; /* Whether data rings initialized */
> - uint8_t msg_ring_info_valid; /* Whether message ring initialized */
> - uint8_t use_msg; /* Whether to use message ring */
> + bool rings_info_valid; /* Whether data rings initialized */
> + bool msg_ring_info_valid; /* Whether message ring initialized */
> + bool use_msg; /* Whether to use message ring */
>
> uint8_t msi_used; /* For migration compatibility */
> PVSCSIRingInfo rings; /* Data transfer rings manager */
> @@ -349,8 +349,8 @@ pvscsi_reset_state(PVSCSIState *s)
> s->reg_command_status = PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> s->reg_interrupt_status = 0;
> pvscsi_ring_cleanup(&s->rings);
> - s->rings_info_valid = FALSE;
> - s->msg_ring_info_valid = FALSE;
> + s->rings_info_valid = false;
> + s->msg_ring_info_valid = false;
> QTAILQ_INIT(&s->pending_queue);
> QTAILQ_INIT(&s->completion_queue);
> }
> @@ -792,7 +792,7 @@ pvscsi_on_cmd_setup_rings(PVSCSIState *s)
> pvscsi_dbg_dump_tx_rings_config(rc);
> pvscsi_ring_init_data(&s->rings, rc);
>
> - s->rings_info_valid = TRUE;
> + s->rings_info_valid = true;
> return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> }
>
> @@ -874,7 +874,7 @@ pvscsi_on_cmd_setup_msg_ring(PVSCSIState *s)
> if (pvscsi_ring_init_msg(&s->rings, rc) < 0) {
> return PVSCSI_COMMAND_PROCESSING_FAILED;
> }
> - s->msg_ring_info_valid = TRUE;
> + s->msg_ring_info_valid = true;
> }
> return sizeof(PVSCSICmdDescSetupMsgRing) / sizeof(uint32_t);
> }
> @@ -1232,9 +1232,9 @@ static const VMStateDescription vmstate_pvscsi = {
> VMSTATE_UINT32(curr_cmd_data_cntr, PVSCSIState),
> VMSTATE_UINT32_ARRAY(curr_cmd_data, PVSCSIState,
> ARRAY_SIZE(((PVSCSIState *)NULL)->curr_cmd_data)),
> - VMSTATE_UINT8(rings_info_valid, PVSCSIState),
> - VMSTATE_UINT8(msg_ring_info_valid, PVSCSIState),
> - VMSTATE_UINT8(use_msg, PVSCSIState),
> + VMSTATE_BOOL(rings_info_valid, PVSCSIState),
> + VMSTATE_BOOL(msg_ring_info_valid, PVSCSIState),
> + VMSTATE_BOOL(use_msg, PVSCSIState),
I believe this change the migration data structure. This
area is described in "Changing migration data structures"
in docs/devel/migration.rst.
If this structure were not affected, your change would be
a good cleanup. However changing migration can become a
nightmare, so ... cleaning this is hard.
Cc'ing Dave (a migration maintainer) as I'm not sure there
already is a document describing easily this problem.
>
> VMSTATE_UINT64(rings.rs_pa, PVSCSIState),
> VMSTATE_UINT32(rings.txr_len_mask, PVSCSIState),
> @@ -1255,7 +1255,7 @@ static const VMStateDescription vmstate_pvscsi = {
> };
>
> static Property pvscsi_properties[] = {
> - DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
> + DEFINE_PROP_BOOL("use_msg", PVSCSIState, use_msg, true),
> DEFINE_PROP_BIT("x-old-pci-configuration", PVSCSIState, compat_flags,
> PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT, false),
> DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
> --
> 2.28.0
>
> This is my first qemu patch. Let know if there are any mistakes
>
On 20/09/14 08:32AM, Philippe Mathieu-Daudé wrote:
> Hi Amey,
>
> On 9/14/20 1:44 AM, Amey Narkhede wrote:
> > rings_info_valid, msg_ring_info_valid and use_msg fields of struct
> > PVSCSIState are using gboolean TRUE/FALSE values with the type uint8_t.
> > Change their type to bool along with the usage of initialization macro
> > VMSTATE_BOOL during initialization of vmstate_pvscsi and
> > pvscsi_properties.
> >
> > Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> > ---
> > hw/scsi/vmw_pvscsi.c | 22 +++++++++++-----------
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > index c071e0c7aa..86f00e3d7e 100644
> > --- a/hw/scsi/vmw_pvscsi.c
> > +++ b/hw/scsi/vmw_pvscsi.c
> > @@ -123,9 +123,9 @@ typedef struct {
> > /* Collector for current command data */
> > uint32_t curr_cmd_data[PVSCSI_MAX_CMD_DATA_WORDS];
> >
> > - uint8_t rings_info_valid; /* Whether data rings initialized */
> > - uint8_t msg_ring_info_valid; /* Whether message ring initialized */
> > - uint8_t use_msg; /* Whether to use message ring */
> > + bool rings_info_valid; /* Whether data rings initialized */
> > + bool msg_ring_info_valid; /* Whether message ring initialized */
> > + bool use_msg; /* Whether to use message ring */
> >
> > uint8_t msi_used; /* For migration compatibility */
> > PVSCSIRingInfo rings; /* Data transfer rings manager */
> > @@ -349,8 +349,8 @@ pvscsi_reset_state(PVSCSIState *s)
> > s->reg_command_status = PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> > s->reg_interrupt_status = 0;
> > pvscsi_ring_cleanup(&s->rings);
> > - s->rings_info_valid = FALSE;
> > - s->msg_ring_info_valid = FALSE;
> > + s->rings_info_valid = false;
> > + s->msg_ring_info_valid = false;
> > QTAILQ_INIT(&s->pending_queue);
> > QTAILQ_INIT(&s->completion_queue);
> > }
> > @@ -792,7 +792,7 @@ pvscsi_on_cmd_setup_rings(PVSCSIState *s)
> > pvscsi_dbg_dump_tx_rings_config(rc);
> > pvscsi_ring_init_data(&s->rings, rc);
> >
> > - s->rings_info_valid = TRUE;
> > + s->rings_info_valid = true;
> > return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> > }
> >
> > @@ -874,7 +874,7 @@ pvscsi_on_cmd_setup_msg_ring(PVSCSIState *s)
> > if (pvscsi_ring_init_msg(&s->rings, rc) < 0) {
> > return PVSCSI_COMMAND_PROCESSING_FAILED;
> > }
> > - s->msg_ring_info_valid = TRUE;
> > + s->msg_ring_info_valid = true;
> > }
> > return sizeof(PVSCSICmdDescSetupMsgRing) / sizeof(uint32_t);
> > }
> > @@ -1232,9 +1232,9 @@ static const VMStateDescription vmstate_pvscsi = {
> > VMSTATE_UINT32(curr_cmd_data_cntr, PVSCSIState),
> > VMSTATE_UINT32_ARRAY(curr_cmd_data, PVSCSIState,
> > ARRAY_SIZE(((PVSCSIState *)NULL)->curr_cmd_data)),
> > - VMSTATE_UINT8(rings_info_valid, PVSCSIState),
> > - VMSTATE_UINT8(msg_ring_info_valid, PVSCSIState),
> > - VMSTATE_UINT8(use_msg, PVSCSIState),
> > + VMSTATE_BOOL(rings_info_valid, PVSCSIState),
> > + VMSTATE_BOOL(msg_ring_info_valid, PVSCSIState),
> > + VMSTATE_BOOL(use_msg, PVSCSIState),
>
> I believe this change the migration data structure. This
> area is described in "Changing migration data structures"
> in docs/devel/migration.rst.
>
> If this structure were not affected, your change would be
> a good cleanup. However changing migration can become a
> nightmare, so ... cleaning this is hard.
>
> Cc'ing Dave (a migration maintainer) as I'm not sure there
> already is a document describing easily this problem.
>
So should I drop those migration related changes?
> >
> > VMSTATE_UINT64(rings.rs_pa, PVSCSIState),
> > VMSTATE_UINT32(rings.txr_len_mask, PVSCSIState),
> > @@ -1255,7 +1255,7 @@ static const VMStateDescription vmstate_pvscsi = {
> > };
> >
> > static Property pvscsi_properties[] = {
> > - DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
> > + DEFINE_PROP_BOOL("use_msg", PVSCSIState, use_msg, true),
> > DEFINE_PROP_BIT("x-old-pci-configuration", PVSCSIState, compat_flags,
> > PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT, false),
> > DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
> > --
> > 2.28.0
> >
> > This is my first qemu patch. Let know if there are any mistakes
> >
>
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Hi Amey,
>
> On 9/14/20 1:44 AM, Amey Narkhede wrote:
> > rings_info_valid, msg_ring_info_valid and use_msg fields of struct
> > PVSCSIState are using gboolean TRUE/FALSE values with the type uint8_t.
> > Change their type to bool along with the usage of initialization macro
> > VMSTATE_BOOL during initialization of vmstate_pvscsi and
> > pvscsi_properties.
> >
> > Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> > ---
> > hw/scsi/vmw_pvscsi.c | 22 +++++++++++-----------
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > index c071e0c7aa..86f00e3d7e 100644
> > --- a/hw/scsi/vmw_pvscsi.c
> > +++ b/hw/scsi/vmw_pvscsi.c
> > @@ -123,9 +123,9 @@ typedef struct {
> > /* Collector for current command data */
> > uint32_t curr_cmd_data[PVSCSI_MAX_CMD_DATA_WORDS];
> >
> > - uint8_t rings_info_valid; /* Whether data rings initialized */
> > - uint8_t msg_ring_info_valid; /* Whether message ring initialized */
> > - uint8_t use_msg; /* Whether to use message ring */
> > + bool rings_info_valid; /* Whether data rings initialized */
> > + bool msg_ring_info_valid; /* Whether message ring initialized */
> > + bool use_msg; /* Whether to use message ring */
> >
> > uint8_t msi_used; /* For migration compatibility */
> > PVSCSIRingInfo rings; /* Data transfer rings manager */
> > @@ -349,8 +349,8 @@ pvscsi_reset_state(PVSCSIState *s)
> > s->reg_command_status = PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> > s->reg_interrupt_status = 0;
> > pvscsi_ring_cleanup(&s->rings);
> > - s->rings_info_valid = FALSE;
> > - s->msg_ring_info_valid = FALSE;
> > + s->rings_info_valid = false;
> > + s->msg_ring_info_valid = false;
> > QTAILQ_INIT(&s->pending_queue);
> > QTAILQ_INIT(&s->completion_queue);
> > }
> > @@ -792,7 +792,7 @@ pvscsi_on_cmd_setup_rings(PVSCSIState *s)
> > pvscsi_dbg_dump_tx_rings_config(rc);
> > pvscsi_ring_init_data(&s->rings, rc);
> >
> > - s->rings_info_valid = TRUE;
> > + s->rings_info_valid = true;
> > return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> > }
> >
> > @@ -874,7 +874,7 @@ pvscsi_on_cmd_setup_msg_ring(PVSCSIState *s)
> > if (pvscsi_ring_init_msg(&s->rings, rc) < 0) {
> > return PVSCSI_COMMAND_PROCESSING_FAILED;
> > }
> > - s->msg_ring_info_valid = TRUE;
> > + s->msg_ring_info_valid = true;
> > }
> > return sizeof(PVSCSICmdDescSetupMsgRing) / sizeof(uint32_t);
> > }
> > @@ -1232,9 +1232,9 @@ static const VMStateDescription vmstate_pvscsi = {
> > VMSTATE_UINT32(curr_cmd_data_cntr, PVSCSIState),
> > VMSTATE_UINT32_ARRAY(curr_cmd_data, PVSCSIState,
> > ARRAY_SIZE(((PVSCSIState *)NULL)->curr_cmd_data)),
> > - VMSTATE_UINT8(rings_info_valid, PVSCSIState),
> > - VMSTATE_UINT8(msg_ring_info_valid, PVSCSIState),
> > - VMSTATE_UINT8(use_msg, PVSCSIState),
> > + VMSTATE_BOOL(rings_info_valid, PVSCSIState),
> > + VMSTATE_BOOL(msg_ring_info_valid, PVSCSIState),
> > + VMSTATE_BOOL(use_msg, PVSCSIState),
>
> I believe this change the migration data structure. This
> area is described in "Changing migration data structures"
> in docs/devel/migration.rst.
>
> If this structure were not affected, your change would be
> a good cleanup. However changing migration can become a
> nightmare, so ... cleaning this is hard.
>
> Cc'ing Dave (a migration maintainer) as I'm not sure there
> already is a document describing easily this problem.
Well; the migration structures are...very unstructured.
It turns out that VMSTATE_BOOL already reads/writes a raw
byte to the file - so it's the same size as the VMSTATE_UINT8 at least.
Disappointingly get_bool and put_bool in vmstate-types.c don't do any
sanitation either; so you get whatever the raw value is.
glib's FALSE/TRUE is:
#define FALSE (0)
#define TRUE (!FALSE)
so that's probably fine; as long as no one does anything silly like
compare the value with TRUE or true.
Dave
> >
> > VMSTATE_UINT64(rings.rs_pa, PVSCSIState),
> > VMSTATE_UINT32(rings.txr_len_mask, PVSCSIState),
> > @@ -1255,7 +1255,7 @@ static const VMStateDescription vmstate_pvscsi = {
> > };
> >
> > static Property pvscsi_properties[] = {
> > - DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
> > + DEFINE_PROP_BOOL("use_msg", PVSCSIState, use_msg, true),
> > DEFINE_PROP_BIT("x-old-pci-configuration", PVSCSIState, compat_flags,
> > PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT, false),
> > DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
> > --
> > 2.28.0
> >
> > This is my first qemu patch. Let know if there are any mistakes
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2024 Red Hat, Inc.