Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Thu, Jun 09, 2022 at 01:56:00PM +0100, Peter Maydell wrote: > > On Tue, 24 May 2022 at 12:46, Daniel P. Berrangé > > wrote: > > > > > > The qemu_update_position method name gives the misleading impression > > > that it is changing the current file offset. Most of the files are > > > just streams, however, so there's no concept of a file offset in the > > > general case. > > > > > > What this method is actually used for is to report on the number of > > > bytes that have been transferred out of band from the main I/O methods. > > > This new name better reflects this purpose. > > > > > > Signed-off-by: Daniel P. Berrangé > > > > > int qemu_peek_byte(QEMUFile *f, int offset); > > > void qemu_file_skip(QEMUFile *f, int size); > > > -void qemu_update_position(QEMUFile *f, size_t size); > > > +/* > > > + * qemu_file_credit_transfer: > > > + * > > > + * Report on a number of bytes that have been transferred > > > + * out of band from the main file object I/O methods. > > > + */ > > > +void qemu_file_credit_transfer(QEMUFile *f, size_t size); > > > void qemu_file_reset_rate_limit(QEMUFile *f); > > > void qemu_file_update_transfer(QEMUFile *f, int64_t len); > > > void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate); > > > > What's the difference between "credit transfer" and "update > > transfer" ? The latter also seems to just be adding a number > > to a count of bytes-transferred... > > The other method is merely related to the rate limiting, and so > probably ought to have 'rate_limit' included in its name too. Bleh that's messy; I see update_transfer is used in the multifd case as well, so makes sense for stats only and not a position in a stream that only makes sense for a single fd. (But now doesn't make any sense any more with these changes either) Dave > With regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
On Tue, 24 May 2022 at 12:46, Daniel P. Berrangé wrote: > > The qemu_update_position method name gives the misleading impression > that it is changing the current file offset. Most of the files are > just streams, however, so there's no concept of a file offset in the > general case. > > What this method is actually used for is to report on the number of > bytes that have been transferred out of band from the main I/O methods. > This new name better reflects this purpose. > > Signed-off-by: Daniel P. Berrangé > int qemu_peek_byte(QEMUFile *f, int offset); > void qemu_file_skip(QEMUFile *f, int size); > -void qemu_update_position(QEMUFile *f, size_t size); > +/* > + * qemu_file_credit_transfer: > + * > + * Report on a number of bytes that have been transferred > + * out of band from the main file object I/O methods. > + */ > +void qemu_file_credit_transfer(QEMUFile *f, size_t size); > void qemu_file_reset_rate_limit(QEMUFile *f); > void qemu_file_update_transfer(QEMUFile *f, int64_t len); > void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate); What's the difference between "credit transfer" and "update transfer" ? The latter also seems to just be adding a number to a count of bytes-transferred... -- PMM
Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
On Thu, Jun 09, 2022 at 01:56:00PM +0100, Peter Maydell wrote: > On Tue, 24 May 2022 at 12:46, Daniel P. Berrangé wrote: > > > > The qemu_update_position method name gives the misleading impression > > that it is changing the current file offset. Most of the files are > > just streams, however, so there's no concept of a file offset in the > > general case. > > > > What this method is actually used for is to report on the number of > > bytes that have been transferred out of band from the main I/O methods. > > This new name better reflects this purpose. > > > > Signed-off-by: Daniel P. Berrangé > > > int qemu_peek_byte(QEMUFile *f, int offset); > > void qemu_file_skip(QEMUFile *f, int size); > > -void qemu_update_position(QEMUFile *f, size_t size); > > +/* > > + * qemu_file_credit_transfer: > > + * > > + * Report on a number of bytes that have been transferred > > + * out of band from the main file object I/O methods. > > + */ > > +void qemu_file_credit_transfer(QEMUFile *f, size_t size); > > void qemu_file_reset_rate_limit(QEMUFile *f); > > void qemu_file_update_transfer(QEMUFile *f, int64_t len); > > void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate); > > What's the difference between "credit transfer" and "update > transfer" ? The latter also seems to just be adding a number > to a count of bytes-transferred... The other method is merely related to the rate limiting, and so probably ought to have 'rate_limit' included in its name too. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> The qemu_update_position method name gives the misleading impression
> that it is changing the current file offset. Most of the files are
> just streams, however, so there's no concept of a file offset in the
> general case.
>
> What this method is actually used for is to report on the number of
> bytes that have been transferred out of band from the main I/O methods.
> This new name better reflects this purpose.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/qemu-file.c | 4 ++--
> migration/qemu-file.h | 8 +++-
> migration/ram.c | 2 +-
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 664ac77067..9a7f715e17 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -319,7 +319,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
> block_offset,
> if (ret != RAM_SAVE_CONTROL_DELAYED &&
> ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> if (bytes_sent && *bytes_sent > 0) {
> -qemu_update_position(f, *bytes_sent);
> +qemu_file_credit_transfer(f, *bytes_sent);
> } else if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -374,7 +374,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> return len;
> }
>
> -void qemu_update_position(QEMUFile *f, size_t size)
> +void qemu_file_credit_transfer(QEMUFile *f, size_t size)
> {
> f->total_transferred += size;
> }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index febc961aa9..81f6fd7db8 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -186,7 +186,13 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
> */
> int qemu_peek_byte(QEMUFile *f, int offset);
> void qemu_file_skip(QEMUFile *f, int size);
> -void qemu_update_position(QEMUFile *f, size_t size);
> +/*
> + * qemu_file_credit_transfer:
> + *
> + * Report on a number of bytes that have been transferred
> + * out of band from the main file object I/O methods.
> + */
> +void qemu_file_credit_transfer(QEMUFile *f, size_t size);
> void qemu_file_reset_rate_limit(QEMUFile *f);
> void qemu_file_update_transfer(QEMUFile *f, int64_t len);
> void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
> diff --git a/migration/ram.c b/migration/ram.c
> index 89082716d6..bf321e1e72 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2301,7 +2301,7 @@ void acct_update_position(QEMUFile *f, size_t size,
> bool zero)
> } else {
> ram_counters.normal += pages;
> ram_transferred_add(size);
> -qemu_update_position(f, size);
> +qemu_file_credit_transfer(f, size);
> }
> }
>
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
