[1/3] two new file wrappers

These patches implement asn1 ber visitors for encoding and decoding data.
References: <20130226230354.982917686@linux.vnet.ibm.com>
Content-Disposition: inline; filename=qemu_file_bits.diff

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 include/migration/qemu-file.h |    4 ++++
 qemu-file.c                   |   33 ++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

jschopp@linux.vnet.ibm.com writes:

> These patches implement asn1 ber visitors for encoding and decoding data.
> References: <20130226230354.982917686@linux.vnet.ibm.com>
> Content-Disposition: inline; filename=qemu_file_bits.diff

Not sure how you sent this but it's not threaded properly and the diffs
aren't git diffs.  Please use git-send-email.

>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
> ---
>  include/migration/qemu-file.h |    4 ++++
>  qemu-file.c                   |   33 ++++++++++++++++++++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
>
> Index: b/qemu-file.c
> ===================================================================
> --- a/qemu-file.c
> +++ b/qemu-file.c
> @@ -367,7 +367,7 @@ static void qemu_file_set_error(QEMUFile
>  /** Flushes QEMUFile buffer
>   *
>   */
> -static int qemu_fflush(QEMUFile *f)
> +int qemu_fflush(QEMUFile *f)
>  {
>      int ret = 0;
>  
> @@ -668,3 +668,34 @@ uint64_t qemu_get_be64(QEMUFile *f)
>      v |= qemu_get_be32(f);
>      return v;
>  }
> +
> +int qemu_read_bytes(QEMUFile *f, uint8_t *buf, int size)
> +{
> +    if (qemu_file_get_error(f)) {
> +        return -1;
> +    }
> +    return qemu_get_buffer(f, buf, size);
> +}
> +
> +int qemu_peek_bytes(QEMUFile *f, uint8_t *buf, int size, size_t offset)
> +{
> +    if (qemu_file_get_error(f)) {
> +        return -1;
> +    }
> +    return qemu_peek_buffer(f, buf, size, offset);
> +}
> +
> +int qemu_write_bytes(QEMUFile *f, const uint8_t *buf, int size)
> +{
> +    if (qemu_file_get_error(f)) {
> +        return -1;
> +    }
> +
> +    qemu_put_buffer(f, buf, size);
> +
> +    if (qemu_file_get_error(f)) {
> +        return -1;
> +    }
> +
> +    return size;
> +}

I think we've moved away from using qemu-file for anything other than
migration.

Regards,

Anthony Liguori

> Index: b/include/migration/qemu-file.h
> ===================================================================
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -79,11 +79,15 @@ QEMUFile *qemu_fdopen(int fd, const char
>  QEMUFile *qemu_fopen_socket(int fd);
>  QEMUFile *qemu_popen(FILE *popen_file, const char *mode);
>  QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
> +int qemu_fflush(QEMUFile *f);
>  int qemu_get_fd(QEMUFile *f);
>  int qemu_fclose(QEMUFile *f);
>  int64_t qemu_ftell(QEMUFile *f);
>  void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
>  void qemu_put_byte(QEMUFile *f, int v);
> +int qemu_read_bytes(QEMUFile *f, uint8_t *buf, int size);
> +int qemu_peek_bytes(QEMUFile *f, uint8_t *buf, int size, size_t offset);
> +int qemu_write_bytes(QEMUFile *f, const uint8_t *buf, int size);
>  
>  static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
>  {

> Not sure how you sent this but it's not threaded properly and the diffs
> aren't git diffs.  Please use git-send-email.

I used quilt mail.  I apologize for the improper threading.  I'll switch 
over to git-send-email in the future.

>> +int qemu_write_bytes(QEMUFile *f, const uint8_t *buf, int size)
>> +{
>> +    if (qemu_file_get_error(f)) {
>> +        return -1;
>> +    }
>> +
>> +    qemu_put_buffer(f, buf, size);
>> +
>> +    if (qemu_file_get_error(f)) {
>> +        return -1;
>> +    }
>> +
>> +    return size;
>> +}
>
> I think we've moved away from using qemu-file for anything other than
> migration.
>

Any suggestions for what should be used instead?

On 02/26/2013 09:19 PM, Anthony Liguori wrote:
> jschopp@linux.vnet.ibm.com writes:
>
> I think we've moved away from using qemu-file for anything other than
> migration.

Our goal was to use the abstraction of a QEMUFile for writing into 
memory buffers and later on to also provide a wrapper for writing into 
block devices. This was all to be used for the NVRAM implementation 
necessary for storing TPM persistent state.

    Stefan

On Wed, Feb 27, 2013 at 08:46:19AM -0500, Stefan Berger wrote:
> On 02/26/2013 09:19 PM, Anthony Liguori wrote:
> >jschopp@linux.vnet.ibm.com writes:
> >
> >I think we've moved away from using qemu-file for anything other than
> >migration.
> 
> Our goal was to use the abstraction of a QEMUFile for writing into
> memory buffers and later on to also provide a wrapper for writing
> into block devices. This was all to be used for the NVRAM
> implementation necessary for storing TPM persistent state.

One approach is to just keep all of this inside BER*Visitor, and do the
distinction between memory/file via, for example,
ber_output_visitor_new_file()/ber_output_visitor_new[_buffer]()

Would be kinda cool to move that into a {BE,LE}{File,Buffer}Visitor
so you could use the visitor interfaces to handle the endian-conversions
automagically, and then use that in BER*Visitor internally, but I'm not
not sure how well that would work in practice.

Although, I did try to do something like that in the past to use it as a shim
between the qemu_{put|get}* interfaces and QEMUFile in a misguided attempt to
introduce a BER protocol for migration, but something similar might still
be useful in your case:

http://lists.gnu.org/archive/html/qemu-devel/2011-09/msg02466.html

There's currently no visitor interface for arbitrary-length byte buffers
though. The aforementioned patches relied on users creating their
own loops around visit_type_int*() and friends. There's a patch floating
around to add them but I don't see a need for it until we have a need to
add support for specifying arrays in schema-defined QAPI types.

Just another approach to consider though, I wouldn't see a problem with all
this being done inside BER*Visitor for now.

> 
>    Stefan
> 
>

On Wed, Feb 27, 2013 at 03:56:27PM -0600, mdroth wrote:
> On Wed, Feb 27, 2013 at 08:46:19AM -0500, Stefan Berger wrote:
> > On 02/26/2013 09:19 PM, Anthony Liguori wrote:
> > >jschopp@linux.vnet.ibm.com writes:
> > >
> > >I think we've moved away from using qemu-file for anything other than
> > >migration.
> > 
> > Our goal was to use the abstraction of a QEMUFile for writing into
> > memory buffers and later on to also provide a wrapper for writing
> > into block devices. This was all to be used for the NVRAM
> > implementation necessary for storing TPM persistent state.
> 
> One approach is to just keep all of this inside BER*Visitor, and do the
> distinction between memory/file via, for example,
> ber_output_visitor_new_file()/ber_output_visitor_new[_buffer]()
> 
> Would be kinda cool to move that into a {BE,LE}{File,Buffer}Visitor
> so you could use the visitor interfaces to handle the endian-conversions
> automagically, and then use that in BER*Visitor internally, but I'm not
> not sure how well that would work in practice.
> 
> Although, I did try to do something like that in the past to use it as a shim
> between the qemu_{put|get}* interfaces and QEMUFile in a misguided attempt to
> introduce a BER protocol for migration, but something similar might still
> be useful in your case:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2011-09/msg02466.html
> 
> There's currently no visitor interface for arbitrary-length byte buffers
> though. The aforementioned patches relied on users creating their
> own loops around visit_type_int*() and friends. There's a patch floating
> around to add them but I don't see a need for it until we have a need to
> add support for specifying arrays in schema-defined QAPI types.

... okay, now i see the need :) see my response in patch 2.

> 
> Just another approach to consider though, I wouldn't see a problem with all
> this being done inside BER*Visitor for now.
> 
> > 
> >    Stefan
> > 
> >