On Thu, May 30, 2024 at 01:11:26PM -0400, Steven Sistare wrote:
> On 5/29/2024 2:53 PM, Peter Xu wrote:
> > On Wed, May 29, 2024 at 01:30:18PM -0400, Steven Sistare wrote:
> > > How about a more general name for the type:
> > > 
> > > migration/misc.h
> > >      typedef char (MigrationId)[256];
> > 
> > How about qemu/typedefs.h?  Not sure whether it's applicable. Markus (in
> > the loop) may have a better idea.
> > 
> > Meanwhile, s/MigrationID/IDString/?
> 
> typedefs.h has a different purpose; giving short names to types
> defined in internal include files.
> 
> This id is specific to migration, so I still think its name should reflect
> migration and it belongs in some include/migration/*.h file.
> 
> ramblocks and migration are already closely related.  There is nothing wrong
> with including a migration header in ramblock.h so it can use a migration 
> type.
> We already have:
>   include/hw/acpi/ich9_tco.h:#include "migration/vmstate.h"
>   include/hw/display/ramfb.h:#include "migration/vmstate.h"
>   include/hw/input/pl050.h:#include "migration/vmstate.h"
>   include/hw/pci/shpc.h:#include "migration/vmstate.h"
>   include/hw/virtio/virtio.h:#include "migration/vmstate.h"
>   include/hw/hyperv/vmbus.h:#include "migration/vmstate.h"
> 
> The 256 byte magic length already appears in too many places, and my code
> would add more occurrences, so I really think that abstracting this type
> would be cleaner.

I agree having a typedef is nicer, but I don't understand why it must be
migration related.  It can be the type QEMU uses to represent any string
based ID, and that's a generic concept to me.

Migration can have a wrapper to process that type, but then migration will
include the generic header in that case, it feels more natural that way?

-- 
Peter Xu


Reply via email to