Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Apr 27, 2011 at 04:04:44PM +0200, Jan Kiszka wrote:
> On 2011-04-27 15:34, Michael S. Tsirkin wrote:
> > On Sat, Apr 23, 2011 at 12:23:35PM +0200, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> This structure will be used for legacy MSI as well and will become part
> >> of the KVM API.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > I'd like KVMMSIMessage better. struct kvm_msi_message would be even
> > better but oh well.
> 
> Yeah, QEMU naming convention is not designed for such use cases... Will
> change it as you prefer.
> 
> > 
> >> ---
> >>  hw/msix.c |   10 +++++-----
> >>  hw/pci.h  |   10 ++--------
> >>  kvm.h     |    7 +++++++
> >>  3 files changed, 14 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/hw/msix.c b/hw/msix.c
> >> index 8b68a6a..42870c5 100644
> >> --- a/hw/msix.c
> >> +++ b/hw/msix.c
> >> @@ -51,7 +51,7 @@ int msix_supported;
> >>  static void kvm_msix_free(PCIDevice *dev)
> >>  {
> >>      int vector, changed = 0;
> >> -    struct kvm_msix_message *kmm;
> >> +    KVMMsiMessage *kmm;
> >>  
> >>      for (vector = 0; vector < dev->msix_entries_nr; ++vector) {
> >>          if (dev->msix_entry_used[vector]) {
> >> @@ -66,7 +66,7 @@ static void kvm_msix_free(PCIDevice *dev)
> >>  }
> >>  
> >>  static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,
> >> -                                         struct kvm_msix_message *kmm)
> >> +                                         KVMMsiMessage *kmm)
> >>  {
> >>      uint8_t *table_entry = dev->msix_table_page + vector * MSIX_ENTRY_SIZE;
> >>  
> >> @@ -78,7 +78,7 @@ static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,
> >>  static void kvm_msix_update(PCIDevice *dev, int vector,
> >>                              int was_masked, int is_masked)
> >>  {
> >> -    struct kvm_msix_message e = {}, *entry;
> >> +    KVMMsiMessage e = {}, *entry;
> >>      int mask_cleared = was_masked && !is_masked;
> >>      /* It is only legal to change an entry when it is masked. Therefore, it is
> >>       * enough to update the routing in kernel when mask is being cleared. */
> >> @@ -114,7 +114,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector,
> >>  
> >>  static int kvm_msix_add(PCIDevice *dev, unsigned vector)
> >>  {
> >> -    struct kvm_msix_message *kmm = dev->msix_irq_entries + vector;
> >> +    KVMMsiMessage *kmm = dev->msix_irq_entries + vector;
> >>      int r;
> >>  
> >>      if (!kvm_has_gsi_routing()) {
> >> @@ -147,7 +147,7 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)
> >>  
> >>  static void kvm_msix_del(PCIDevice *dev, unsigned vector)
> >>  {
> >> -    struct kvm_msix_message *kmm;
> >> +    KVMMsiMessage *kmm;
> >>  
> >>      if (dev->msix_entry_used[vector]) {
> >>          return;
> >> diff --git a/hw/pci.h b/hw/pci.h
> >> index dd09494..dc5df17 100644
> >> --- a/hw/pci.h
> >> +++ b/hw/pci.h
> >> @@ -5,6 +5,7 @@
> >>  #include "qobject.h"
> >>  
> >>  #include "qdev.h"
> >> +#include "kvm.h"
> > 
> > I put kvm_msix_message in pci.h to avoid having every pci device pull in kvm.h
> > is anything wrong with that?  Maybe just rename it to make it generic
> > for msi and leave if where it is.
> 
> kvm.h shall provide kvm related types, not some unrelated header. That's
> even more important with MSI support for non-PCI devices (aka HPET).
> 
> Jan

Generally qemu suggests forward-declaring types so all users don't
need to pull them in. This should certainly work here:
declare the type in qemu-common.h, then msix.c and kvm are
the only ones that need to include kvm.h


> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux