Re: [PATCH spice-common 7/9] codegen: Remove support for --ptrsize

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> 

Thanks, looking at reply to 9/9 however I think one thing is not clear from
what I wrote in the commit message.
The reason is not only that was used in protocol 1 only and now is not used
but also that it was wrong from the beginning and useless. I would keep it
if I found it useful.

> On Sun, Mar 03, 2019 at 07:10:28PM +0000, Frediano Ziglio wrote:
> > This option was used in protocol 1 to generate 64 bit pointers.
> > A pointer in the protocol is an offset in the current message.
> > This allows the possibility to have messages with pointers with
> > more than 4GB. This feature was removed and not used in protocol 2.
> > The reason is that messages more than 4GB would cause:

Maybe this could be changed in

"The reason this feature was correctly removed in protocol 2 is that
having 64 bit pointers in the protocol would require messages larger
than 4GB which would cause:"

> > - huge latency as a single message would take more than 4 seconds
> >   to be send in a 10Gb connection;
> > - huge memory requirements.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> >  common/marshaller.c       | 14 +++-----------
> >  common/marshaller.h       |  2 +-
> >  python_modules/marshal.py |  2 +-
> >  python_modules/ptypes.py  | 18 +++---------------
> >  spice_codegen.py          |  4 ----
> >  5 files changed, 8 insertions(+), 32 deletions(-)
> > 
> > diff --git a/common/marshaller.c b/common/marshaller.c
> > index 4379aa6..c77129b 100644
> > --- a/common/marshaller.c
> > +++ b/common/marshaller.c
> > @@ -96,7 +96,6 @@ typedef struct SpiceMarshallerData SpiceMarshallerData;
> >  typedef struct {
> >      SpiceMarshaller *marshaller;
> >      int item_nr;
> > -    int is_64bit;
> >      size_t offset;
> >  } MarshallerRef;
> >  
> > @@ -419,13 +418,13 @@ SpiceMarshaller
> > *spice_marshaller_get_submarshaller(SpiceMarshaller *m)
> >      return m2;
> >  }
> >  
> > -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m, int is_64bit)
> > +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m)
> >  {
> >      SpiceMarshaller *m2;
> >      uint8_t *p;
> >      int size;
> >  
> > -    size = is_64bit ? 8 : 4;
> > +    size = 4;
> >  
> >      p = spice_marshaller_reserve_space(m, size);
> >      memset(p, 0, size);
> > @@ -433,7 +432,6 @@ SpiceMarshaller
> > *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m, int
> >      m2->pointer_ref.marshaller = m;
> >      m2->pointer_ref.item_nr = m->n_items - 1;
> >      m2->pointer_ref.offset = m->items[m->n_items - 1].len - size;
> > -    m2->pointer_ref.is_64bit = is_64bit;
> >  
> >      return m2;
> >  }
> > @@ -538,13 +536,7 @@ void spice_marshaller_flush(SpiceMarshaller *m)
> >      for (m2 = m; m2 != NULL; m2 = m2->next) {
> >          if (m2->pointer_ref.marshaller != NULL && m2->total_size > 0) {
> >              ptr_pos = lookup_ref(&m2->pointer_ref);
> > -            if (m2->pointer_ref.is_64bit) {
> > -                write_uint64(ptr_pos,
> > -                             spice_marshaller_get_offset(m2));
> > -            } else {
> > -                write_uint32(ptr_pos,
> > -                             spice_marshaller_get_offset(m2));
> > -            }
> > +            write_uint32(ptr_pos, spice_marshaller_get_offset(m2));
> >          }
> >      }
> >  }
> > diff --git a/common/marshaller.h b/common/marshaller.h
> > index 041d16e..a631c85 100644
> > --- a/common/marshaller.h
> > +++ b/common/marshaller.h
> > @@ -53,7 +53,7 @@ size_t spice_marshaller_get_offset(SpiceMarshaller *m);
> >  size_t spice_marshaller_get_size(SpiceMarshaller *m);
> >  size_t spice_marshaller_get_total_size(SpiceMarshaller *m);
> >  SpiceMarshaller *spice_marshaller_get_submarshaller(SpiceMarshaller *m);
> > -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m, int is_64bit);
> > +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m);
> >  int spice_marshaller_fill_iovec(SpiceMarshaller *m, struct iovec *vec,
> >                                  int n_vec, size_t skip_bytes);
> >  void *spice_marshaller_add_uint64(SpiceMarshaller *m, uint64_t v);
> > diff --git a/python_modules/marshal.py b/python_modules/marshal.py
> > index 4e98993..74f3a54 100644
> > --- a/python_modules/marshal.py
> > +++ b/python_modules/marshal.py
> > @@ -234,7 +234,7 @@ def write_array_marshaller(writer, member, array,
> > container_src, scope):
> >  def write_pointer_marshaller(writer, member, src):
> >      t = member.member_type
> >      ptr_func = write_marshal_ptr_function(writer, t.target_type)
> > -    submarshaller = "spice_marshaller_get_ptr_submarshaller(m, %d)" % (1
> > if member.get_fixed_nw_size() == 8 else 0)
> > +    submarshaller = "spice_marshaller_get_ptr_submarshaller(m)"
> >      if member.has_attr("marshall"):
> >          rest_args = ""
> >          if t.target_type.is_array():
> > diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
> > index 64c198e..bd5f542 100644
> > --- a/python_modules/ptypes.py
> > +++ b/python_modules/ptypes.py
> > @@ -4,8 +4,6 @@ import types
> >  _types_by_name = {}
> >  _types = []
> >  
> > -default_pointer_size = 4
> > -
> >  def type_exists(name):
> >      return name in _types_by_name
> >  
> > @@ -512,7 +510,6 @@ class PointerType(Type):
> >          Type.__init__(self)
> >          self.name = None
> >          self.target_type = target_type
> > -        self.pointer_size = default_pointer_size
> >  
> >      def __str__(self):
> >          return "%s*" % (str(self.target_type))
> > @@ -521,9 +518,6 @@ class PointerType(Type):
> >          self.target_type = self.target_type.resolve()
> >          return self
> >  
> > -    def set_ptr_size(self, new_size):
> > -        self.pointer_size = new_size
> > -
> >      def is_fixed_nw_size(self):
> >          return True
> >  
> > @@ -531,19 +525,13 @@ class PointerType(Type):
> >          return True
> >  
> >      def primitive_type(self):
> > -        if self.pointer_size == 4:
> > -            return "uint32"
> > -        else:
> > -            return "uint64"
> > +        return "uint32"
> >  
> >      def get_fixed_nw_size(self):
> > -        return self.pointer_size
> > +        return 4
> >  
> >      def c_type(self):
> > -        if self.pointer_size == 4:
> > -            return "uint32_t"
> > -        else:
> > -            return "uint64_t"
> > +        return "uint32_t"
> >  
> >      def contains_extra_size(self):
> >          return True
> > diff --git a/spice_codegen.py b/spice_codegen.py
> > index 66f99a5..e518686 100755
> > --- a/spice_codegen.py
> > +++ b/spice_codegen.py
> > @@ -171,8 +171,6 @@ parser.add_option("-i", "--include",
> >                    help="Include FILE in generated code")
> >  parser.add_option("--prefix", dest="prefix",
> >                    help="set public symbol prefix", default="")
> > -parser.add_option("--ptrsize", dest="ptrsize",
> > -                  help="set default pointer size", default="4")
> >  parser.add_option("--license", dest="license",
> >                    help="license to use for generated file(s) (LGPL/BSD)",
> >                    default="LGPL")
> >  parser.add_option("--generated-header", dest="generated_header",
> >  metavar="FILE",
> > @@ -188,8 +186,6 @@ if len(args) == 0:
> >  if len(args) == 1:
> >      parser.error("No destination file specified")
> >  
> > -ptypes.default_pointer_size = int(options.ptrsize)
> > -
> >  proto_file = args[0]
> >  dest_file = args[1]
> >  proto = spice_parser.parse(proto_file)
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]