Message ID | 20200913234451.170022-1-ameynarkhede03@gmail.com |
---|---|
State | New |
Headers | show |
Series | hw/scsi/vmw_pvscsi.c: Fix wrong usage of gboolean types in PVSCSIState | expand |
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 >
* 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 > > >
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,
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(-) -- 2.28.0 This is my first qemu patch. Let know if there are any mistakes